Use the same unique index name #11273
Conversation
src/Tools/SchemaTool.php
Outdated
| @@ -337,7 +337,8 @@ public function getSchemaFromMetadata(array $classes): Schema | |||
|
|
|||
| if (isset($class->table['uniqueConstraints'])) { | |||
| foreach ($class->table['uniqueConstraints'] as $indexName => $indexData) { | |||
| $uniqIndex = new Index($indexName, $this->getIndexColumns($class, $indexData), true, false, [], $indexData['options'] ?? []); | |||
| $uniqIndexName = is_numeric($indexName) ? null : $indexName; | |||
| $uniqIndex = new Index($uniqIndexName, $this->getIndexColumns($class, $indexData), true, false, [], $indexData['options'] ?? []); | |||
There was a problem hiding this comment.
ORM 3.0 still supports DBAL 3.6 and up, where Doctrine\DBAL\Schema\Index can't have null as an index name. A nullable name was introduced in DBAL 4.0. It looks like the Index instance is only created to be used in $tableIndex->isFulfilledBy($uniqIndex) a few lines later.
There was a problem hiding this comment.
Hmm, yes, you are right. As I understand it, this check was added as part of issue #1375. Under the current circumstances, there is another quick fix that I see: to compare with existing indexes, use an index with an empty name if the array key is a number. Given that the index name is not used for comparison and is declared as an empty string by default, this looks like it would work. Tests also confirm this for dbal versions 3.8 and 4.0. However, when adding a unique index, passing the index name as null seems to be fine, as long as the index name is empty. On the other hand, it doesn't seem very nice. What do you think about that ?
There was a problem hiding this comment.
I don't expect that the name will be used in a check, because the focus is on involved columns to prevent duplicates. It would be the same having two unique indices with null. Please try your suggested approach on this.
There was a problem hiding this comment.
Fine, I've made a few minor edits.
deasraze
force-pushed
the
fix-unique-constraint-name
branch
from
54a6dc9
to
a97c431
Compare
|
Thank you. This would at least handle the type error. Could you please do a rebase of your branch an resolve the conflicts? |
|
Actually, this should've been fixed by #11314 already which was shipped with 2.18.3 and 3.0.3. Can you please check if you still have this issue with the latest ORM release (3.1.0 or 2.19.0 at the moment)? |
|
I checked on the latest release 3.1.0 with tests from the current PR. The issue is not reproducible. In that case, I'm closing the PR. |
|
Thank you! |
Fixes #11246.