tomjaguarpaw / haskell-opaleye

Other
598 stars 115 forks source link

Function Request: Inner Join #475

Open jaredramirez opened 3 years ago

jaredramirez commented 3 years ago

I think it would be helpful to have a function to make inner joins more clear for people getting started with Opaleye, since it's a concept directly from relational DBs.

We can easily join on a table like so:

row <- Opaleye.selectTable table -< ()
Opaleye.restrict -< ...
-- use `row`

If we packaged this up into a a function, we could more succinctly do:

row <- innerJoin table -< (\row -> ...)
-- use `row`

This might seems trivial, but as I've been onboarding folks on our team at work, I think little things to make Opaleye more familiar (most folks are comfortable with raw SQL) could go a long way. Additionally, it could make a consistent API with things like https://github.com/tomjaguarpaw/haskell-opaleye/issues/446, whenever that happens.

I think this could be implemented pretty easily, and I'm happy to submit a PR if you think this would be something good to add:

innerJoin ::
  Default Unpackspec fields fields =>
  Opaleye.Table a fields ->
  Opaleye.SelectArr
    ( fields ->
      Opaleye.Field Opaleye.SqlBool
    )
    fields
innerJoin table =
  proc predicate -> do
    purchaseOrder <- Opaleye.selectTable table -< ()
    Opaleye.restrict -< predicate purchaseOrder
    returnA -< purchaseOrder
tomjaguarpaw commented 3 years ago

This is an interesting idea. I've mostly approached Opaleye from the point of view of "how do we make relational algebra more palletable to programmers familiar with Haskell". You're pointing out that there is dual problem to solve of "how do we make Haskell more palletable to programmers familiar with relational algebra".

tomjaguarpaw commented 3 years ago

I am in the process of changing the "recommended" way of using joins in Opaleye. I have recently learned that it's possible to improve the interface to left joins (including inferrability) by using an Opaleye-level analogue of Maybe (which I call MaybeFields) and by using lateral joins. That work is on the lateral branch. You may like to have a look.

Adding your innerJoin could be included as part of that. The only thing holding me back from adding it is a desire not to clutter the API. I would have about 10x the impetus to add it if you can tell me "we've added it to our codebase and we have had success using it for several months", and 100x if someone else can say "we've had the same experience in a separate organisation".

jaredramirez commented 3 years ago

Thanks for the feedback! MaybeFields is something I've been looking forward too since starting to use Opaleye, I'm super exited to see that land.

I hear your concern on cluttering the API. If optionalRestrict is widely adopted/liked, it could make sense to have that replace leftJoin then add something like innerJoin to have a streamlined join module. But I digress, I only recently added it to our codebase, so I'll come back to this issue report how we're feeling after several months of usage.