Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Querying against a non-default primary key uses wrong value #18813

Closed
x2764tech opened this issue Feb 4, 2015 · 12 comments
Closed

Querying against a non-default primary key uses wrong value #18813

x2764tech opened this issue Feb 4, 2015 · 12 comments
Assignees
Milestone

Comments

@x2764tech
Copy link

I have a gist: https://gist.github.com/x2764tech/4ff7c015cd5d7137c295

I have the follow models:

class Post < ActiveRecord::Base
  has_many :comments, primary_key: :system_id
end

class Comment < ActiveRecord::Base
  belongs_to :post, primary_key: :system_id
end

and the test

class BugTest < Minitest::Test
  def test_association_stuff
    post = Post.create! system_id: 23
    post.comments << Comment.create!

    assert Comment.exists?(post: post)
  end
end

The data is created successfully, but the query to check whether a post exists uses Post.id and not Post.system_id:

SELECT  1 AS one FROM "comments" WHERE "comments"."post_id" = 1 LIMIT 1

The query should use the primary key specified in the relationship.

@x2764tech
Copy link
Author

I don't know if it helps, but I noticed this in 4.1.6, and replicated it in 4.1.9 and 4.2.0, as well as the master branch

@sgrif
Copy link
Contributor

sgrif commented Feb 4, 2015

This looks like the correct behavior. You are confusing the primary_key and foreign_key options.

@sgrif sgrif closed this as completed Feb 4, 2015
@sgrif
Copy link
Contributor

sgrif commented Feb 4, 2015

Never mind, I misread your issue.

@sgrif sgrif reopened this Feb 4, 2015
@sgrif
Copy link
Contributor

sgrif commented Feb 4, 2015

This is my daily reminder not to go through the issue tracker before I've had my coffee...

@sgrif sgrif added this to the 4.1.10 milestone Feb 4, 2015
@sgrif sgrif self-assigned this Feb 4, 2015
@x2764tech
Copy link
Author

@sgrif In production code, it uses a named foreign_key as well, so I was pretty certain I had it right.

@sgrif
Copy link
Contributor

sgrif commented Feb 4, 2015

Yes, you have it right. I was just reading it wrong. I'm drinking my coffee now, and then I'll fix it. 😁

@sgrif
Copy link
Contributor

sgrif commented Feb 4, 2015

This might not be something that can be backported to pre-5.0, since the fix on master depends a lot on the refactoring work I've been doing this year. I'll see what I can do to get it on 4.2 and 4.1 though

@sgrif sgrif closed this as completed in cd0ed12 Feb 4, 2015
@x2764tech
Copy link
Author

In case I hit this again:

The obvious work-around is to query the foreign-key column with the correct primary-key value instead of the object, e.g. in the example above

Comment.exists?(post_id: post.system_id)

returns the correct result.

@sgrif
Copy link
Contributor

sgrif commented Feb 4, 2015

You can also just pass the ID without changing the column. Doing Comment.exists?(post: post.system_id) will work, as well. However, I got the backport working for 4.2 and 4.1, I'll link the commits here when I'm done.

sgrif added a commit that referenced this issue Feb 4, 2015
While we query the proper columns, we go through normal handling for
converting the value to a primitive which assumes it should use the
table's primary key. If the association specifies a different value (and
we know that we're working with an association), we should use the
custom primary key instead.

Fixes #18813.

Conflicts:
    activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb
    activerecord/lib/active_record/table_metadata.rb
sgrif added a commit that referenced this issue Feb 4, 2015
While we query the proper columns, we go through normal handling for
converting the value to a primitive which assumes it should use the
table's primary key. If the association specifies a different value (and
we know that we're working with an association), we should use the
custom primary key instead.

Fixes #18813.

Conflicts:
	activerecord/lib/active_record/relation/predicate_builder/association_query_handler.rb
	activerecord/lib/active_record/table_metadata.rb
@sgrif
Copy link
Contributor

sgrif commented Feb 4, 2015

Backported for 4.1.10 in b8ba202

Backported for 4.2.1 in e43d96c

Thank you for the report.

@x2764tech
Copy link
Author

@sgrif Thanks for the quick fix!

@sgrif
Copy link
Contributor

sgrif commented Feb 4, 2015

Any time. It's my job. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants