ygrek / sqlgg

SQL Guided (code) Generator
https://ygrek.org/p/sqlgg/
GNU General Public License v2.0
61 stars 20 forks source link

add insert_id to sqlgg_mariadb execute response #140

Closed tatchi closed 3 weeks ago

tatchi commented 1 month ago

add support for https://github.com/ocaml-community/ocaml-mariadb/pull/58

tatchi commented 1 month ago

So it looks like it's not that simple. I hit this error in a generated code by sqlgg with mariadb and the changes in this PR.

let add_many_person db ~persons =
  ( match persons with [] -> IO.return 0L | _ :: _ -> T.execute db ....

Error: This expression has type T.execute_response IO.future
       but an expression was expected of type int64 IO.future
       Type T.execute_response is not compatible with type int64 

I changed OL to the new execute_response, i.e { affected_rows = 0; insert_id = 0 } but the problem is that it doesn't work for e.g sqlite which expects execute_response to be an int64.

I guess if we want to support different execute_response for each implementations we'd have to expose a way to construct such response in the Traits (not entirely sure it'd work?).

Fortunately, I realized that we could also get the insert_id with sqlite, so instead we could just have all implementations to return the same type (as currently). That's what I did in e5e9626 (#140). You can see that I use int64 because there's always at least one implementation that requires it, and we need to use a common type. This is probably also why we currently have int64 and not int.

EDIT: I'm realizing that it changes the sqlgg interface, which I think you were against. Can you think of another way to do it ?