Rails 3.2.0 + mysql2 0.3.11 + Model.select("some joined col.").includes = FAIL #4564

Closed
NoICE opened this Issue Jan 20, 2012 · 19 comments

Projects

None yet

8 participants

@NoICE
NoICE commented Jan 20, 2012

Don't know if this is issue with mysql2 gem or ActiveRelation (feel free to redirect me), but using .includes on has_many association causes ARel to ignore call to .select("...").

class User::User < AR........
  # ...
  has_many :user_share, :class_name => 'User::RokShare', :foreign_key => :rs_user
  # ...

end

# later on...
      share = User::User.select('u_name, u_surname, rs_amount').where('rs_rok = ?', self.u_id).joins(:user_share).includes(:user_share).all
      unless share.nil? || share.empty?
        at['rok_share'] = share.collect { |s| [s.name, s.rs_amount] }
      end

gives:

  SQL (0.2ms)  SELECT `user_tbl_user`.`u_id` AS t0_r0, `user_tbl_user`.`u_number` AS t0_r1, `user_tbl_user`.`u_name` AS t0_r2, `user_tbl_user`.`u_surname` AS t0_r3, `user_tbl_user`.`u_password` AS t0_r4, `user_tbl_user`.`u_password_readonly` AS t0_r5, `user_tbl_user`.`u_brand` AS t0_r6, `user_tbl_user`.`u_active_from` AS t0_r7, `user_tbl_user`.`u_active_to` AS t0_r8, `user_tbl_user`.`u_street` AS t0_r9, `user_tbl_user`.`u_zip_code` AS t0_r10, `user_tbl_user`.`u_town` AS t0_r11, `user_tbl_user`.`u_phone` AS t0_r12, `user_tbl_user`.`u_mobile` AS t0_r13, `user_tbl_user`.`u_email` AS t0_r14, `user_tbl_user`.`u_birthday` AS t0_r15, `user_tbl_user`.`u_coop` AS t0_r16, `user_tbl_user`.`u_coop_birthday` AS t0_r17, `user_tbl_user`.`u_photo` AS t0_r18, `user_tbl_user`.`u_recruitment` AS t0_r19, `user_tbl_user`.`u_sponsor` AS t0_r20, `user_tbl_user`.`u_reference` AS t0_r21, `user_tbl_user`.`u_level` AS t0_r22, `user_tbl_user`.`u_f_manager` AS t0_r23, `user_tbl_user`.`u_f_ror` AS t0_r24, `user_tbl_user`.`u_f_rok` AS t0_r25, `user_tbl_user`.`u_in_rewards_list` AS t0_r26, `user_tbl_user`.`u_parent_rtm` AS t0_r27, `user_tbl_user`.`created_at` AS t0_r28, `user_tbl_user`.`updated_at` AS t0_r29, `user_tbl_user`.`u_login` AS t0_r30, `user_tbl_user`.`u_dismiss_argument` AS t0_r31, `user_tbl_user`.`u_active_from_calendar` AS t0_r32, `user_tbl_user`.`u_locale` AS t0_r33, `user_tbl_user`.`u_shown_name` AS t0_r34, `user_tbl_rok_share`.`rs_id` AS t1_r0, `user_tbl_rok_share`.`rs_rok` AS t1_r1, `user_tbl_rok_share`.`rs_user` AS t1_r2, `user_tbl_rok_share`.`rs_amount` AS t1_r3, `user_tbl_rok_share`.`created_at` AS t1_r4, `user_tbl_rok_share`.`updated_at` AS t1_r5, `user_tbl_rok_share`.`rs_min` AS t1_r6, `user_tbl_rok_share`.`rs_max` AS t1_r7 FROM `user_tbl_user` INNER JOIN `user_tbl_rok_share` ON `user_tbl_rok_share`.`rs_user` = `user_tbl_user`.`u_id` WHERE (rs_rok = 28151)
Completed 500 Internal Server Error in 21ms

