Rails Ruby on Rails → 2.3.6

Comments and changes to this ticket

• You flagged this item as spam.
  

  Philip Hallstrom August 12th, 2008 @ 07:12 PM

  • Tag set to “patch”

  I've written up a patch that seems to work. I may not be thinking it all the way through, but maybe it will get things closer to where they need to be. I've done limited testing (no unit tests that is), but it is working for me.

  Also, 'rake test_sqlite3' in the activerecord directory still passes all tests.

  
  class Occupation < ActiveRecord::Base
  
    has_many :aliases, :class_name => 'OccupationAlias', :primary_key => :onet_soc_code, :foreign_key => :onet_soc_code
  
  end
  

  
  class OccupationAlias < ActiveRecord::Base
  
    belongs_to :occupation, :primary_key => :onet_soc_code, :foreign_key => :onet_soc_code
  
  end
  

  
  $ ./script/runner "o = Occupation.find(:first); puts o.id; puts OccupationAlias.find(o.aliases.first.id).occupation.id"
  3797
  3797
  

  The SQL run is:

  
    Occupation Load (0.002571)   SELECT * FROM "occupations" LIMIT 1
    OccupationAlias Load (0.034346)   SELECT * FROM "occupation_aliases" WHERE ("occupation_aliases".onet_soc_code = E'11-1011.00') ORDER BY title LIMIT 1
    OccupationAlias Load (0.000405)   SELECT * FROM "occupation_aliases" WHERE ("occupation_aliases"."id" = 102190)
    Occupation Load (0.000505)   SELECT * FROM "occupations" WHERE ("occupations"."onet_soc_code" = E'11-1011.00') LIMIT 1
  

  Patch is attached.

• You flagged this item as spam.
  

  Philip Hallstrom August 12th, 2008 @ 07:26 PM

  I dug into the unit tests and believe I've added a correct unit test to test this functionality.

  
  $ rake test_sqlite3 TEST=test/cases/associations/belongs_to_associations_test.rb
  (in /Users/philip/Work/Careers.org/web/vendor/rails/activerecord)
  /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/bin/ruby -Ilib:test:test/connections/native_sqlite3 "/Library/Ruby/Gems/1.8/gems/rake-0.8.1/lib/rake/rake_test_loader.rb" "test/cases/associations/belongs_to_associations_test.rb"
  Using native SQLite3
  Loaded suite /Library/Ruby/Gems/1.8/gems/rake-0.8.1/lib/rake/rake_test_loader
  Started
  ....................................
  Finished in 0.472048 seconds.
  
  36 tests, 123 assertions, 0 failures, 0 errors
  

  Original patch with unit tests is attached.

• You flagged this item as spam.
  

  Philip Hallstrom August 15th, 2008 @ 06:21 AM

  • Title changed from “:primary_key option for belongs_to ” to “PATCH: primary_key option for belongs_to ”
  • Tag changed from “patch” to “activerecord, belongs_to, patch, tests”
• 

  Pratik August 16th, 2008 @ 08:32 PM

  • Title changed from “PATCH: primary_key option for belongs_to ” to “primary_key option for belongs_to ”

• You flagged this item as spam.
  

  Philip Hallstrom August 18th, 2008 @ 09:52 PM

  Same patch with tests in format correct git/rails format for submission.

• You flagged this item as spam.
  

  Philip Hallstrom August 19th, 2008 @ 05:57 AM

  This patch doesn't cover creation (State.cities.build(...)) correctly.

• You flagged this item as spam.
  

  Philip Hallstrom August 19th, 2008 @ 06:38 AM

  Updated patch to fix association build/create.

  • primary_key_for_belongs_to.diff 5.5 KB
• 

  Frederick Cheung August 21st, 2008 @ 02:20 AM

  A few comments:

  *

  
  owner_id = @reflection.options.has_key?(:primary_key) ? @owner.send(@reflection.options[:primary_key]) : @owner.id
  

  might be a bit neater as

  
  owner_id = @owner.send(@reflection.options[:primary_key] || :id)
  

  • You'll need to handle eager & preloading
  • the find_by bit in find_target looks a bit grungy
• 

  Jeremy Kemper August 28th, 2008 @ 07:33 AM

  • State changed from “new” to “incomplete”
  • Milestone cleared.

