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.

Using Model.includes(:relation_with_custom_primary_key)

#5605

Hi

It seems that when specifying a custom primary key in a belongs_to
belongs_to :model, :primary_key => :some_key
and then calling

Model.includes(:other_model).all

will throw a exception.
This is still a working theory but I created a sample app to demonstrate.
I tried adding a test in rails/activerecord/cases/associations_test.rb

def test_eager_loading_with_custom_primary_key
    assert_nothing_raised{Firm.includes(:account).all} 
end

But for some reason this passes when I run the test.

Which led me to create a example app where i tried

belongs_to :post, :primary_key => :id

that also throws the same exception

http://github.com/Fonsan/custom_primary_key_preload created by the following sequence

rails new custom_primary_key_preload
cd custom_primary_key_preload
rails g scaffold Post content:string
rails g scaffold Comment post_id:integer content:string
rake db:migrate

# db/seeds.rb
@post = Post.create(:content => "test")
Comment.create(:content => "test comment", :post_id => @post.id)

rake db:seed

# app/models/comment.rb
class Comment < ActiveRecord::Base
  belongs_to :post, :primary_key => :id
end

# app/controllers/comments_controller.rb
class CommentsController < ApplicationController
# GET /comments
# GET /comments.xml
def index
  @comments = Comment.includes(:post)

  respond_to do |format|
    format.html # index.html.erb
    format.xml  { render :xml => @comments }
  end
end

...
end

rails s

Visit /comments

NoMethodError in Comments#index

Showing /home/fonsan/custom_primary_key_preload/app/views/comments/index.html.erb where line #12 Bounded params in find(...:from) raised:

undefined method type' for nil:NilClass

Extracted source (around line #12 Bounded params in find(...:from)):

9:


10:
11:
12: <% @comments.each do |comment| %>
13:
14:
15:
<%= comment.post_id %> <%= comment.content %>

Reported by Erik Fonselius · September 10th, 2010 @ 12:58 PM

State: committed
Milestone: 3.x
Assigned to: Aaron Patterson Aaron Patterson
Importance: Low

Activity

  1. Erik Fonselius
    Erik Fonselius

    sorry missed the env

    ubuntu 10.04 64bit

    1.9.2 p0 in rvm
    Rails 3.0.0

    September 10th, 2010 @ 01:00 PM

  2. Ryan Bigg
    Ryan Bigg
    • Tag cleared.
    • Importance changed from to Low

    Automatic cleanup of spam.

    October 21st, 2010 @ 03:38 AM

  3. Denis Odorcic
    Denis Odorcic
    • Tag set to patch

    Attached a failing test and fix. It was only comparing strings, so if you pass in a symbol to :primary_key it'll fail.

    October 21st, 2010 @ 05:29 AM

  4. Aditya Sanghi
    Aditya Sanghi
    • State changed from new to open
    • Milestone set to 3.0.2
    • Assigned user set to Aaron Patterson
    • Tag changed from patch to active_record, patch

    Test applies fine on master.

    Not so sure about the multi-line primary_key assignment.

      primary_key = (clause1 || clause2).to_s
    

    instead of

      primary_key = clause1 || clause2
      primary_key = primary_key.to_s
    

    Just my 2 cents.

    October 21st, 2010 @ 11:15 AM

  5. Aditya Sanghi
    Aditya Sanghi
    • Milestone changed from 3.0.2 to 3.x

    We should perhaps document this as well, as i think documentation only has examples of primary_key being specified as a string not as a symbol.

    Pushing this to 3.x instead of 3.0.2 as it is not technically a bug, it's almost a feature request to get symbols as primary_key supported.

    October 21st, 2010 @ 11:20 AM

  6. Denis Odorcic
    Denis Odorcic

    I can update the patch tonight if the one-liner is the preferred method.

    October 21st, 2010 @ 03:11 PM

  7. Denis Odorcic
  8. Aditya Sanghi
    Aditya Sanghi
    • State changed from open to verified

    October 23rd, 2010 @ 07:58 PM

  9. Repository
    Repository
    • State changed from verified to resolved

    (from [cc9742920ccaf8e985fbe5239edb966949eb91c3]) Convert :primary_key in association to a string before comparing to column names, so that for example :primary_key => :another_pk works as well [#5605 Using Model.includes(:relation_with_custom_primary_key) state:resolved] http://github.com/rails/rails/commit/cc9742920ccaf8e985fbe5239edb96...

    October 30th, 2010 @ 07:32 PM

  10. Aaron Patterson
    Aaron Patterson
    • State changed from resolved to committed

    Committed, thanks!

    October 30th, 2010 @ 07:32 PM

  11. bingbing