This project is archived and is in readonly mode.
[PATCH] Non standard ID column on has_many :through
Reported by James Pearson | August 3rd, 2009 @ 10:27 PM
Usingthe example code below:
class User < ActiveRecord::Base
has_many :followers has_many :suspects, :through => :followers
end
class Connection < ActiveRecord::Base
belongs_to :user belongs_to :suspect, :primary_key => :case_id ,
:foreign_key => :case_id end
class Suspect < ActiveRecord::Base
belongs_to :connection, :primary_key => :case_id , :foreign_key
=> :case_id end
The problem is that the belongs_to seems to ignore the :primary key.
If I do
u = User.find(:first)
u.suspects
The SQL generated is:
SELECT suspects
.* FROM suspects
INNER
JOIN connections
ON suspects
.id =
connections
.case_id WHERE
((followers
.user_id = 1))
However it should be:
SELECT suspects
.* FROM suspects
INNER
JOIN connections
ON suspects
.case_id =
connections
.case_id WHERE
((followers
.user_id = 1))
This was partially fixed in 2.3.3 but not for has_many :through.
The patch applied to 2.3.3 can be found here: http://github.com/rails/rails/commit/b3ec7b2d03a52e43a4451d522eea7e...
Comments and changes to this ticket
-
José Valim August 8th, 2009 @ 02:43 PM
- Tag changed from hasmany, primarykey to bugmash, hasmany, primarykey
-
dira August 9th, 2009 @ 05:29 PM
Attached a new patch with tests & implementation. Please ignore the previous patch.
I'm not sure whether the fix is in the right place or it should be a dedicated method somewhere.
-
dira August 9th, 2009 @ 05:30 PM
- Tag changed from bugmash, hasmany, primarykey to bugmash, hasmany, patch, primarykey
-
Rajesh August 9th, 2009 @ 06:19 PM
- Tag changed from bugmash, hasmany, patch, primarykey to bugmash, hasmany, primarykey
verified
the patch has a failing test case for this case -
José Valim August 9th, 2009 @ 07:34 PM
Dira, do you think we can change your patch to use already existing models?
-
Arthur Zapparoli August 9th, 2009 @ 08:05 PM
+1 for the idea, -1 for the patch
Dira: I agree with José Valim, and think this patch needs to be applied using the existing AR models on the test suite.
Also, I can't see any assertion in your test. And why you added: has_many :posts_by_title, :through => :authorship , :source => :post to Person model?
-
dira August 9th, 2009 @ 10:49 PM
@José: Thanks for the feedback; I started on a new patch with the existing models. Also I realized that the original patch was broken (foreign_key & primary_key interchanged). Will submit a new patch tomorrow morning.
-
dira August 10th, 2009 @ 09:26 AM
- Tag changed from bugmash, hasmany, primarykey to bugmash, hasmany, patch, primarykey
Here is the correct patch - with tests and implementation.
Thank you all for the feedback.
-
dira August 10th, 2009 @ 09:27 AM
Here is the correct patch - with tests and implementation.
Thank you all for the feedback.
-
Szymon Nowak August 13th, 2009 @ 08:20 PM
Here are patches for 2-3-stable and master branches, based on dira's last patch. They also fix methods like collection.build/create/delete/clear/<< etc.
-
Szymon Nowak August 15th, 2009 @ 04:43 PM
- Assigned user set to Jeremy Kemper
- Title changed from None standard ID column on has_many :through to [PATCH] Non standard ID column on has_many :through
-
dira September 27th, 2009 @ 11:37 AM
the master patch applies cleanly
the 2.3.stable patch applies cleanly
-
Prem Sichanugrist (sikachu) January 25th, 2010 @ 05:20 PM
- Tag changed from bugmash, hasmany, patch, primarykey to bugmash, hasmany, patch, primarykey, review
- State changed from new to open
-
Rizwan Reza February 12th, 2010 @ 12:46 PM
- Tag changed from bugmash, hasmany, patch, primarykey, review to hasmany, patch, primarykey, review
-
Prem Sichanugrist (sikachu) September 10th, 2010 @ 02:59 PM
- Assigned user changed from Jeremy Kemper to José Valim
- Importance changed from to Medium
Seems like it has been about a year since there's a patch in this ticket!
I've confirmed that patch from Szymon still applies on
2-3-stable
, and I've update the patch to apply cleanly onmaster
. Can someone please apply this?Thank you
-
Prem Sichanugrist (sikachu) September 11th, 2010 @ 09:35 AM
- Assigned user changed from José Valim to Santiago Pastorino
Santiago, could you please apply this patch for me?
Thank you
-
Jon Leighton December 19th, 2010 @ 04:10 PM
- Assigned user changed from Santiago Pastorino to Aaron Patterson
- Tag changed from hasmany, patch, primarykey, review to hasmany, patch, primarykey, review, verified
I have updated Szymon Nowak's patch to current master, and verified that it works correctly.
Note that two of the tests (test_associate_existing_with_nonstandard_primary_key_on_belongs_to and test_collection_delete_with_nonstandard_primary_key_on_belongs_to) no longer fail in current master due to fixes elsewhere, but I thought it was worth leaving them in for assurance in any case.
Also note that I have added the method
AssociationReflection#association_primary_key
. This method is also part of my fix to #2801 and also features in my patch for nested :through associations [#1152], so I'd say it's a good one to have.Reassigning to Aaron as he is current the Active Record person.
-
Repository December 23rd, 2010 @ 11:20 PM
- State changed from open to resolved
(from [85683f2a79dbf81130361cb6426786cf6b0d1925]) Fix creation of has_many through records with custom primary_key option on belongs_to [#2990 state:resolved] https://github.com/rails/rails/commit/85683f2a79dbf81130361cb642678...
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
Tags
Referenced by
- 2990 [PATCH] Non standard ID column on has_many :through (from [85683f2a79dbf81130361cb6426786cf6b0d1925]) Fix cre...