Foreign key creation fails when a table prefix or suffix is used #19749
Conversation
|
This spec isn't failing locally, reopening. |
| @@ -796,7 +796,7 @@ def remove_foreign_key(from_table, options_or_to_table = {}) | |||
| end | |||
|
|
|||
| def foreign_key_column_for(table_name) # :nodoc: | |||
| "#{table_name.to_s.singularize}_id" | |||
| "#{ActiveRecord::Base.connection.remove_prefix_and_suffix(table_name).to_s.singularize}_id" | |||
There was a problem hiding this comment.
@sgrif I didn't realize I was in that context here, I'll make the update.
|
Please squash into a single commit |
|
/cc @senny |
|
@sgrif I'd like some more feedback on a few of your comments before I re-submit with the updates. Thanks for the feedback. |
arjes
force-pushed
the
correct_foreign_keys_with_prefix_and_suffix
branch
from
047ce9e
to
c7457d0
Compare
When using a prefix of suffix with AR, the add_foreign_key migrator would fail to properly identify the destination column properly. i.e. ActiveRecord::Base.table_name_prefix = "foo_" add_foreign_key :comments, :posts => Attempts to create a foreign key on the column name foo_posts_id Since this code is used in multiple places, the function proper_table_name was extracted to the abstract_adapter.
arjes
force-pushed
the
correct_foreign_keys_with_prefix_and_suffix
branch
from
c7457d0
to
eca2484
Compare
|
Squashed & Rebased. Waiting for feedback on remaining items. |
| ActiveRecord::Base.table_name_suffix = '' | ||
| end | ||
|
|
||
| def test_add_foreign_key_with_prefix_and_suffix |
There was a problem hiding this comment.
test_remove_foreign_key_with_prefix_and_suffix and test_add_foreign_key_with_prefix_and_suffix are very similar. It's probably worth combining them. Then we can add and remove the foreign key in a single test. Something along the lines of
test_managing_foreign_keys_with_prefix_and_suffix
|
Sorry to close after all of this, but I just realized this is against 4-2-stable. Can you re-open against master? |
Simply using a prefix, or suffix:
will break the addition of a foreign key with the following migration:
This migration attempts to add a foreign key referencing the test_post_id column. If you specify the column the migration will run correctly.
In order to correct this I extracted the proper_table_name and the remove_prefix_and_suffix function to the abstract connector since it is needed in multiple contexts (SchemaStatments and Migrations)