• You flagged this item as spam.
  

  logan September 3rd, 2008 @ 09:00 PM

  Just wanted to add some additional motivation for this patch. I'm using uuids in a multi-machine, multi-database configuration with ActiveResource. Since objects are moved between databases on each machine, they can't use the autoincrementing 'id' column, hence uuids. Changing the database primary key to be a uuid has a negative impact on Innodb performance so this patch is the right solution.

  Is there a workaround or patch available for has_and_belongs_to_many?

• You flagged this item as spam.
  

  Zyclops October 16th, 2008 @ 01:32 AM

  This is a must have feature for us. We are using the finder_sql for the has_many relationship but need this to get the belongs_to to work.

  We currently need to regularly import data from a legacy system and combine them with additional records from the system. Using a separate key would solve the issue.

• 

  Jeremy Kemper October 16th, 2008 @ 03:02 AM

  Complete the patch and the feature will go into Rails. It's that easy :)

• 

  Pratik October 17th, 2008 @ 05:18 PM

  • Assigned user changed from “Jeremy Kemper” to “Pratik”
  • Milestone set to “2.x”

• You flagged this item as spam.
  

  noelrocha (at gmail) January 21st, 2009 @ 06:47 PM

  I've applied this patch by hand and it doesn't work in 2.2.

  The find_target and set_belongs_to_association_for methods aren't even called.

  I've overridden the modules and put the file at rails initializers folder.

  • activerecord_overrides.rb 1.6 KB

• You flagged this item as spam.
  

  noelrocha (at gmail) January 22nd, 2009 @ 07:19 AM

  The original patch only works for things like that: Occupation.find(:first).aliases.create(:name => 'alias name')

  But doesn't works for: al = Alias.new(:name => 'alias name') al.occupation = Occupation.find(:first)

  Follow attached all my overrides in ActiveRecord that worked. I've put together previous patch and changes at: ActiveRecord::Associations::ClassMethods.belongs_to ActiveRecord::Associations::BelongsToAssociation.replace

  I know, I should at least make a diff, but it is 5am here in Brazil, I'm tired.

  Thanks

  • activerecord_overrides.rb 5.2 KB

• You flagged this item as spam.
  

  Jacek Becela May 26th, 2009 @ 12:43 PM

  I could use this feature right now. Will definitely +1 a good patch.

• 

  Jeremy Kemper June 15th, 2009 @ 03:12 AM

  Good feature; still incomplete.

• You flagged this item as spam.
  

  Szymon Nowak July 15th, 2009 @ 09:53 PM

  • Tag changed from “activerecord, belongs_to, patch, tests” to “activerecord, belongs_to, patch, primary_key, tests”
  • Title changed from “primary_key option for belongs_to ” to “PATCH: primary_key option for belongs_to ”

  Here's hopefully complete patch. I tested assignments, counter cache (which resulted in new ticket #2907) and polymorphic associations.
  There are actually 2 patches - one for master branch and one for 2-3-stable.

  BTW. In belongs_to_association_test.rb file there are 2 almost identical tests named "test_belongs_to_counter_after_save" and 2 "test_belongs_to_counter_after_update_attributes". Should I create a patch that removes duplicates?

  • belongs_to_with_primary_key.diff 22.5 KB
  • belongs_to_with_primary_key_2-3-stable.diff 22.7 KB
• 

  Pratik July 15th, 2009 @ 09:54 PM

  • State changed from “incomplete” to “open”
• 

  Repository July 16th, 2009 @ 02:17 AM

  • State changed from “open” to “committed”

  (from [b3ec7b2d03a52e43a4451d522eea7e6499289daa]) Add primary_key option to belongs_to association

  [#765 state:committed]

  Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
  http://github.com/rails/rails/commit/b3ec7b2d03a52e43a4451d522eea7e...

• 

  Repository July 16th, 2009 @ 02:17 AM

  (from [1c11437a32a973fa9b521c32caa7256f9772acd7]) Add primary_key option to belongs_to association

  [#765 state:committed]

  Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
  http://github.com/rails/rails/commit/1c11437a32a973fa9b521c32caa725...

• You flagged this item as spam.
  

  Joey A August 27th, 2009 @ 09:15 PM

  • Assigned user changed from “Pratik” to “Jeremy Kemper”

  I believe the :primary_key option to belongs_to was not being applied when using :include to eagerly load the associated objects. I've attached a test demonstrating the bug and a patch (to 2-3-stable). I'd appreciate any feedback about how this works, particularly with regard to polymorphism, since I'm not really exercising those features.

  • fix_eager_loading_of_belongs_to_associations_2-3-stable.diff 2.1 KB
• 

  Jeremy Kemper August 27th, 2009 @ 09:58 PM

  Joey, there's another ticket open for this, but I can't find it for the life of me.

• You flagged this item as spam.
  

  Ernie Miller December 18th, 2009 @ 06:16 PM

  This seems as useful a place to put the following patch as well. Looks like association.rb wassn't accounting for :primary_key option in association_join.

  This seems to fix the issue for me.

  I'd include a test but I'm leaving to head to my wedding in about 2 hours and need to finish up some work. :)

  • associations.patch 630 Bytes
• 

  Jeremy Kemper December 18th, 2009 @ 09:26 PM

  • Milestone changed from “2.x” to “2.3.6”
  • State changed from “committed” to “open”

  Nice catch, Ernie. Congratulations! Test forthcoming, or honeymoon? :)

