ppx syntax extensions

[tizoc]

Is this planned or being worked on?

[darioteixeira]

AFAIK, there's no one actively working on this. It would be of course welcome, but it's low priority for now.

[foretspaisibles]

Just a gentle ping to get an update on this? Is there anybody known to be working on such extensions? I thought I once read about it for a few weeks but I cannot find the reference again.

[darioteixeira]

I don't know of anybody working on such extensions. It's on my TODO list, but it has a fairly low priority since Camlp4 is still supported.

[dra27]

A nice side-effect of switching is that presumably we'd get Merlin support "for free", as far as I can tell?

[eras]

Hi,

I ported the syntax extension to ppx. The tree is available at: https://github.com/eras/pgocaml/tree/ppx . I did it to learn ppx, so there might be some issues left :).

I think there might be issues with the "execute" feature, plus I'm not all sure the _oasis is properly configured for ppx, but all in all it seems pretty complete.. It is still work in progress, though.

The syntax is: [%pgsql dbh "sql here"].

Bonus: it indeed seems to work fine with Merlin!

[darioteixeira]

@eras: Awesome! I'm a bit short on time to play with this right now, but I'll gladly merge any PR once you're satisfied with it...

There is one extra feature which might be worth considering for a PPX extension: for many SQL statements (most other than SELECT, in fact) we don't really care about the return value. I though it would be useful to have a variant of %pgsql which discarded the result and returned type unit monad. Something like %pgsql_exec?

[foretspaisibles]

@eras Thank you for doing this! :)

[Tormen]

Hi.
As people keep telling me : USE PPX, campl4 is dying ... I am wondering @darioteixeira : Are there any plans in officially swtiching in the short / middle term to ppx ? (the next year)
Tormen

[darioteixeira]

Yes, it's something I'd like to do sooner rather than later. There's just a fair bit on my plate right now...

[tizoc]

I have been playing with @eras syntax extension using ppx ( I haven't used it for anything serious yet, just toy programs to see if it works), and except for some merlin issues related to "execute" queries, it seems to work fine.

Description of the issue:

Lets say I have an "execute" pgsql call, where I create some temporary tables, so that they are visible to the queries validator.

After that, I add a bunch of queries (using the %pgsql syntax), but one of those queries contains an error. All the queries that appear in the file before the one containing an error will now not see the results of the "execute" call (so, if you created some tables, it is like if you had never created them). I haven't tried to understand the code yet but my guess here is that when an error is raised the connection gets reset, and the temporary tables go away (and the order in which queries are validated is from bottom to top).

This can be reproduced easily if you open thetests_ppx/test_ppx.ml file with merlin enabled (once you have installed pgocaml pinned to the ppx repo) and with this in the .merlin file:

PKG pgocaml
PKG pgocaml.ppx
PKG ounit

This issue may also happen in the camlp4 version, but since it doesn't work with merlin I don't really know. What I know is that the translation seems quite direct when looking at the diffs so this behavior is probably shared by the two implementations of the syntax extension.

There is one extra feature which might be worth considering for a PPX extension: for many SQL statements (most other than SELECT, in fact) we don't really care about the return value. I though it would be useful to have a variant of %pgsql which discarded the result and returned type unit monad. Something like %pgsql_exec?

Doesn't that happen automatically already based on what the query itself returns? I get unit PGOCaml.monad on inserts for example, but if I add returning * then I get a tuple.

[tizoc]

Ok, I found what causes this. The ppx extension removes the connection if an exception is raisen (the camlp4 doesn't)

https://github.com/eras/pgocaml/blob/ppx/ppx/ppx_pgsql.ml#L435-L438

I don't know why doing so is necessary in the ppx extension (maybe it isn't).

Anyway, I modified my local version to stop doing that and now it works as expected.

Edit: here is the reason eras@b51021a

@eras so far I'm not having this problem, do you remember when did it happen to you? I would like to know how to reproduce it.

[tizoc]

Ok, think I know why it doesn't happen to me. My local copy is in sync with master from the main repo, and I see that one of the changes that was introduced after @eras wrote the ppx extension is that now the connection is resynchronized after an error occurs (which didn't happen before, leaving it in a hung status).

If anyone wants to give it a try just pin this repo and reinstall pgocaml:

opam pin add pgocaml -k git 'https://github.com/tizoc/pgocaml#ppx'

Your .merlin file needs to include a PKG pgocaml.ppx line for the ppx extension to be enabled.

[tizoc]

