Table name can't be dynamic

[adrien-n]

I wanted to do something like

[%sql "CREATE TABLE pads_%s (id);"]

However this does not work and I get an error at runtime about "pads_?" from sqlite. I also get an error when using SELECT queries.

I guess this isn't supported and I have to admit I don't know how it should be done with arbitrary names (and that's also the reason I wanted to do it through sqlexpr). If it's not possible, sqlexpr should refuse such expressions at compile-time.

(sorry, I mistyped and sent the issue too soon)

[adrien-n]

It seems that sqlite3 itself cannot handle this. When using ocaml-sqlite3 directly, I get the following exception: Sqlite3.Error("Sqlite3.prepare: near \":name\": syntax error") (with ":name" being the placeholder for the table name).

[mfp]

On Sun, Jul 16, 2017 at 02:13:52AM -0700, adrien-n wrote:

It seems that sqlite3 itself cannot handle this. When using ocaml-sqlite3 directly, I get the following exception: Sqlite3.Error("Sqlite3.prepare: near \":name\": syntax error") (with ":name" being the placeholder for the table name).

Indeed, ocaml-sqlexpr uses (cached) prepared statements under the hood, which cannot be parametrized arbitrarily -- you cannot replace table or column names dynamically, for instance.

In order to execute such SQL statements, you will need to go the traditional unsafe route, and build the textual statement by hand (with the usual precautions to avoid SQL injection, etc.).

I believed the required "unsafe eval" function was exposed, but when I went check the API I found it was in POOL and not directly usable. This goes to show the number of times I have used string-based SQL execution externally (Sqlexpr does use unsafe_execute internally to implement transactions).

This doesn't mean you cannot build such queries manually though, you just need to use a single "literal" directive manually, as shown in the following self-contained example:

(* ocamlfind ocamlopt -o tsql -package sqlexpr,sqlexpr.ppx -linkpkg tsql.ml  -thread *)
open Printf

module Sqlexpr = Sqlexpr_sqlite.Make(Sqlexpr_concurrency.Id)
module S = Sqlexpr

let () =
  let db = S.open_db "foo.db" in
    S.execute db
      [%sql "CREATE TABLE IF NOT EXISTS users(login TEXT NOT NULL PRIMARY KEY, \
                                              password TEXT NOT NULL)"];
  S.iter db
    (fun (n, p) -> Printf.printf "User %S, password %S\n" n p)
    [%sqlc "SELECT @s{login}, @s{password} FROM users"];
  List.iter
    (fun (n, p) -> S.execute db [%sqlc "INSERT INTO users VALUES(%s, %s)"] n p)
    [
     "coder24", "badpass";
     "tokyo3", "12345"
    ];

  (*************************************************************************)
  (* ----------------->  build the SQL statement manually <--------------- *)
  (*************************************************************************)
  let update login field v =
    let escape x = "'" ^ x ^ "'" in (* FIXME *)

    let sql =
      sprintf "UPDATE users SET %s=%s WHERE login=%s" field (escape v) (escape login)
    in
      {
        S.sql_statement = sql;
        stmt_id         = None;
        directive       = S.Directives.literal sql;
      }
  in
    S.execute db @@ update "tokyo3" "password" "camel"

-- Mauricio Fernández

[adrien-n]

Thanks a lot for your input. Is it possible to mix the unsafe and safe ways? For instance, it would be handy to build the beginning of the query in an unsafe manner and then use the safe API for the rest.

[mfp]

On Sun, Jul 16, 2017 at 11:05:19AM +0000, adrien-n wrote:

Thanks a lot for your input. Is it possible to mix the unsafe and safe ways? For instance, it would be handy to build the beginning of the query in an unsafe manner and then use the safe API for the rest.

As I thought about this, I kept running into the usual problems of composability with the added twist of extra dynamism/unsafety in allowing ad-hoc textual substitutions in the statement (was thinking about something like context-aware directives and "first-order" predicate composition limited to single table selects), but as it turns out there is an easy way to keep some semblance of type safety for this particular case:

(* schema:   CREATE TABLE pads_%s (edit BLOB NOT NULL, condensate BLOB) *)

let insert_into_pad pad =
  {
    S.sql_statement = sprintf "INSERT INTO pad_%s VALUES(?, ?)" pad;
    stmt_id         = None;
    directive       = (fun k1 k2 -> S.Directives.(blob (blob k1) k2));
  }
in
  S.execute db (insert_into_pad "bar") "editXXXX" "condensateXXXX"

This at least allows to reuse the rest of the API as-is, and retains some type-safety as long as the hand-written SQL statement generation is OK.

Would this cover your needs for dynamism in the statement generation?

A priori, it'd be possible to perform the above expansion with a new directive (say, %u for Unsafe, or even more contextually-aware, %t for Table?), but it's not really something I'd want to encourage, as we lose the current ability to test the SQL statements "semi-statically"...

-- Mauricio Fernández

[adrien-n]

Thanks a lot for the proposed solution. I need to test it but might be too busy this week; worst case, I'll do it next week.

I perfectly understand the will to not make it too easy to conduct unsafe operation, especially in a context where the user might forget that they're unsafe because everything else is safe.

Currently I am using ocaml-sqlite3 and your solution already seems a hundred time simpler so I doubt I'll consider it too complicated. :)

I'll report as soon as I can try this, thanks again.

[adrien-n]

Hi,

I finally got time to try what you've described, it has been fairly easy to do and I've pushed it in prose: https://gitlab.com/adrien-n/prose/commit/09851e76b56188dccf8a7f086032472a085bb81a

It is read much more easily as a side-by-side diff or in the file directly: https://gitlab.com/adrien-n/prose/blob/09851e76b56188dccf8a7f086032472a085bb81a/prose_db.eliom

After reading your explanation, the only thing I had trouble with was the order for binding values in prepared statements. My code is similar to the following:

let sql = Printf.sprintf "UPDATE %s SET condensate = ? WHERE rowid = ? AND condensate IS NULL" (table_name name) in fun (rowid, condensate) -> S.execute db { S.sql_statement = sql; stmt_id = None; directive = (fun k1 k2 -> S.Directives.(blob (int64 k1) k2)) } condensate rowid

There are two values to bind and I had to proceed with trial and error, and even now I'm not sure what the logic behind the order for blob (int64 k1) k2 is.

I think I can also introduce some caching of prepared statements but I'm not sure which value to cache.

In any case, it has been a pleasure to switch sqlexpr. :)

-- Adrien