This project is archived and is in readonly mode.
[PATCH] A bug in ActiveRecord::Associations#using_limitable_reflections
After 2.3.5 to 2.3.8 transition I found unexpected breakage in my code:
OrderItem.sum(:quantity, {
:joins => [:menu_item, :order],
:group => 'menu_items.dish_id',
:order => 'sum_quantity DESC', :limit => 10,
:conditions => { 'orders.user_id' => user.id }}.merge(options))
It tried to issue second SELECT query and substitute some ids,
which is quite a nonsense here.
I investigated the problem and finally found that the breakage has
been introduced in c48a71c7. Enumerable#reject was refactored into
Enumerable#select, but it should be #collect.
The patch fixes the problem. I used #none? which is neater.
-
oleg dashevskii- Title changed from A bug in ActiveRecord::Associations#using_limitable_reflections to [PATCH] A bug in ActiveRecord::Associations#using_limitable_reflections
-
Ken CollinsConfirmed here too. This killed my pagination after an upgrade. Ticket #3520 < with a commit http://github.com/rails/rails/commit/ff508640e28914da2b546f6a8c9f21... introduced the bug. I will see if I can add a test to 2-3-stable and submit a tested patch.
-
Ken Collins
OK, I did some research and found the issue. This commit http://github.com/rails/rails/commit/51e6124e6a5fcb370201d49b042d33... was the source of the bug. Looks like an innocent refactor issue. This bug affects both 2-3-stable as well as master since that helper was moved to help Arel finders methods.
So I have created two patches with tests. The first is 2-3-stable_FixLimitableReflections.diff and is for the 2-3-stable branch and the second is master_FixLimitableReflections.diff, for the current master.
Both patches include what I call white box testing by testing the helper vs testing how the helper is used. Feedback welcome.
-
-
oleg dashevskii
Actually, it's http://github.com/rails/rails/commit/c48a71c7#L0L1786 where bug has been introduced.
-
Eloy Duran- Assigned user changed from Eloy Duran to José Valim
- Importance changed from to Low
-
Repository- State changed from new to resolved
(from [0e9bc23c0e5dba228626ffbc2bef069331b2e471]) Fix the #using_limitable_reflections? helper to work correctly by not examining the length of an array which contains false/true, hence always passing. [#4869 state:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/0e9bc23c0e5dba228626ffbc2bef06... -
Michael Koziarski- Milestone set to 2.3.9
- State changed from resolved to open
Wasn't this also meant to be applied to 2-3-stable?
-
José Valim- State changed from open to resolved
Hey Koz! I just checked and I applied it:
http://github.com/rails/rails/commit/504f7cfbb3d6fab0f2bacaad48e030...
My bad, I forgot to make the commit message point to this ticket. :)
-
Ken Collins
What about the 3 branch? This bug is in there too?
-
José Valim
Ken, it was applied in both branches. See repository comment above. :)
-
Rohit Arondekar- Tag changed from activerecord associations, 2.3-stable, bug, patch to 2.3-stable, activerecord, associations, bug, patch