fix: qualify and order the foreign key lookup in rex_sql_table - #6623
Conversation
The INFORMATION_SCHEMA lookup joined REFERENTIAL_CONSTRAINTS to KEY_COLUMN_USAGE on the constraint name alone. Constraint names are only unique per schema, so on a server hosting several databases the join also picks up same-named constraints from other schemas. On a local setup with 39 REDAXO databases, reading the two foreign keys of rex_user_session returned 78 rows instead of 2. The duplicates usually collapse again, because the columns are collected into a map keyed by column name. They stop being harmless as soon as a same-named constraint in another database covers different columns: the foreign key then carries columns that do not belong to it, and consumers comparing against a stored schema see a difference that is not there. The query also had no ORDER BY. INFORMATION_SCHEMA gives no ordering guarantee, so both the order of the foreign keys and the column order within a composite key were left to chance. The latter matters, since that order ends up in the generated DDL. Qualify the join by schema and table, and order by constraint name and ordinal position.
gharlan
left a comment
gharlan
left a comment
There was a problem hiding this comment.
Note: This review was written by Claude (Claude Code), run by @gharlan. The findings were verified against a local MariaDB 12.3; @gharlan has not reviewed the wording line by line, so treat the reasoning as something to check rather than as a maintainer verdict.
The analysis holds up: joining REFERENTIAL_CONSTRAINTS to KEY_COLUMN_USAGE on the constraint name alone is genuinely broken, because KEY_COLUMN_USAGE spans all schemas while FK names are unique only per schema. And c.TABLE_NAME = k.TABLE_NAME is less redundant than it looks — index names are unique only per table, so a same-named UNIQUE index can sit on a different table of the same schema; that condition catches it.
The ORDER BY k.ORDINAL_POSITION part is arguably the more valuable half of the fix, since that order ends up in the generated DDL.
Two inline notes below, plus one optional test suggestion:
Test coverage (optional): testAddForeignKey verifies the round-trip with assertEquals($fk, ...), and == on arrays is order-insensitive — so the test cannot observe the column order this PR is fixing. One line would pin the intent down:
self::assertSame(
['config_namespace' => 'namespace', 'config_key' => 'key'],
$table->getForeignKey('test1_fk_config')?->getColumns(),
);Covering the cross-schema case properly would need a second database, which seems out of scope here.
KEY_COLUMN_USAGE lists primary, unique and foreign key constraints
alike, and a unique index may share its name with a foreign key on the
same table. Both statements are accepted:
CREATE TABLE child (x varchar(20), y varchar(20), UNIQUE KEY myname (x,y));
ALTER TABLE child ADD CONSTRAINT myname FOREIGN KEY (x,y) REFERENCES parent (a,b);
The unique index rows then come back alongside the foreign key rows.
They carry REFERENCED_COLUMN_NAME = NULL, which contradicts the string
type in the annotation above, and they share the ordinal position of the
real rows, so the ORDER BY does not decide which of them wins the column
assignment. Restricting the join to rows that belong to a referential
constraint drops them.
Also pin the column order in testAddForeignKey: assertEquals compares
arrays order-insensitively and could not observe it.
Verified on MySQL 8.4 and, in review, on MariaDB 12.3.
gharlan
left a comment
gharlan
left a comment
There was a problem hiding this comment.
Claude sagt mir:
Alle drei Punkte übernommen, und zwar sauber. Ich hab's lokal durchgeprüft.
|
meiner hatte alles auf mysql 8.4 getestet. |
|
Viel zu viel KI involved, ist mir ein bisschen ungewohnt |
|
aber das thema nervte mich, hat mir claude rausgesucht über die Jahre: │ xxxos │ 19 |
|
Bei uns kein einziges Mal gesehen, ggf MariaDB vs MySQL. |
|
thx |
3 participants
The INFORMATION_SCHEMA lookup joined REFERENTIAL_CONSTRAINTS to KEY_COLUMN_USAGE on the constraint name alone. Constraint names are only unique per schema, so on a server hosting several databases the join also picks up same-named constraints from other schemas. On a local setup with 39 REDAXO databases, reading the two foreign keys of rex_user_session returned 78 rows instead of 2.
The duplicates usually collapse again, because the columns are collected into a map keyed by column name. They stop being harmless as soon as a same-named constraint in another database covers different columns: the foreign key then carries columns that do not belong to it, and consumers comparing against a stored schema see a difference that is not there.
The query also had no ORDER BY. INFORMATION_SCHEMA gives no ordering guarantee, so both the order of the foreign keys and the column order within a composite key were left to chance. The latter matters, since that order ends up in the generated DDL.
Qualify the join by schema and table, and order by constraint name and ordinal position.