bug#69320: 30.0.50; Support keyword-based substitutions in sqlite

[J.P.]

"J.P." <jp@neverwas.me> writes:

> Severity: wishlist
> Tags: patch
>
> SQLite integrations on other platforms tend to recognize keyword-like
> replacement specifiers in SQL statements and queries. For example, a
> statement like
>
>   INSERT INTO Messages(target, time, sender, text)
>   SELECT t.id, :time, :sender, :text From Targets t
>   JOIN Networks n ON t.network = n.id
>   WHERE n.name = :network and t.name = :sender
>
> would correspond to keyword parameters like
>
>   (:text "Hi"
>    :sender "Bob"
>    :network "MyNet"
>    :time "2012-06-30T23:59:60.419Z")
>
> In Emacs, we currently use positional substitutions only, like
>
>   INSERT INTO Messages(target, time, sender, text)
>   SELECT t.id,?, ?, ? From Targets t
>   JOIN Networks n ON t.network = n.id
>   WHERE n.name = ? and t.name = ?
>
> which are somewhat easy to flub, IMO. Keywords also have the added
> benefit of requiring only a single evaluation for repeated
> substitutions.

I've come to discover that positional arguments can be reused as well
(see below), which makes that last point somewhat less compelling.

>
> I've made a simplistic attempt at implementing this,

Too simplistic, actually. Apparently, the SQL compiler is meant to
support the mixing and matching of positional and named parameters [1],
which my initial version doesn't comply with (but see below).

[1] 
https://www.mail-archive.com/sqlite-users@mailinglists.sqlite.org/msg05313.html

> but as a perennial
> SQL novice (and a certified Emacs dummy), I'd prefer someone more in the
> know to adopt this or at least review it carefully.
>
[...]
> @@ -333,6 +338,7 @@ bind_values (sqlite3 *db, sqlite3_stmt *stmt, Lisp_Object 
> values)
>  {
>    sqlite3_reset (stmt);
>    int len;
> +  int kw_dex = 0;

I think one way to support interspersed param types would be to maintain
two separate indexes, e.g,

     int pos_dex = 0;

>    if (VECTORP (values))
>      len = ASIZE (values);
>    else
> @@ -341,6 +347,7 @@ bind_values (sqlite3 *db, sqlite3_stmt *stmt, Lisp_Object 
> values)
>    for (int i = 0; i < len; ++i)
>      {
>        int ret = SQLITE_MISMATCH;
> +      int j = (kw_dex ? kw_dex : i + 1);

which would mean changing this to something like

         int j = (kw_dex ? kw_dex : ++pos_dex);

>        Lisp_Object value;
>        if (VECTORP (values))
[...]
> --- a/test/src/sqlite-tests.el
> +++ b/test/src/sqlite-tests.el
> @@ -142,6 +142,37 @@ sqlite-param
>        (sqlite-select db "select * from test4 where col2 = ?" [1])
>        '(("foo" 1))))))
>  
> +(ert-deftest sqlite-keyword-params ()
> +  (skip-unless (sqlite-available-p))
> +  (let ((db (sqlite-open)))
> +    (sqlite-execute
> +     db "CREATE TABLE IF NOT EXISTS test4a (col1 TEXT, col2 NUMBER)")
> +    (sqlite-execute db "INSERT INTO test4a VALUES (:a, :b)" '(:a "foo" :b 1))
> +    (should
> +     (equal
> +      (sqlite-select db "SELECT * FROM test4a WHERE col2 = :a" '(:a 1))
> +      '(("foo" 1))))
> +    (should
> +     (equal
> +      (sqlite-select db "SELECT * FROM test4a WHERE col1 = :b" [:b "foo"])
> +      '(("foo" 1))))
> +
> +    ;; Template specifiers reused.
> +    (sqlite-execute
> +     db (concat "CREATE TABLE IF NOT EXISTS test4b ("
> +                " u, v, w, x, y, z, FOREIGN KEY(v) REFERENCES test4a(rowid)"
> +                ")"))
> +    ;; Here, t matches `col2' because it's a boolean and is coerced to 1.
> +    (sqlite-execute db (concat "INSERT INTO test4b(u, v, w, x, y, z)"
> +                               " SELECT :a, t.rowid, :b, :c, :d, :e"
                                                            ^

Here, replacing the :c with a ?

> +                               " FROM test4a as t "
> +                               " WHERE t.col1 = :a AND t.col2 = :b")
> +                    '(:a "foo" :b t :c false :d 3.14159 :e nil)) ; e is NULL

and removing the corresponding :c in this values list is meant to be
supported (but isn't in my initial patch). I believe something like the
changes mentioned above would fix this.

> +    (should
> +     (equal
> +      (sqlite-select db "SELECT * FROM test4b WHERE v = :a AND w = :a" '(:a 
> 1))

Re repeated positionals: the following query should be equivalent to the
last case just above:

  (sqlite-select db "SELECT * FROM test4b WHERE v = ? AND w = ?1" '(1))

> +      '(("foo" 1 1 0 3.14159 nil))))))
> +
>  (ert-deftest sqlite-binary ()
>    (skip-unless (sqlite-available-p))
>    (let (db)