Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

PATCH: primary_key option for belongs_to

#765

Could some one add :primary_key option into belongs_to in a similar fashion like ticket 292? This option is a nice-to-have for has_many, because you can always use finder_sql to workaround it. But it is a must-to-have for belongs_to because there is no way to wrok around it. I had a quick look at the source code and added

def owner_quoted_id

if @reflection.options:primary_key]

quote_value(@owner.send @reflection.options:primary_key]))

else

@owner.quoted_id

end

end

into belongs_to.rb.

it didn't work

Reported by Shawn · August 6th, 2008 @ 04:40 PM

State: committed
Milestone: 2.3.6
Assigned to: Jeremy Kemper Jeremy Kemper
Importance: Medium

Activity

  1. Philip Hallstrom
    Philip Hallstrom
    • 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.

    August 12th, 2008 @ 07:12 PM

  2. Philip Hallstrom
    Philip Hallstrom

    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.

    August 12th, 2008 @ 07:26 PM

  3. Philip Hallstrom
    Philip Hallstrom
    • 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

    August 15th, 2008 @ 06:21 AM

  4. Pratik
    Pratik
    • Title changed from PATCH: primary_key option for belongs_to to primary_key option for belongs_to

    August 16th, 2008 @ 08:32 PM

  5. Philip Hallstrom
    Philip Hallstrom

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

    August 18th, 2008 @ 09:52 PM

  6. Philip Hallstrom
    Philip Hallstrom

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

    August 19th, 2008 @ 05:57 AM

  7. Philip Hallstrom
    Philip Hallstrom

    Updated patch to fix association build/create.

    August 19th, 2008 @ 06:38 AM

  8. Frederick Cheung
    Frederick Cheung

    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

    August 21st, 2008 @ 02:20 AM

  9. Jeremy Kemper
    Jeremy Kemper
    • State changed from new to incomplete
    • Milestone cleared.

    August 28th, 2008 @ 07:33 AM

  10. logan
    logan

    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?

    September 3rd, 2008 @ 09:00 PM

  11. Zyclops
    Zyclops

    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.

    October 16th, 2008 @ 01:32 AM

  12. Jeremy Kemper
    Jeremy Kemper

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

    October 16th, 2008 @ 03:02 AM

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

    October 17th, 2008 @ 05:18 PM

  14. noelrocha (at gmail)
    noelrocha (at gmail)

    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.

    January 21st, 2009 @ 06:47 PM

  15. noelrocha (at gmail)
    noelrocha (at gmail)

    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

    January 22nd, 2009 @ 07:19 AM

  16. Jacek Becela
    Jacek Becela

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

    May 26th, 2009 @ 12:43 PM

  17. Jeremy Kemper
    Jeremy Kemper

    Good feature; still incomplete.

    June 15th, 2009 @ 03:12 AM

  18. Szymon Nowak
    Szymon Nowak
    • 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 Custom counter_cache is not used when using collection.size method) 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?

    July 15th, 2009 @ 09:53 PM

  19. Pratik
    Pratik
    • State changed from incomplete to open

    July 15th, 2009 @ 09:54 PM

  20. Repository
    Repository
    • State changed from open to committed

    July 16th, 2009 @ 02:17 AM

  21. Repository
  22. Joey A
    Joey A
    • 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.

    August 27th, 2009 @ 09:15 PM

  23. Jeremy Kemper
    Jeremy Kemper

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

    August 27th, 2009 @ 09:58 PM

  24. Ernie Miller
    Ernie Miller

    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. :)

    December 18th, 2009 @ 06:16 PM

  25. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.x to 2.3.6
    • State changed from committed to open

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

    December 18th, 2009 @ 09:26 PM

  26. Andrew Selder
    Andrew Selder

    +1

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

    February 2nd, 2010 @ 10:45 PM

  27. Jeremy Kemper
    Jeremy Kemper
    • State changed from open to incomplete

    Ernie's doesn't, Andrew.

    February 2nd, 2010 @ 11:29 PM

  28. Andrew Selder
    Andrew Selder

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

    February 2nd, 2010 @ 11:31 PM

  29. Ernie Miller
    Ernie Miller

    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.

    February 11th, 2010 @ 05:36 PM

  30. Ernie Miller
    Ernie Miller

    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.

    March 26th, 2010 @ 05:58 PM

  31. Jeremy Kemper
    Jeremy Kemper
    • State changed from incomplete to open

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

    March 27th, 2010 @ 05:45 PM

  32. Ernie Miller
    Ernie Miller

    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.

    March 29th, 2010 @ 07:00 PM

  33. Jeremy Kemper
    Jeremy Kemper

    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.

    March 29th, 2010 @ 07:50 PM

  34. Ernie Miller
    Ernie Miller

    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. :)

    March 29th, 2010 @ 08:44 PM

  35. Ernie Miller
    Ernie Miller

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

    March 29th, 2010 @ 09:43 PM

  36. Repository
    Repository
    • State changed from open to committed

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

    [#765 PATCH: primary_key option for belongs_to state:committed]

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

    March 29th, 2010 @ 11:34 PM

  37. Repository
    Repository

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

    [#765 PATCH: primary_key option for belongs_to ]

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

    March 29th, 2010 @ 11:34 PM

  38. Ryan Bigg
    Ryan Bigg
    • Tag cleared.

    Automatic cleanup of spam.

    October 9th, 2010 @ 09:49 PM

  39. Jeff Kreeftmeijer