rzane/baby_squeel

Join by adding conditions instead of replacing them

Opened this issue · 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

rzane commented

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

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

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?

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

rzane commented

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.

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.

rzane commented

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.

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.

@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.

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.