Finish ppx integration

[j0sh]

Tests now run and pass

[foretspaisibles]

How do you run the tests? I could contribute Travis CI integration. :)

BTW, since the project is called sqlexpr it is maybe a good idea to also use sqlexpr (instead of sql) to identify the extension point handled by the project. How do you see this?

[c-cube]

I think it's ok to type [%sql instead of [%sqlexpr, verbosity is important. Apart from that I find this transition to ppx very nice, the tests look good too.

[mfp]

I apologize for letting this aside for so long. I'm really swamped at the moment, but I'll try to take a look back at the ppx branch by the end of the week. IIRC it was essentially good to merge and the only thing missing relative to the camlp4 extension was sqlc statement hoisting? If I cannot see an obvious way to adapt that part from estring (my absolute lack of experience with ppx so far will likely get in the way) I'll probably merge as-is.

Also, [%sql is nice indeed. It's maybe an ambitious claim in the ppx namespace, but I don't anticipate anybody wanting to use 2 colliding extensions at once (the other ~SQL libs I can see on OPAM's repos are essentially alternative Sqlite3 libs, so it's one or the other, and PGOcaml, which AFAIK lacks a ppx extension but uses PGSQL in the camlp4 one)

[c-cube]

It might be naive, but would it be possible to have [%sqlc "foo"] simply declare a toplevel value let stmt_37 = Sqlite3.prepare "foo";; and then replace the query with stmt_37?

I suppose the issue is that cached statements depend on the connection, so they cannot be totally static...?

[j0sh]

The statement ID is generated and cached already with the [%sqlc ...] directive, unless estring does additional work that I'm missing? The ppx code in ppx_sqlexpr.ml is a fairly mechanical translation of pa_sql.ml, although I didn't look too much at what estring actually does.

[mfp]

On Mon, Feb 22, 2016 at 03:38:57PM -0800, Simon Cruanes wrote:

It might be naive, but would it be possible to have [%sqlc "foo"] simply declare a toplevel value let stmt_37 = Sqlite3.prepare "foo";; and then replace the query with stmt_37?

I suppose the issue is that cached statements depend on the connection, so they cannot be totally static...?

In fact the statement cache is held by Sqlexpr's DB handle, the statement carries an id used to retrieve the compiled statement from such cache.

estring essentially did the hoisting you showed in a more elegant way, without exposing new bindings in the toplevel enviroment.

Anyway, the effect of moving the statement definition is limited, since IIRC the only difference is the record not being allocated in the lexical site. So in the end it comes down to 6 words not being allocated.

Mauricio Fernández

[mfp]

On Mon, Feb 22, 2016 at 03:54:42PM -0800, Josh Allmann wrote:

The statement ID is generated and cached already with the [%sqlc ...] directive, unless estring does additional work that I'm missing? The ppx code in ppx_sqlexpr.ml is a fairly mechanical translation of pa_sql.ml, although I didn't look too much at what estring actually does.

Yes, the actual caching is already there. The only extra thing estring does is hoisting the record definition so that it's allocated statically when the module is initialized. IOW just sparing 6 words worth of allocation for a statement (O(params) for queries). I thought there was something else involved, but after a quick look at the code that seems to be all there's to it. I feel a bit silly now :)

Mauricio Fernández

[j0sh]

The only extra thing estring does is hoisting the record definition so that it's allocated statically when the module is initialized.

Ah, I see it now -- the register_shared_expr line in pa_sql.ml. Missed that part. Will look at achieving a similar effect here, it would be an interesting exercise even if the benefit is marginal.

[mfp]

hmm github got me scratching my head for a second, I just merged this into ppx and the latter into master, yet this is not detected as merged(?).

j0sh, are you still willing to do the shared expression thing? I've taken a look at ppx_sqlexpr.ml, parsetree.mli and ast_mapper.mli, and think I got the gist of how the AST mapping is done (keep in mind I haven't dabbled in ppx yet). So as a sign of atonement I can try to do it if you want :)

[j0sh]

Hmm, seems that github only detects as merged when merging with the master/base branch.

WIP branch here for hosting shared expressions: https://github.com/mfp/ocaml-sqlexpr/pull/14

[mfp]

On Wed, Mar 02, 2016 at 10:40:53PM -0800, Josh Allmann wrote:

Closed #10.

Thanks for all your great work, j0sh.

Mauricio Fernández