search

brianhempel / active_record_union

UNIONs in ActiveRecord! Adds proper union and union_all methods to ActiveRecord::Relation.
Other
423 stars 41 forks source link

Eager-loading and support for includes, joins and references #15

Open oyeanuj opened 7 years ago

oyeanuj commented 7 years ago

Hi @brianhempel @glebm! I am coming from the Readme where you asked folks to make a noise around supporting 'eager-loading'. Is that still on the radar? In my case, almost all scopes have eager-loading and includes, joins and references, and I'd love to clean up my code using this!

FWIW, testing it so far, I keep getting the following error.

ArgumentError: Cannot union relation with includes.
    from /Users/anuj/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/active_record_union-1.2.0/lib/active_record_union/active_record/relation/union.rb:53:in `verify_relations_for_set_operation!'
    from /Users/anuj/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/active_record_union-1.2.0/lib/active_record_union/active_record/relation/union.rb:27:in `set_operation'
    from /Users/anuj/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/active_record_union-1.2.0/lib/active_record_union/active_record/relation/union.rb:11:in `union'

Thank you for putting out this library!

brianhempel commented 7 years ago

Is making a change "on the radar"? Considering you're the first to ask for it, probably not yet, though third-party PRs are always welcome of course. But:

  1. Do you need the eager loading in the places where you would like to use union?
  2. Can you move the eager loading after the union?

In the other thread you mentioned your scopes have eager loading. Without seeing your application, my intuition is that placing includes etc. in a scope is premature. In any case you should be able to write:

relation_1.union(relation_2).includes(...)

I haven't done Rails work in a while so I don't remember specifically, but I think the above should work.

oyeanuj commented 7 years ago

@brianhempel Thank you for the quick response!

I'm following your suggestion in places where I can, where simple relations are being used. However, unfortunately, in my code, a lot of the associations come via multiple different models (scopes were just one example), and hence moving includes away means changing a lot of code at a lot of places.

I understand that it might not be worth the effort based on a single use-case, but hope it starts the discussion.