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

Fix reflection.association_primary_key for has_many association #27609

Merged
merged 1 commit into from
Aug 14, 2017

Conversation

kamipo
Copy link
Member

@kamipo kamipo commented Jan 8, 2017

It is incorrect to treat options[:primary_key] as
association_primary_key if has_many associations because the
:primary_key means the column on the owner record, not on the
association record. It will break ids_reader and ids_writer.

  people(:david).essay_ids
  # => ActiveRecord::StatementInvalid: Mysql2::Error: Unknown column 'essays.first_name' in 'field list': SELECT `essays`.first_name FROM `essays` WHERE `essays`.`writer_id` = 'David'

Fixes #14439.

@kamipo
Copy link
Member Author

kamipo commented Mar 4, 2017

@tenderlove Could you review this?

It is incorrect to treat `options[:primary_key]` as
`association_primary_key` if `has_many` associations because the
`:primary_key` means the column on the owner record, not on the
association record. It will break `ids_reader` and `ids_writer`.

```ruby
  people(:david).essay_ids
  # => ActiveRecord::StatementInvalid: Mysql2::Error: Unknown column 'essays.first_name' in 'field list': SELECT `essays`.first_name FROM `essays` WHERE `essays`.`writer_id` = 'David'
```

Fixes rails#14439.
@kamipo kamipo force-pushed the fix_association_primary_key branch from 09e8cfc to b23e869 Compare August 13, 2017 07:28
@rafaelfranca rafaelfranca merged commit 8244a45 into rails:master Aug 14, 2017
rafaelfranca added a commit that referenced this pull request Aug 14, 2017
Fix `reflection.association_primary_key` for `has_many` association
rafaelfranca added a commit that referenced this pull request Aug 14, 2017
Fix `reflection.association_primary_key` for `has_many` association
@kamipo kamipo deleted the fix_association_primary_key branch August 15, 2017 02:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants