search

kiranandcode / petrol

Petrol's an OCaml SQL API made to go FAST.
https://kiranandcode.github.io/petrol/petrol/index.html
Other
113 stars 6 forks source link

feat: add Query.table for joins directly on a table #11

Open tjdevries opened 1 year ago

tjdevries commented 1 year ago

Hi!

First off, really enjoying petrol. Thanks for the library.

I was wondering if this would be a welcome addition to the library. I'm not necessarily 100% sold on using TABLE as a new variant, but I wasn't sure how else to handle this easily (and also allows inserting tables into other locations in queries later, if needed).

Usage looks now like this:

    Query.select ~from:table Expr.(User.f_twitch_display_name :: fields)
    |> Query.join
         ~op:Query.INNER
         ~on:Expr.(User.f_twitch_user_id = f_user_id)
         (Query.table User.table)

Printed:

SELECT users.twitch_display_name, suggestions.id, ...
FROM suggestions
   INNER JOIN users ON users.twitch_user_id = suggestions.user_id

Thanks

kiranandcode commented 1 year ago

Ohh, nice this looks interesting! Thanks so much for your PR. This change makes a lot of sense... I had noticed a lot of repetition when doing joins with tables where I was using all the columns.

My only hesitation is that the last time I was working on the code, I remember making a quick hack to make sub-queries work on postgres involving automatically giving names to sub-queries (https://github.com/Gopiandcode/petrol/blob/e83d3c833b98a3a915949651316a04469417d7f5/lib/types.ml#L276), which I think is the cause of some issues discussed in #3.

I was thinking of making some kind of large change to allow representing table aliases (JOIN ... AS B) explicitly in the DSL. It may be the case that this kind of change may subsume this one.

tjdevries commented 1 year ago

I was thinking of making some kind of large change to allow representing table aliases (JOIN ... AS B) explicitly in the DSL. It may be the case that this kind of change may subsume this one.

No problem :) it was a fun PR. I don't know that the subquery problem will exist because there isn't a subquery when using the table this way (it's just a join on a table, so the resulting query should work fine in postgres or not, at least that is my understanding).

That's why I have this part:

https://github.com/Gopiandcode/petrol/pull/11/files#diff-c6395e65757951e5358bc2a6170b82ae6bf1ef444258b3918fb58a5fa5394245R280-R290

I think it should avoid the problem.

Either way, thanks for the response :)