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.

belongs_to with :primary_key fails to preload

#4384

Using Rails 2.3.5

with something like this:

class Foo < ActiveRecord::Base
  belongs_to :bar, :foreign_key => 'some_id', :primary_key => 'guid'
end

create_table "foos", :force => true do |t|
  t.string   "some_id"
end

create_table "bars", :force => true do |t|
  t.string   "guid",         :limit => 36,                  :null => false
end

These queries are fine:

>> Foo.find(567158)
  Foo Load (0.1ms)   SELECT * FROM foos WHERE (foos.id = 567158)
=> #<Foo id: 567158, some_id: "9917de13-99a5-45b9-a032-45d8c0ac4a81">
>> Foo.find(567158).bar
  Bar Load (0.4ms)   SELECT * FROM bars WHERE (bars.guid = '9917de13-99a5-45b9-a032-45d8c0ac4a81') LIMIT 1
=> #<Bar id: 3823066, guid: "9917de13-99a5-45b9-a032-45d8c0ac4a81">

But this query fails:

>> Foo.find(567158, :include => bar)
  Foo Load (0.2ms)   SELECT * FROM foos WHERE (foos.id = 567158)
  Bar Load (0.1ms)   SELECT * FROM bars WHERE (bars.id = 9917)
NoMethodError: undefined method each' for nil:NilClass


    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/association_preload.rb:154:in `set_association_single_records'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/association_preload.rb:147:in `each'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/association_preload.rb:147:in `set_association_single_records'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/association_preload.rb:344:in `preload_belongs_to_association'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/association_preload.rb:318:in `each'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/association_preload.rb:318:in `preload_belongs_to_association'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/association_preload.rb:120:in `send'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/association_preload.rb:120:in `preload_one_association'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activesupport-2.3.5/lib/active_support/ordered_hash.rb:97:in `each'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activesupport-2.3.5/lib/active_support/ordered_hash.rb:97:in `each'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/association_preload.rb:114:in `preload_one_association'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/association_preload.rb:91:in `preload_associations'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/association_preload.rb:90:in `preload_associations'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/association_preload.rb:90:in `each'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/association_preload.rb:90:in `preload_associations'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/base.rb:1550:in `find_every'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/base.rb:1583:in `find_one'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/base.rb:1569:in `find_from_ids'
    from /usr/local/ruby/lib/ruby/gems/1.8/gems/activerecord-2.3.5/lib/active_record/base.rb:616:in `find'
    from (irb):2</code>

Notice that it's looking up by id=9917, which is the wrong column and the wrong value. You do get 9917 if you run '9917de13-99a5-45b9-a032-45d8c0ac4a81'.to_i, so the error here seems obvious.

I believe the bug is in preload_belongs_to_association where it uses "primary_key" which in this case is "id", instead of reflection.options[:primary_key], which would be 'guid'.

This patch seems to work for me, but as I'm not familiar with the code I would not call it complete: http://pastie.org/917089

Edit: boy, the markup language on this ticket system is bad.

Reported by Greg Hazel · April 13th, 2010 @ 08:46 AM

State: resolved
Milestone: none
Assigned to: Ryan Bigg Ryan Bigg
Importance: Low

Activity

  1. David Trasbo
    David Trasbo
    • Assigned user set to Ryan Bigg

    Rails is not supposed to support foreign key columns of other types than integer. This ticket can be marked as invalid.

    April 15th, 2010 @ 08:09 PM

  2. Ryan Bigg
    Ryan Bigg
    • State changed from new to wontfix

    I agree, this seems like an edge-case better suited to a plugin.

    April 15th, 2010 @ 10:57 PM

  3. Greg Hazel
    Greg Hazel

    What? This would also be broken if the key were an integer.

    April 16th, 2010 @ 12:31 AM

  4. Ryan Bigg
    Ryan Bigg
    • State changed from wontfix to open

    Leaving it open for discussion. OP has indicated he does not want to submit a patch.

    April 16th, 2010 @ 12:47 AM

  5. Greg Hazel
    Greg Hazel
    • Title changed from belongs_to with non-integer :foreign_key fails to preload to belongs_to with :primary_key fails to preload

    April 16th, 2010 @ 10:53 AM

  6. Greg Hazel
    Greg Hazel

    April 16th, 2010 @ 11:13 AM

  7. Greg Hazel
    Greg Hazel

    And, guys, if "Rails is not supposed to support foreign key columns of other types than integer", why are they used in the ActiveRecord unit tests themselves?

    class Reply < Topic
  ...
  belongs_to :topic_with_primary_key, :class_name => "Topic", :primary_key => "title", :foreign_key => "parent_title", :counter_cache => "replies_count"
  ...
end

...

create_table :topics, :force => true do |t|
  t.string   :title
  ...
  t.string   :parent_title
  ...
end

    April 16th, 2010 @ 11:17 AM

  8. Jeff Kreeftmeijer
    Jeff Kreeftmeijer

    I did a little test on 2.3.5 with the Foo and Bar models you described (I only changed the some_id and guid fields to be integers). I'm not getting any NilClass errors, it seems to work perfectly:

    >> Foo.find(2)
=> #<Foo id: 2, some_id: 123, created_at: "2010-04-16 10:18:32", updated_at: "2010-04-16 10:18:32">
>> Foo.find(2).bar
=> #<Bar id: 1, guid: 123, created_at: "2010-04-16 10:19:58", updated_at: "2010-04-16 10:19:58">
>> Foo.find(2, :include => :bar)
=> #<Foo id: 2, some_id: 123, created_at: "2010-04-16 10:18:32", updated_at: "2010-04-16 10:18:32">

    April 16th, 2010 @ 11:26 AM

  9. Greg Hazel
    Greg Hazel

    Jeff: Is id the primary_key for Bar? Can you show the sql queries it is submitting? According to line 324 and 335 of associtation_preload.rb in Rails 2.3.5 it should be submitting:

    primary_key = klass.primary_key

    "#{table_name}.#{connection.quote_column_name(primary_key)} #{in_or_equals_for_ids(ids)}"

    which in your case would be:

    Bar.id = 123

    So, I'm not sure why you can't reproduce it exactly, but the bug certainly existed for integer columns, and was fixed by http://github.com/rails/rails/commit/63026541b209cc11ffd74cf3ca04b8...

    April 16th, 2010 @ 11:36 AM

  10. Jeff Kreeftmeijer
    Jeff Kreeftmeijer

    Foo:

    class Foo < ActiveRecord::Base
  belongs_to :bar, :foreign_key => 'some_id', :primary_key => 'guid'
end

    FoosController:

    class FoosController < ApplicationController
  def index
    @foo = Foo.find(1, :include => :bar)
  end
end

    foos/index.html.erb:

    <%= @foo.bar.to_yaml %>

    queries:

    Processing FoosController#index (for 127.0.0.1 at 2010-04-16 12:50:15) [GET]
  Foo Load (0.2ms)   SELECT * FROM "foos" WHERE ("foos"."id" = 1) 
  Bar Load (0.2ms)   SELECT * FROM "bars" WHERE ("bars"."id" = 123) 
Rendering foos/index
  Bar Load (0.3ms)   SELECT * FROM "bars" WHERE ("bars"."guid" = 123) LIMIT 1
Completed in 16ms (View: 4, DB: 1) | 200 OK [http://localhost/foos]

    So I think you're right. It's not properly preloading in 2.3.5 when using integers if you ask me...

    April 16th, 2010 @ 11:58 AM

  11. Jeff Kreeftmeijer
    Jeff Kreeftmeijer

    Damn, missed the link. Like Greg said: fixed by http://github.com/rails/rails/commit/63026541b209cc11ffd74cf3ca04b8....

    April 16th, 2010 @ 12:14 PM

  12. Ryan Bigg
    Ryan Bigg
    • State changed from open to resolved

    April 19th, 2010 @ 10:14 PM

  13. Jamison Wilde
    Jamison Wilde
    • Importance changed from to Low

    There is a additional bug exposed with this fix, in that if you use intern'd column names as your primary key, the comparison then fails.

    :primary_key => :my_column fails while :primary_key => 'my_column' succeeds.

    The patch is to use to_s:
    options[:primary_key].to_s

    Obviously the workaround is to not use intern'd column names.

    Putting this here since this is the first place people will find in a search engine presumably.

    November 12th, 2010 @ 10:40 PM

  14. Jamison Wilde
    Jamison Wilde

    Also, some more background:
    This fails silently in Ruby 1.8.

    In Ruby 1.9 it fails loudly with:

    NoMethodError: undefined method type' for nil:NilClass<br/>

    from /home/slain/.rvm/gems/ruby-1.9.2-p0/gems/activesupport-2.3.8/lib/active_support/whiny_nil.rb:52:in `method_missing'

    Since the columns.detect fails and Ruby 1.9 no longer supports type as a method on NilClass.

    November 12th, 2010 @ 11:09 PM