Merge pull request #6550 from morozov/implicit-index-names

Simplify tracking implicitly created indexes

Author: morozov

phpcs.xml.dist

@@ -102,9 +102,7 @@
 
     <!-- See https://github.com/squizlabs/PHP_CodeSniffer/issues/2837 -->
     <rule ref="Squiz.NamingConventions.ValidVariableName.NotCamelCaps">
-        <exclude-pattern>src/Connection.php</exclude-pattern>
-        <exclude-pattern>src/Schema/Comparator.php</exclude-pattern>
-        <exclude-pattern>src/SQLParserUtils.php</exclude-pattern>
+        <exclude-pattern>src/Schema/Table.php</exclude-pattern>
     </rule>
 
     <!-- some statement classes close cursor using an empty while-loop -->

src/Schema/Exception/PrimaryKeyAlreadyExists.php

@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Schema\Exception;
+
+use Doctrine\DBAL\Schema\SchemaException;
+use LogicException;
+
+use function sprintf;
+
+/** @psalm-immutable */
+final class PrimaryKeyAlreadyExists extends LogicException implements SchemaException
+{
+    public static function new(string $tableName): self
+    {
+        return new self(
+            sprintf('Primary key was already defined on table "%s".', $tableName),
+        );
+    }
+}

src/Schema/Table.php

@@ -11,13 +11,13 @@
 use Doctrine\DBAL\Schema\Exception\IndexDoesNotExist;
 use Doctrine\DBAL\Schema\Exception\IndexNameInvalid;
 use Doctrine\DBAL\Schema\Exception\InvalidTableName;
+use Doctrine\DBAL\Schema\Exception\PrimaryKeyAlreadyExists;
 use Doctrine\DBAL\Schema\Exception\UniqueConstraintDoesNotExist;
 use Doctrine\DBAL\Types\Type;
 use LogicException;
 
 use function array_merge;
 use function array_values;
-use function in_array;
 use function preg_match;
 use function sprintf;
 use function strtolower;
@@ -30,15 +30,20 @@ class Table extends AbstractAsset
     /** @var Column[] */
     protected array $_columns = [];
 
-    /** @var Index[] */
-    private array $implicitIndexes = [];
-
     /** @var array<string, string> keys are new names, values are old names */
     protected array $renamedColumns = [];
 
     /** @var Index[] */
     protected array $_indexes = [];
 
+    /**
+     * The keys of this array are the names of the indexes that were implicitly created as backing for foreign key
+     * constraints. The values are not used but must be non-null for {@link isset()} to work correctly.
+     *
+     * @var array<string,true>
+     */
+    private array $implicitIndexNames = [];
+
     protected ?string $_primaryKeyName = null;
 
     /** @var UniqueConstraint[] */
@@ -606,36 +611,41 @@ protected function _addColumn(Column $column): void
     /**
      * Adds an index to the table.
      */
-    protected function _addIndex(Index $indexCandidate): self
+    protected function _addIndex(Index $index): self
     {
-        $indexName               = $indexCandidate->getName();
-        $indexName               = $this->normalizeIdentifier($indexName);
-        $replacedImplicitIndexes = [];
+        $indexName = $this->normalizeIdentifier($index->getName());
 
-        foreach ($this->implicitIndexes as $name => $implicitIndex) {
-            if (! $implicitIndex->isFulfilledBy($indexCandidate) || ! isset($this->_indexes[$name])) {
+        $replacedImplicitIndexNames = [];
+
+        foreach ($this->implicitIndexNames as $implicitIndexName => $_) {
+            if (! isset($this->_indexes[$implicitIndexName])) {
+                continue;
+            }
+
+            if (! $this->_indexes[$implicitIndexName]->isFulfilledBy($index)) {
                 continue;
             }
 
-            $replacedImplicitIndexes[] = $name;
+            $replacedImplicitIndexNames[$implicitIndexName] = true;
         }
 
-        if (
-            (isset($this->_indexes[$indexName]) && ! in_array($indexName, $replacedImplicitIndexes, true)) ||
-            ($this->_primaryKeyName !== null && $indexCandidate->isPrimary())
-        ) {
+        if (isset($this->_indexes[$indexName]) && ! isset($replacedImplicitIndexNames[$indexName])) {
             throw IndexAlreadyExists::new($indexName, $this->_name);
         }
 
-        foreach ($replacedImplicitIndexes as $name) {
-            unset($this->_indexes[$name], $this->implicitIndexes[$name]);
+        if ($this->_primaryKeyName !== null && $index->isPrimary()) {
+            throw PrimaryKeyAlreadyExists::new($this->_name);
+        }
+
+        foreach ($replacedImplicitIndexNames as $name => $_) {
+            unset($this->_indexes[$name], $this->implicitIndexNames[$name]);
         }
 
-        if ($indexCandidate->isPrimary()) {
+        if ($index->isPrimary()) {
             $this->_primaryKeyName = $indexName;
         }
 
-        $this->_indexes[$indexName] = $indexCandidate;
+        $this->_indexes[$indexName] = $index;
 
         return $this;
     }
@@ -671,7 +681,7 @@ protected function _addUniqueConstraint(UniqueConstraint $constraint): self
             }
         }
 
-        $this->implicitIndexes[$this->normalizeIdentifier($indexName)] = $indexCandidate;
+        $this->implicitIndexNames[$this->normalizeIdentifier($indexName)] = true;
 
         return $this;
     }
@@ -709,7 +719,7 @@ protected function _addForeignKeyConstraint(ForeignKeyConstraint $constraint): s
         }
 
         $this->_addIndex($indexCandidate);
-        $this->implicitIndexes[$this->normalizeIdentifier($indexName)] = $indexCandidate;
+        $this->implicitIndexNames[$this->normalizeIdentifier($indexName)] = true;
 
         return $this;
     }