This project is archived and is in readonly mode.
[PATCH] A bug in ActiveRecord::Associations#using_limitable_reflections
Reported by oleg dashevskii | June 16th, 2010 @ 05:40 AM | in 2.3.9
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.
Comments and changes to this ticket
-
oleg dashevskii June 17th, 2010 @ 05:07 AM
- Title changed from A bug in ActiveRecord::Associations#using_limitable_reflections to [PATCH] A bug in ActiveRecord::Associations#using_limitable_reflections
-
Ken Collins July 7th, 2010 @ 03:20 PM
Confirmed 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 July 7th, 2010 @ 08:52 PM
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.
-
Ken Collins July 7th, 2010 @ 08:52 PM
- no changes were found...
-
oleg dashevskii July 8th, 2010 @ 08:30 AM
Actually, it's http://github.com/rails/rails/commit/c48a71c7#L0L1786 where bug has been introduced.
-
Eloy Duran July 8th, 2010 @ 08:36 AM
- Assigned user changed from Eloy Duran to José Valim
- Importance changed from to Low
-
Repository July 8th, 2010 @ 10:03 PM
- 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 July 11th, 2010 @ 09:37 AM
- 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 July 11th, 2010 @ 09:47 AM
- 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. :)
-
José Valim July 15th, 2010 @ 07:58 PM
Ken, it was applied in both branches. See repository comment above. :)
-
Rohit Arondekar October 7th, 2010 @ 05:21 AM
- Tag changed from activerecord associations, 2.3-stable, bug, patch to 2.3-stable, activerecord, associations, bug, patch
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Referenced by
- 4869 [PATCH] A bug in ActiveRecord::Associations#using_limitable_reflections (from [0e9bc23c0e5dba228626ffbc2bef069331b2e471]) Fix the...
- 5124 [PATCH] ActiveRecord count fails when run with multiple includes and a join Is this related to #4869?
- 5124 [PATCH] ActiveRecord count fails when run with multiple includes and a join Just curious. When I found the bug in #4869 it was under ...