baby_squeel icon indicating copy to clipboard operation
baby_squeel copied to clipboard

Join by adding conditions instead of replacing them

Open pelletencate opened this issue 7 years ago • 10 comments

Feature request: In an ORM I used to use in my former life, there was an alternative to .on for joins that was called .with. Basically, what it did was take the original join conditions, and add another condition to it using AND. I found this to be extremely helpful, as part of the reasons why you use an ORM is that you don't have to bother with join conditions, and most of the time when you want to override them, it's because you want to restrict them by adding something to the original conditions.

Basically, it works like this:

# Implicitly follows schema
A.joining { b }
# SELECT * FROM a INNER JOIN b ON b.a_id = a.id

# Explicitly, it'd look like this:
A.joining { b.on(b.a_id == id) }
# SELECT * FROM a INNER JOIN b ON b.a_id = a.id

# Now, adding a condition
A.joining { b.on((b.a_id == id) & (b.active == true)) }
# SELECT * FROM a INNER JOIN b ON b.a_id = a.id AND b.active == true

# My suggestion:
A.joining { b.with(b.active == true) }
# SELECT * FROM a INNER JOIN b ON b.a_id = a.id AND b.active == true

Especially if your models are named MerchantIdentityOperatorExtension instead of A, this can save you a lot of code that would be the same in > 90% of the usages of .on

pelletencate avatar May 12 '17 16:05 pelletencate

This is definitely an interesting idea. Though, I think it might be really tricky to implement.

The problem is that BabySqueel lets Active Record do the heavy lifting in the case of an implicit join. In the case of an explicit join, it's just Arel.

Combining the two would be very difficult and more likely to break as Active Record changes. However, I think your idea could be implemented more easily if it worked like this:

A.joining { b.with(b.active == true) }
# SELECT * FROM a INNER JOIN b ON b.a_id = a.id WHERE b.active = true

rzane avatar May 12 '17 18:05 rzane

This won't give the same result in an outer join.

octavpo avatar May 23 '17 07:05 octavpo

True, that won't work for outer joins.

Can't you convert a query to Arel after AR has done the heavy lifting, and then simply append an AND part to it?

pelletencate avatar Jul 14 '17 12:07 pelletencate

Any chance to have this implemented? It would be nice not to have to repeat the implicit join clause.

octavpo avatar Sep 11 '17 15:09 octavpo

I'd say pretty unlikely. BabySqueel isn't generating those join nodes: Active Record is (with a lot of help from polyamorous). I think even if we got this working, it would be brittle, and I'd like to avoid that.

That being said, if someone came up with a clever solution here, I'd be very interested in seeing it. It is a really cool feature request and I'd love to have it in baby_squeel.

rzane avatar Sep 11 '17 15:09 rzane

Wouldn't @pelletencate's suggestion work? So for cases where the joined table is an association, you'd call AR to generate an AR relation, then apply an .arel and add the on condition. For other cases just do arel directly as usual. I don't even think it needs a new construct, it's hard to believe anybody would want to join an association and not want the default condition added automatically.

octavpo avatar Sep 14 '17 16:09 octavpo

Yes, I agree, but I think this is one of those things that is easier said than done. More than happy to accept a PR.

rzane avatar Sep 14 '17 18:09 rzane

I'm not quite a wizard with Arel, but if I have some spare time within the next few weeks (and that's unfortunately a big if), I'll give it a try.

pelletencate avatar Oct 10 '17 13:10 pelletencate

@rzane I haven't had time to look into this yet. But I do realize, we may be able to steal some code from ActiveRecord, they must do something similar in their implementation of .where; that is, if you call .where multiple times on a single relation, it somehow hangs all of those together with AND. The exact same logic should be used using the .with. I haven't looked in the source code yet, but I just realized that where it's most of the time a convenience, it can become a necessity if you are working with default scopes that need to be applied in place to joined models.

pelletencate avatar Nov 29 '17 11:11 pelletencate

Unless I'm missing something I don't see how that helps. For all query helpers AR just stores all user options in instance variables until the time it needs to generate the query, when it combines them appropriately. The issue here is that unlike in the case of where, we have these on conditions that AR doesn't know how to handle AFAIK. And on the other hand Arel doesn't know how to generate the implicit join conditions. So you still need to somehow send the 'naked' joined models to AR first to make it generate the implicit conditions, and then get the generated Arel and add the on conditions, as you were suggesting in a previous comment. Or the only alternative I can see would be to mimic the AR code in baby_squeel and make it generate the implicit Arel conditions, which I think goes beyond what @rzane wants to do in this package.

octavpo avatar Nov 29 '17 17:11 octavpo