Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #7621 (new defect)

Opened 2 years ago

Last modified 9 months ago

[PATCH] Using "has_many :through" with non-standard primary key columns is broken

Reported by: fedot Assigned to: core
Priority: normal Milestone:
Component: ActiveRecord Version: edge
Severity: normal Keywords:
Cc: shoe

Description

When attempting to use a "has_many :through" association with three models using non-standard primary keys, Rails is not constructing SQL finder query properly. It is not using correct primary key column for the "through" table. Here is a detailed example:

create_table :books, :primary_key => :my_book_id, :force => true do |t|
  t.column :name, :string
end

create_table :chapters, :primary_key => :my_chapter_id, :force => true
do |t|
  t.column :parent_book_id, :integer
  t.column :name, :string
end

create_table :paragraphs, :primary_key => :paragraph_id, :force => true
do |t|
  t.column :parent_chapter_id, :integer
  t.column :name, :string
end

class Book < ActiveRecord::Base
  set_primary_key(:my_book_id)
  has_many :chapters, :foreign_key => :parent_book_id
  has_many :paragraphs, :through => :chapters, :source => :paragraphs
end

class Chapter < ActiveRecord::Base
  set_primary_key(:my_chapter_id)
  has_many :paragraphs, :foreign_key => :parent_chapter_id
  belongs_to :book, :foreign_key => :parent_book_id
end

class Paragraph < ActiveRecord::Base
  set_primary_key(:my_paragraph_id)
  belongs_to :chapter, :foreign_key => :parent_chapter_id
end

Now, when I run the following from the console:

Book.find(1).paragraphs

The following SQL gets sent to MySQL:

SELECT paragraphs.* FROM paragraphs INNER JOIN chapters ON
paragraphs.parent_chapter_id = chapters.my_paragraph_id WHERE
chapters.parent_book_id = 1

The ON clause is in error. Instead of saying:

ON paragraphs.parent_chapter_id = chapters.my_paragraph_id

it should be:

ON paragraphs.parent_chapter_id = chapters.my_chapter_id

Attached patch fixes the problem.

Attachments

has_many_through_and_non_standard_primary_keys.diff (1.2 kB) - added by fedot on 02/21/07 22:57:57.
change_primary_key_for_taggings_to_tagging_id.diff (2.1 kB) - added by fedot on 02/22/07 18:21:26.
has_many_through_with_custom_source_primary_keys.diff (8.8 kB) - added by jcoglan on 08/09/07 14:10:28.

Change History

02/21/07 22:57:57 changed by fedot

  • attachment has_many_through_and_non_standard_primary_keys.diff added.

02/22/07 06:01:50 changed by hasmanyjosh

I think this looks good (though I haven't had time to try it out). Can you please include a test case in the patch that shows the problem and that your change fixes it?

02/22/07 18:21:26 changed by fedot

  • attachment change_primary_key_for_taggings_to_tagging_id.diff added.

02/22/07 18:28:37 changed by fedot

  • version changed from 1.2.1 to edge.
  • milestone deleted.

Actually, my fix (first attached patch) doesn't appear to fix the problem, even though it doesn't break any tests. The reason tests still pass is that Rails tests do not use custom id's for any of the "through" tables.

I attached another "diff" that changed primary key for table "taggings" from default "id" to "tagging_id". Now, that breaks a couple of tests, in particular, test_has_many_through_polymorphic_has_many and test_include_has_many_through_polymorphic_has_many fail in join_model_test.rb, trying to use posts.tagging_id instead of proper posts.id when doing a JOIN.

I've been banging my head against the wall trying to fix ActiveRecord::Associations::HasManyThroughAssociation#construct_joins, but no matter what I try it breaks some tests while fixing others. So, I would like to suggest that someone more knowledgeable than me applies my patch to use custom primary key to "taggings" and see if they can fix the problem, now that we can replicate it with standard Rails tests.

08/09/07 14:10:08 changed by jcoglan

The patch here makes one too many changes - only the second reflection_primary_key assignment needs to change. I'm adding a patch that adds two new fixture models (rather than messing with existing fixtures and breaking everything), both with custom primary and foreign keys. Specifically:

  • Comment has_many :comment_attachments
  • Comment belongs_to :comment_status

And Post has_many :through for both of those. I've written the schema definition in the MySQL files - not sure whether I need to put them in schema.rb (putting them there doesn't seem to actually create any tables when I run tests), or manually port them to all the other DB files. I added a couple of test cases, both of which pass with the fix in place. One test broke due to a NULL status_id field in a comment, so I fixed that too.

There's a working fix in #6461 for the construct_joins issue that lets you nest has_many :through rules, if you're interested in that. I've been trying to modify that method for some time now and am going round in circles, but the guys on that ticket seem to have it nailed.

08/09/07 14:10:28 changed by jcoglan

  • attachment has_many_through_with_custom_source_primary_keys.diff added.

03/11/08 17:31:50 changed by shoe

  • cc set to shoe.