Auto-generate foreign key constraint names when not provided#1041
Auto-generate foreign key constraint names when not provided#1041dereuromark wants to merge 4 commits into5.xfrom
Conversation
dereuromark
force-pushed
the
fix-auto-generate-fk-constraint-name
branch
2 times, most recently
from
c582b85 to
809abc9
Compare
When a foreign key is added without an explicit name, the MysqlAdapter and SqliteAdapter now generate a name following the pattern 'tablename_columnname' (e.g., 'articles_user_id' for a FK on the user_id column in the articles table). This matches the behavior of PostgresAdapter and SqlserverAdapter, which already auto-generate FK names. This ensures constraint names are always strings and prevents issues with constraint lookup methods that expect string names.
Update the expected FK names in comparison files and schema dumps to match the new auto-generated naming pattern (tablename_columnname).
dereuromark
force-pushed
the
fix-auto-generate-fk-constraint-name
branch
from
ed2b2d3 to
629c85e
Compare
The FK constraint name change from auto-generated (ibfk_N) to explicit (table_column) also affects the implicit index MySQL creates for FKs. Update comparison files to reflect the index rename from user_id to articles_user_id.
With the FK naming changes, both the lock file and database have the index named articles_user_id, so no diff is needed for this index. Remove the index rename operations that were incorrectly added.
| if ($foreignKey->getName()) { | ||
| $def .= ' CONSTRAINT ' . $this->quoteColumnName((string)$foreignKey->getName()); | ||
| } | ||
| $constraintName = $foreignKey->getName() ?: ($tableName . '_' . implode('_', $foreignKey->getColumns())); |
There was a problem hiding this comment.
Does mysql have any length limits we have to be concerned about like with postgres?
There was a problem hiding this comment.
I notice the existing PostgreSQL adapter autogenerate doesn't have explicit truncation either - it builds names like {table}_{columns}_fkey without length limiting.
The question is valid. MySQL has a 64-character limit for identifiers (including constraint names). The current implementation:
$constraintName = $foreignKey->getName() ?: ($tableName . '_' . implode('_',
$foreignKey->getColumns())); Could exceed 64 characters with long table names or multiple columns.
There was a problem hiding this comment.
We could built this into #1042
For now this PR aims to fix the critical bug and the inconsistency first.
Having a too long index is usually seldom and would often require a custom naming anyway.
|
Updated PR #1042 with database-specific limits:
|
markstory
left a comment
markstory
left a comment
There was a problem hiding this comment.
Changing the name of indexes, will change the schema created by a migration history. What used to be automatically named by the database engine will now be given a different name by migrations which could cause index add, index drop/change operations to fail as the name of an index has changed.
I don't think we have a good solution for this, but it should be something we point out in the release notes as a potential upgrade issue.
|
After the merge of this and follow up, maybe I can invest some time into shimming possibilities? // Pseudocode for dropForeignKey fallback
$fk = $this->findForeignKeyByName($name);
if (!$fk) {
$fk = $this->findForeignKeyByColumns($columns);
// Log warning about name mismatch
} But that would only work if we can reverse engineer the columns from the up() statement. |
2 participants
Summary
addForeignKey(){table}_{columns}(matching PostgresAdapter and SqlserverAdapter behavior)This fixes an issue where unnamed constraints could cause problems with CakePHP's
ConnectionHelper::dropTables()which expects string constraint names. When no name was provided, MySQL would auto-generate one, but the schema reflection could store it with integer array keys, causingTypeErrorinTableSchema::getConstraint().Fixes #1040