IreneKnapp / direct-sqlite

MIT License
35 stars 54 forks source link

Add bindParameterIndex for working with named bound parameters #44

Closed nurpax closed 10 years ago

nurpax commented 10 years ago

Add Database.SQLite3.Direct support for querying the index of a named parameter in a statement.

Named parameters (e.g. 'SELECT :foo') allow for some convenient forms of developing SQL queries that are harder to write and maintain with positional arguments.

nurpax commented 10 years ago

@lpsmith would be a good reviewer for this too. Some design considerations below, please read, worth thinking this through before merging.

The automated test in my commit is a good example of how to use this API.

SQLite is a bit funny in the sense that the named parameters are not stripped of their :/@ prefixes when bound. E.g.

"SELECT :foo + @bar"

will have bound parameter names :foo and @bar as opposed to just foo and bar. I would've expected the latter. Some sqlite bindings (python https://docs.python.org/2/library/sqlite3.html) seem to rewrite these so that the : or @ char is stripped away. E.g.

# And this is the named style:
cur.execute("select * from people where name_last=:who and age=:age", {"who": who, "age": age})

OTOH, it looks like Go doesn't do any mangling of these names: https://godoc.org/code.google.com/p/go-sqlite/go1/sqlite3.

I think we should NOT touch the names but just pass directly to sqlite. This IMO is clear especially for Database.SQLite3.Direct. I'm not sure what I should do for sqlite-simple, I think I'm in favor of 1:1 passing there too. I think stripping the prefix correctly to allow mixing : and @ would mean also (at least partially) parsing the SQL query string which is something I've always wanted to keep away from.

nurpax commented 10 years ago

Oh, I actually meant to ping @joeyadams but Leon's thoughts are welcome too!

lpsmith commented 10 years ago

Heh, well I'm not all that familiar with the SQLite API, so I don't think I can offer up a very informed review, but I have been thinking of doing something similar with postgresql-simple, namely, to extend the quasiquoter to allow for named parameters.

However, libpq doesn't really offer comparable functionality, and my thought is to have a DRY interface. This seems a little low level and verbose, are you planning a higher-level binding at some point?

nurpax commented 10 years ago

@lpsmith Yeah, this is just the binding code (direct-sqlite package), I'm planning on building on top of this in sqlite-simple with a higher level API. I'm going to send some design considerations about it on database-devel about this once I get there. I had something like this in mind:

result <- queryNamed c "SELECT foo,bar FROM baz where id = :id" [(":id", 13)]

(This won't work as-is in practice, because each parameter value can have its own type and so we cannot stick all of them into a single list.)

I haven't been able to make my mind up whether the name in the higher level should be ":id" or "id" . :)

@IreneKnapp @joeyadams I think passing the names unchanged to the native library is the right choice here.

IreneKnapp commented 10 years ago

I agree that passing unchanged is correct. As soon as I review the actual code tomorrow, I'll get this merged.

nurpax commented 10 years ago

@IreneKnapp here we go, I added a helper function bindNamed too in order to be orthogonal with bind. AFAIAC this is good to go. I already implemented support to sqlite-simple using this.

The API in sqlite-simple turned out to be really nice, I found a nice trick from bos's wreq library for supporting parameter list of mixed types in a single list with GADTs. So this works:

queryNamed conn "SELECT :foo,:bar" [":foo" := ("foo" :: T.Text), ":bar" := (42 :: Int)]
IreneKnapp commented 10 years ago

This is a new feature, so I'm going to release to Hackage as well.