dbal
dbal copied to clipboard
doctrine
Fix implicit/generated unique index name typo
| Q | A |
|---|---|
| Type | bug |
| Fixed issues | n/a |
Summary
In Table::addUniqueConstraint() it was/is correct, but in Table::_addUniqueConstraint() it was wrong.
This change is barely acceptable in a patch release since it doesn't fix a bug and requires a schema migration. Depending on how the DBAL handles the rename, it may be a BC break. Please test the migration across the platforms.
@morozov this is more or less typo, as addUniqueIndex method https://github.com/doctrine/dbal/blob/3.3.7/src/Schema/Table.php#L237 uses uniq, but _addUniqueConstraint does not.
If it's a typo that doesn't change any behavior, why does this code exist in the first place?
If it's a typo that doesn't change any behavior, why does this code exist in the first place?
it depends if the unique index is created using a constructor or not. I agree, the same should not be done on 2 places, but for now, I just correct the typo to unify the behaviour.
it depends if the unique index is created using a constructor or not.
If it's created via the constructor, the resulting name will change. The migration from the old schema to the new one must be tested across platforms.
I changed the target to 3.4.x. As discussed above, the code is basically on 2 places, it is a typo fix, thus no extra test added. I belive it can be merged as is, the code is welcomed to be deduplicated, but I will not pursue the refactoring.
If the changes in this code do not result in any behavior changes, this code is redundant and should be removed. Changing code for the sake of changing it is not welcomed. It requires effort to review the changes and doesn't fix the actual problem (the code still doesn't do anything).