Switch to ppx

[mfp]

camlp4 is on the way out, ppx is becoming increasingly popular, and using both at once can be problematic (camlp4 doesn't recognize newer ppx syntax)...

What should the syntax look like, though? Does something like this make sense (I haven't worked with ppx yet...):

1. [%sql {s| select @s{login}, @s{password} FROM users WHERE id = %s|s}], which would be expanded the same way as sql"..." with the current camlp4 extension (also [%sqlc ...] and [%sqlinit ...] forms, plus sql_check"...")
2. recognize sql"...", sqlc"...", sqlinit"..." and sqlcheck"..." forms and expand (providing source compatibility), maybe (de)activated with switch

(2)'s appeal is that, as far as ocaml-sqlexpr is concerned, the same source code could be compiled with both the ppx and the camlp4 extensions (so the user would be able to choose based on the other extensions in use). In fact, both sqlexpr.ppx and sqlexpr.syntax could be maintained in the same tree and installed at once.

[c-cube]

Why not directly use {sql| select @s{login} ... |sql}, {sqlc| .... |sqlc}, etc.? It should be relatively straightforward to write a ppx that transforms that into proper queries, using Ast_mapper. The major advantage is that it combines well with other ppx.

[c-cube]

Regarding internal use of camlp4, I've started a branch in which lwt.syntax is replaced with lwt.ppx. It should be tested for regressions, as some lwt.syntax idioms are not totally isomorphic to lwt.ppx.

[mfp]

On Sun, Jul 19, 2015 at 10:32:50AM -0700, Simon Cruanes wrote:

Why not directly use {sql| select @s{login} ... |sql}, {sqlc| .... |sqlc}, etc.? It should be relatively straightforward to write a ppx that transforms that into proper queries, using Ast_mapper. The major advantage is that it combines well with other ppx.

Does ppx have access to the "x" in {x| ... |x} ? If so, sure, why not, it's shorter than [%sql {s| select ... |s}}.

sqlc will be a bit trickier since it's built on estring's (register_shared_expr) ability to hoist the definitions, so that

let foo x y =
  ... sqlc"xxxx" ...
  ...

becomes

let foo =
  let __estring_shared_0 = .... (* sqlc"xxx" expansion *) in
    fun x y -> ...

As for the backward-compatible stuff, does it seem doable? It could come in (at least) two shapes:

• sql"select @s{foo} from bar where id=%d" n

  which as far as the compiler is concerned looks like a string and an integer being applied to the "sql" function

• xxx sql"yyy" zzz

  where the "sql value" is applied to the preceding function

(doh' at issue title typo)

Mauricio Fernández

[mfp]

On Sun, Jul 19, 2015 at 10:34:34AM -0700, Simon Cruanes wrote:

Regarding internal use of camlp4, I've started a branch in which lwt.syntax is replaced with lwt.ppx. It should be tested for regressions, as some lwt.syntax idioms are not totally isomorphic to lwt.ppx.

I was going to write that ocaml-sqlexpr should be desugared altogether, but on second thought it doesn't really matter: internal use of camlp4 does not prevent the user from building client code with ppx.

In fact, keeping the lib as is (i.e., using camlp4) gives a small compatibility edge for the time being (since the ppx syntax extension would be built only if the compiler is recent enough).

Mauricio Fernández

[Drup]

If so, sure, why not, it's shorter than [%sql {s| select ... |s}}.

Several people made the good point that 'x' in {x| should be reserved to avoid escaping, and that a ppx should use [%sql{| ... |}] (which is equally short, if quite ugly).

• sql"select @s{foo} from bar where id=%d" n

Yes

• xxx sql"yyy" zzz

Not without a lot of pain and corner cases.

[c-cube]

in many cases, then, [%sql "select foo from bar"] would work (without needing special strings). I think that's probably convenient enough. I also agree with @Drup that xxx sql"foo" yyy would be too complicated (and ugly!) to handle. It's probably not that difficult to use sed with something like s/sql\"([^"])*\"/[%sql {|\1|}]/g. If janestreet uses this library they probably can use their camlp4 rewriter anyway :)

[j0sh]

Here is a patch set adding ppx support, comments welcome: https://github.com/mfp/ocaml-sqlexpr/pull/6

The patch set uses the [%sql "..."] notation.

[c-cube]

Looks like this could be closed, since #6 was merged?