I have been using it more in the last days (from https://github.com/tizoc/pgocaml#ppx-ocaml403 since I upgraded to OCaml 4.03 and the ppx extension had to be updated, that branch also includes a few other changes like handling of enums as if they were strings).

Something I think can be improved is for the syntax to not take the database handle parameter, and instead just produce a one argument function, that takes the database handle, and then runs the query at that moment. Something like this:

(* query : ('a PGOCaml.t -> 'b PGOCaml.monad) -> 'b PGOCaml.monad *)
let query = {pgsql| SELECT name, age FROM people |pgsql} in
let%lwt dbh = connect_to_db () in
let%lwt rows = query dbh in
(* ... *)

This kind of syntax doesn't handle the extra flags that the current syntax support (more importantly, "execute" and "nullable-results", not sure what the best way to do that would be).

Anyway, I haven't experimented with that yet, just sharing my thoughts for now.

[eras]

Hi,

Sorry for not being more active about this project. It wouldn't require that much to finish it I think.. Also thanks for the detective work on how the connection syncing was introduced.

But one thing holding me back is that there could indeed be a nicer syntax for the ppx. The one you suggest seem infact nicer than any of the ones I thought of! Though as you mention, there should still be a way to handle those additional options. I had some ideas but have since forgotten them :).

One thing I was hoping would be a way to embed strings in such a way that they could be constructed by other PPX modules, allowing constructing queries with macros from fragments, even if truly building queries runtime from fragments is not possible. For example user-controllable sorting would basically require something like that. But I suppose this is not a priority, it's enough to get this out of the door so people can rejoice the cool PPX functionality with Merlin ;-).

[j0sh]

allowing constructing queries with macros from fragments, even if truly building queries runtime from fragments is not possible

There was a similar request posed for sqlexpr recently, here's a good answer: mfp/ocaml-sqlexpr#18 (comment)

Nice job on the ppx conversion, @eras. Would be nice if some of the WIP commits could be squashed together for easier review?

[tizoc]

@j0sh you can see the changes all combined in one diff here master...eras:ppx

But what you probably want to do is diff the files implementing the ppx and camlp4 extensions (because @eras didn't really modify the existing code, he wrote a new file containing a port from camlp4 to ppx).

[tizoc]

For anyone interested, I made a new ppx extension that I have been using lately and has worked pretty well so far. I haven't uploaded it anywhere yet but will probably do so soon.

It looks like this:

  let update =
    {sql|
      UPDATE accounts
         SET email = $email, account_type = $account_type
       WHERE account_id = $account_id
   RETURNING account_id, created_at, account_type, email
    |sql}

it returns a labelled function (with one label per argument name, and optional argument labels for parameters marked as optional with ?) that takes a database connection handle as its last argument and then runs the query, the type of the above code looks like this:

email:string ->
account_type:string ->
account_id:int64 ->
(string, bool) Hashtbl.t Pg_store_helpers.PGOCaml.t ->
(int64 * CalendarLib.Calendar.t * string * string) list
PGOCaml.monad

Like the camlp4 extension it connects to the database to figure out the types of columns and validity of queries.

It doesn't support the execution of queries at compile time, and there there is no way to configure the connection, I may add that later.

[zbroyar]

@eras thanks for ppx. Good job!
Now my Emacs with merlin-mode checks my SQL statements while saving a file and I'm pretty excited about it :-)

[zbroyar]

@eras, @tizoc, I have a question for both of you :-)

Would you mind to merge your ppx-related work to the main pgocaml branch? Actually, I use your code in production over pretty big datasets for a while and didn't meet any issues.

[tizoc]

@zbroyar thats up to @darioteixeira. I haven't been using @eras' ppx extension myself, but my own instead. It is different, and doesn't contain the same features, I just did the minimium required for my use cases. I have been using it for months already without issues.

I just pushed it to github, you can find it here: https://github.com/tizoc/ppx_pgsql

[charlesetc]

These forks look like great starts but it's difficult to use them without them being on opam. Is there a plan to merge either into the current pgocaml repository or publish them separately to opam?

The problem I'm encountering specifically is helping a project that uses pgocaml to be able to use reason syntax as well, which doesn't support camlp4.

[tizoc]

@charlesetc you can pin ppx_pgsql and it will be installable through opam:

opam pin add ppx_pgsql -k git https://github.com/tizoc/ppx_pgsql.git

[darioteixeira]

I can merge the PPX into PG'OCaml if the general feeling is that it works and is stable...

[j0sh]

For what it's worth, here's a branch based off @eras's nice ppx work, compared against pgocaml master here. https://github.com/darioteixeira/pgocaml/compare/master...j0sh:ppx?expand=1

Haven't done a lot of testing myself yet other than running the preexisting tests.

More context here: eras#1

This branch squashes the WIP ppx commits, moves everything on top of pgocaml master, makes pgocaml buildable from a fresh install without camlp4, and makes the ppx tests runnable. Tested to work with OCaml 4.03 up to 4.05, so added opam version constraints for those.

[darioteixeira]

There's more than one branch in discussion here. I'll merge one which a) supports the killer-feature of the existing Camlp4 extension (compile-time type-checking against the DB), b) has users, c) applies cleanly against master.

@tizoc: I personally don't use compile-time execution of queries (in contrast with compile-time type-checking), but I reckon some users may rely on that feature and will be upset if it's dropped...

@eras and @j0sh: I'll merge whichever branch you guys agree on for merging. Hash it out between you and tell me which one I should review...

[tizoc]

@tizoc: I personally don't use compile-time execution of queries (in contrast with compile-time type-checking), but I reckon some users may rely on that feature and will be upset if it's dropped...

My ppx extension is also a bit different from the original one (in a way that I like more, thats why I wrote it), so even if it supported compile-time execution (something that can be added), it is probably not the syntax extension current users of the camlp4 extension would expect. I have been using it for a good amount of time already without problems (even since before I pushed it to Github), but it is just an alternative, I didn't meant for it to be merged back into pgocaml.

[dra27]

If I can add a little bit of noise on this PR just to say that I most definitely rely on the execute and nullable-results flags. I also agree that while @tizoc's ppx does look very nice, it is important to have a ppx which is a fairly close match to the camlp4 version just to allow a reasonably mechanical conversion for existing projects.

[charlesetc]

I'd understand completely if something like @tizoc's branch can't be merged - is there a way to pin a dependency like ppx_pgsql to a git repository in an OPAM file?

[chrismamo1]

I've made a couple of minor changes on top of @j0sh 's work, with the most relevant being some use documentation in the readme and a slight refactoring of how PG'OCaml handles connections to Postgres (rather, to how PG'OCaml handles information about potential connections) in order to provide more informative error messages on connection failures.

[jrochel]

I have just merged @chrismamo1's PR #56 (and #57) which implements full PPX support. It has been tested reasonably well and will be continued to be tested (as it is used in a fairly large project). After some more testing I will soon release a version with PPX support on opam.