• You flagged this item as spam.
  

  Andrew Selder February 2nd, 2010 @ 10:45 PM

  +1

  It looks like the patch has a test there contrary to the comment.

• 

  Jeremy Kemper February 2nd, 2010 @ 11:29 PM

  • State changed from “open” to “incomplete”

  Ernie's doesn't, Andrew.

• You flagged this item as spam.
  

  Andrew Selder February 2nd, 2010 @ 11:31 PM

  Ahhh... good point.. I'll see if I can bang out a test tonight and get this thing finished.

• You flagged this item as spam.
  

  Ernie Miller February 11th, 2010 @ 05:36 PM

  I have been a bit busy (insert jokes here) but just came back to see my patch was still missing tests. Here you go. Also, I think the existing test_belongs_to_with_primary_key may not be testing what it's supposed to be testing -- but to be safe I just created a new test.

  • belongs_to_honoring_primary_key.diff 2 KB

• You flagged this item as spam.
  

  Ernie Miller March 26th, 2010 @ 05:58 PM

  Not to be a nuisance, as I'm sure that everyone (like me) is far too busy working on Rails 3 related stuff at the moment, but was wondering if anyone had taken a look at the updated patch with tests.

• 

  Jeremy Kemper March 27th, 2010 @ 05:45 PM

  • State changed from “incomplete” to “open”

  Is this patch against 2-3-stable? Could you combine with Joey's change and rebase against master?

• You flagged this item as spam.
  

  Ernie Miller March 29th, 2010 @ 07:00 PM

  Jeremy, yes, it was against 2-3-stable. I combined both mine and Joey's against the current 2-3-stable. I'm assuming that's what you meant since a rebase against master would be for Rails 3. :) Let me know if this does the trick.

  • combined_primary_key_fixes.diff 4.1 KB
• 

  Jeremy Kemper March 29th, 2010 @ 07:50 PM

  Thanks! Yes, for Rails 3. All new development happens on master first, then we may backport to 2-3-stable. Adding a feature to 2-3-stable that isn't on master would be broken.

• You flagged this item as spam.
  

  Ernie Miller March 29th, 2010 @ 08:44 PM

  Oh. My assumption (perhaps incorrectly) was that this wasn't broken in Rails 3 with the Arel-based overhaul. Maybe that's wrong. Let me go back and rebase against master and see how horribly broken the patch is. :)

• You flagged this item as spam.
  

  Ernie Miller March 29th, 2010 @ 09:43 PM

  Here we go. I was wrong, these were broken in Rails 3 too. Here's a working patch.

  • honor_primary_key_with_belongs_to.diff 3.5 KB
• 

  Repository March 29th, 2010 @ 11:34 PM

  • State changed from “open” to “committed”

  (from [63026541b209cc11ffd74cf3ca04b89d1e437737]) Fix honoring :primary_key option when joining or eager loading a belongs_to association

  [#765 state:committed]

  Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
  http://github.com/rails/rails/commit/63026541b209cc11ffd74cf3ca04b8...

• 

  Repository March 29th, 2010 @ 11:34 PM

  (from [715b34fdff1e1e93204e57737d69f8d365e17504]) use supplied primary key when eager-loading belongs_to associations rather than default primary key

  [#765]

  Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
  http://github.com/rails/rails/commit/715b34fdff1e1e93204e57737d69f8...

• 

  Ryan Bigg October 9th, 2010 @ 09:49 PM

  • Tag cleared.

  Automatic cleanup of spam.

• 

  Jeff Kreeftmeijer November 7th, 2010 @ 04:51 PM

  Automatic cleanup of spam.