This project is archived and is in readonly mode.
Eager loading uniq habtm association is broken
Reported by Niels Meersschaert | August 27th, 2009 @ 08:26 PM | in 3.x
Loading a model thru eager loading or simple loading should return same results for associations. That is not the case with habtm with uniq option.
I have a model Foo with a habtm relationship to Bar that is
uniq.
class Foo < ActiveRecord::Base
has_and_belongs_to_many :bars, :uniq => true end
class Bar < ActiveRecord::Base
end
Let's build 2 models, one of each type:
foo = Foo.create(:name => "foo_test") #ID 13
bar = Bar.create(:name => "bar_test") #ID 87
I add the same instance of Bar to a Foo multiple times:
5.times do
foo.bars << bar
end
If I do a clean load of foo & then ask for bars, I get 1 as expected:
foo2 = Foo.find(13)
foo2.bars
[#]
However, if I eager load the bars:
foo3 = Foo.find(13, :include => :bars)
foo3.bars
[#,#,#,#,#]
The returned array isn't uniq, but shows all 5 instances of the duplicate bar.
Comments and changes to this ticket
-
CancelProfileIsBroken September 25th, 2009 @ 12:39 PM
- Tag changed from 2.3.3, activerecord to 2.3.3, activerecord, bugmash
-
Elad Meidar September 26th, 2009 @ 04:38 AM
+1 verified and reproduced on master and 2-3-stable. i've attached a failing test.
-
hsume2 (Henry) September 26th, 2009 @ 10:07 AM
-1 changing the
developers.yml
somehow introduced failures in other tests.I rewrote it using
dev_3
instead. I've attached the edited failing test. -
hsume2 (Henry) September 26th, 2009 @ 10:38 AM
Also, this is my take on patching the issue. I thought of adding something like (-190,6 +190,7):
+ associated_records.uniq! if options[:uniq] set_association_collection_records(id_to_record_map, reflection.name, associated_records, 'the_parent_record_id')
to
#preload_has_and_belongs_to_many_association
, but I think this is a more general issue. Instead, it seems that when#load_target
is called by#count_records
, the target is already loaded. I've attached a patch. (Applies to master and 2-3-stable).I'm also unsure if this is the optimal way (alternative,
options[:select] = 'DISTINCT *'
). -
hsume2 (Henry) September 28th, 2009 @ 10:25 AM
- Tag changed from 2.3.3, activerecord, bugmash to 2.3.3, activerecord, bugmash, bugmash-review
-
Rizwan Reza February 12th, 2010 @ 12:46 PM
- Tag changed from 2.3.3, activerecord, bugmash, bugmash-review to 2.3.3, activerecord, bugmash-review
-
Rizwan Reza May 15th, 2010 @ 06:45 PM
- Tag changed from 2.3.3, activerecord, bugmash-review to 2.3.3, activerecord, bugmash
-
Subba July 17th, 2010 @ 02:14 PM
- Importance changed from to
Henry can you update your patch it seems your patch link is broken.
-
hsume2 (Henry) July 17th, 2010 @ 07:29 PM
I've attached reworked patches for master and 2-3-stable. The implementation is slightly different than previously specified. Instead of
associated_records.uniq! if options[:uniq]
I've gone the
DISTINCT
method, because that seems to be the behavior forhas_many :through
with:uniq => true
and custom:select
(seeconstruct_select
inthrough_association_scope.rb:48
). -
Neeraj Singh July 18th, 2010 @ 02:27 PM
- Milestone set to 3.x
@Henry looks like you were not in the rails root directory while creating the patch. I tried applying rails edge patch and it failed.
Could you please create that patch again. thanks.
-
Neeraj Singh July 18th, 2010 @ 08:41 PM
not sure why but I am still getting the error while applying your patch
(master)$ git am < patch.txt Applying: Fix a bug where eager loading of a :uniq => true has_and_belongs_to_many returns duplicate entries error: activerecord/lib/active_record/association_preload.rb: does not match index error: activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb: does not match index Patch failed at 0001 Fix a bug where eager loading of a :uniq => true has_and_belongs_to_many returns duplicate entries When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort".
-
hsume2 (Henry) July 18th, 2010 @ 09:03 PM
@Neeraj, I'm not sure why either. I've applied the patch multiple times today at different commits, most recently on [387036609268c4cf68401a1333715f2ee4aecffc], and it's worked.
It looks like you've had a few patches accepted recently, did you reset the branch and pull from master before you applied my patch? Or do you have any unstaged changes on
association_preload.rb
orhas_and_belongs_to_many_associations_test.rb
? -
Neeraj Singh July 18th, 2010 @ 09:15 PM
I checked out brand new rails app before applying the patch. Well someone else will verify your patch.
-
Santiago Pastorino February 2nd, 2011 @ 04:35 PM
- State changed from new to open
- Tag changed from 2.3.3, activerecord, bugmash to 233, activerecord, bugmash
This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.
Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.
-
Santiago Pastorino February 2nd, 2011 @ 04:35 PM
- State changed from open to stale
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>