NoMethodError (undefined method `rs_amount' for #<User::User:0x007f9be26463a8>):

Why?
Why it ignores my own call to .select ? I've tried to pass an array to .select to see if I'm just dumb.. but evidently it behaves the same way... just crashes.

If using select with includes is a bad idea, because of how eager loading works, it should raise an exception right away and not mangle the SQL query this way...

The same call but without the includes part (i.e. share = User::User.select('u_name, u_surname, rs_amount').where('rs_rok = ?', self.u_id).joins(:user_share).all) produces this SQL:

User::User Load (0.5ms)  SELECT u_name, u_surname, rs_amount FROM `user_tbl_user` INNER JOIN `user_tbl_rok_share` ON `user_tbl_rok_share`.`rs_user` = `user_tbl_user`.`u_id` WHERE (rs_rok = 28151)
Contributor

Maybe the Rails 4.0 changelog might be relevant here: https://github.com/rails/rails/blob/master/activerecord/CHANGELOG.md

Also I already saw a similar ticket a while ago but I can't find it at the moment.

NoICE commented Jan 21, 2012

I think I hit the same issue with rails 3.1.1 and mysql 0.3.6.. so maybe it is not new.

I like the idea of includes inlining into the single parent select by saving-the-queries means, but it also hurts performance. A lot. Another query which I optimized and ran in a few miliseconds is transformed to database killing beast which now (since the upgrade from 3.0.11) it takes 1,5 SECONDS just because of the another non-optimized automatically-inserted join...

I thought before that includes does inner join only for belongs_to associations (i.e. 1:1) and performs separate SQL for has_many... which would be "The Right Thing" even for MySQL Cluster (which hates joins) and for classic MySQL which then can optimize the two separate queries better (heck, better than rails in this case for sure).

Without any thrown warnings now I have to go through this whole (now somewhat ready for rewrite) application and check for all those cases... This is of course my own fault of ignoring lack of the tests in this application. (It was fast and dirty rewrite from PHP which I regret since. I should've take much more time to proper rewrite :/ ).

NoICE commented Mar 12, 2012

bump?

Contributor

Is this still an issue? Is this similar to #6132?

Contributor
dgm commented May 18, 2012

I finally tracked down a regression bug to this problem. I had something sort of like:
Person.select('people.*, households.id as household_id').joins(:households).includes(:households).where(.....)

It worked well in 3.0, but after upgrading to 3.2, it bombs. I've simplified the query some but the idea was that I needed to do a group_by on the household_id, among other things.

NoICE commented May 22, 2012

@isaacsanders I rewrote that arel call so it doesn't use includes (as I said it also hurt performance), so I can't really test it right now. But it seems very related to #6132 so feel free to close this as a duplicate.

Contributor
dgm commented May 23, 2012

It's similar but I'm not sure if it is the same - they are talking about altering to_sql to see the query after includes, but the problem in this ticket is that the includes call smashes the select call. The fact that includes changes the query late in the game obvious comes as a surprise to both to_sql and select, but is altering just one going to fix the other?

(I had to rewrite my query to avoid this too, but I think my performance went down as a result)

Contributor
parndt commented Nov 25, 2012

Does the patch mentioned in #5990 fix this issue? Agreed that this seems very related to #6132

Is the answer that this simply won't work until Rails 4.0?

Owner
senny commented Mar 7, 2013

Is this still an issue on 3.2.x ? or on master?

NoICE commented Mar 7, 2013

Sorry, can't test it anymore, that app is being rewritten.
You could test it by just writing something like the example at the top.
Feel free to close this as a duplicate of #6132.

Owner
senny commented Mar 7, 2013

Is anyone else experiencing this issue? Does someone have a small script, example app or a test-case to reproduce?

Contributor
dgm commented Mar 7, 2013

sure. clone https://github.com/dgm/arel_bug and run rake test

Owner
senny commented Apr 2, 2013

I can confirm this problem. I don't think it's related to arel though. It's the way AR deconstructs the Result in the JoinDependency to instantiate the records. I don't have much insight into this part of AR so it's hard to tell wether this is expected or buggy behavior.

@jonleighton could you elaborate?

@NoICE NoICE added the stale label Apr 23, 2014
Contributor

I created this gist (based on @dgm's test case) reproducing this behavior, but I think it does make sense to select not work with includes. If you are already selecting some columns, why do you need to eager load?

Contributor
dgm commented Apr 24, 2014

@laurocaetano I think it has more to do with the fact that includes can be used to make a left outer join instead of the inner join used by joins. Perhaps this explains the impedance mismatch... If AR had an option like outer_joins() I may never have tripped on it.

Owner

@dgm agree... making #12071 potentially very relevant.

IMO, includes should be treated as a clever black box, and not used as a general source of "left join plz" behaviour: I'm pretty sure it has other behavioural peculiarities beyond this.

NoICE commented Apr 26, 2014

@laurocaetano because you may want to select columns which were eager loaded, and I think that's the reason most use .includes instead of .preload, but that's not the point. Point is the behavior is inconsistent between Rails versions and this change is not mentioned anywhere, since it is a bug. If it should not be used this way, it should raise some exception... at least...? Then it would be OK with me..

@laurocaetano laurocaetano added needs feedback and removed stale labels Apr 26, 2014
Contributor

@matthewd @dgm I totally agree with your argument!
@NoICE thanks, now I see that I was wrong about it.

I'm 👍 for #12071 too.

Owner

I'm going to close this, in favour of the in-progress #12071 as a future means of achieving the result that might bring you to this problem... and I think we'll declare the ship to have sailed on the behavioural regression: reverting to the 3.0 behaviour at this point would be more likely to harm people upgrading from 3.1+.

@matthewd matthewd closed this May 15, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment