paurkedal / ocaml-caqti

Cooperative-threaded access to relational data
https://paurkedal.github.io/ocaml-caqti/index.html
GNU Lesser General Public License v3.0
299 stars 36 forks source link

Add failing test for "?1 is null or ?1 = _" #77

Closed reynir closed 2 years ago

reynir commented 2 years ago

@hannesm and I observed a failing assertion in the sqlite3 driver when we perform a query of the form SELECT ... FROM ... WHERE ?1 IS NULL OR ?1 = .... This adds a failing test case to the bikereg example / test. It would be nicer to have a test case separate from the example.

paurkedal commented 2 years ago

The syntax for explicitly indexed parameters is $1, $2, ..., so the issue is rather lack of a useful error message saying that the number of formal and actual parameters does not match. The fix, however, reveals that PostgreSQL cannot infer the type of this parameters, and adding a cast, like SELECT * FROM bikereg WHERE CAST($1 AS varchar) IS NULL OR description = $1 is not accepted by MariaDB (even 10.3.31 which should support it as far as I read the manual). That can be fixed by splitting it up per database, though.

(BTW, an alternative way of writing search queries which only use a single parameter is SELECT * FROM bikereg WHERE COALESCE(description = ?, true).)

reynir commented 2 years ago

Thank you for improving the error message.

I updated the title to better reflect the code. As far as I can tell you can use ?1 instead of $1 with sqlite3? Or are they different? https://www.sqlite.org/c3ref/bind_blob.html

paurkedal commented 2 years ago

Caqti has its own syntax for parameters which offers a choice between ? and $n. The query strings are parsed into a Caqti_query.t representation, which can also be used directly. The drivers will convert this to their native format. So, it's not possible to use the parameter style of a particular driver. The reason for doing it this way is to factor out a common difference between backends, avoiding the need of providing per-backend query strings for common cases.

reynir commented 2 years ago

I opened a new PR #79 to raise an error when you try to use ?n non-linear substitutions. This PR was made because of a misconception on my side that those kinds of substitutions were allowed. I have been using ?n non-linear substitutions since January without realizing I wasn't supposed to :-)