Merge pull request #3583 from morozov/event-types-cleanup

Getting rid of the column name index

UPGRADE.md

 # Upgrade to 3.0
 
+## BC BREAK: Changes in `Doctrine\DBAL\Event\SchemaCreateTableEventArgs`
+
+Table columns are no longer indexed by column name. Use the `name` attribute of the column instead.
+
 ## BC BREAK: Changes in the `Doctrine\DBAL\Schema` API
 
 - Column precision no longer defaults to 10. The default value is NULL.

lib/Doctrine/DBAL/Event/SchemaCreateTableEventArgs.php

@@ -9,14 +9,15 @@ use Doctrine\DBAL\Schema\Table;
 use function array_merge;
 
 /**
- * Event Arguments used when SQL queries for creating tables are generated inside Doctrine\DBAL\Platform\AbstractPlatform.
+ * Event Arguments used when SQL queries for creating tables are generated
+ * inside Doctrine\DBAL\Platform\AbstractPlatform.
  */
 class SchemaCreateTableEventArgs extends SchemaEventArgs
 {
     /** @var Table */
     private $table;
 
-    /** @var array<string, array<string, mixed>> */
+    /** @var array<int, array<string, mixed>> */
     private $columns;
 
     /** @var array<string, mixed> */
@@ -29,8 +30,8 @@ class SchemaCreateTableEventArgs extends SchemaEventArgs
     private $sql = [];
 
     /**
-     * @param array<string, array<string, mixed>> $columns
-     * @param array<string, mixed>                $options
+     * @param array<int, array<string, mixed>> $columns
+     * @param array<string, mixed>             $options
      */
     public function __construct(Table $table, array $columns, array $options, AbstractPlatform $platform)
     {
@@ -46,7 +47,7 @@ class SchemaCreateTableEventArgs extends SchemaEventArgs
     }
 
     /**
-     * @return array<string, array<string, mixed>>
+     * @return array<int, array<string, mixed>>
      */
     public function getColumns() : array
     {

lib/Doctrine/DBAL/Platforms/AbstractPlatform.php

@@ -1345,10 +1345,7 @@ abstract class AbstractPlatform
                 $columnData['primary'] = true;
             }
 
-            $columnName = $columnData['name'];
-            assert(is_string($columnName));
-
-            $columns[$columnName] = $columnData;
+            $columns[] = $columnData;
         }
 
         if ($this->_eventManager !== null && $this->_eventManager->hasListeners(Events::onSchemaCreateTable)) {
@@ -1875,9 +1872,9 @@ abstract class AbstractPlatform
     /**
      * Gets declaration of a number of fields in bulk.
      *
-     * @param mixed[][] $fields A multidimensional associative array.
-     *                          The first dimension determines the field name, while the second
-     *                          dimension is keyed with the name of the properties
+     * @param mixed[][] $fields A multidimensional array.
+     *                          The first dimension determines the ordinal position of the field,
+     *                          while the second dimension is keyed with the name of the properties
      *                          of the field being declared as array indexes. Currently, the types
      *                          of supported field properties are as follows:
      *
@@ -1903,8 +1900,8 @@ abstract class AbstractPlatform
     {
         $queryFields = [];
 
-        foreach ($fields as $fieldName => $field) {
-            $queryFields[] = $this->getColumnDeclarationSQL($fieldName, $field);
+        foreach ($fields as $field) {
+            $queryFields[] = $this->getColumnDeclarationSQL($field['name'], $field);
         }
 
         return implode(', ', $queryFields);
@@ -2050,19 +2047,19 @@ abstract class AbstractPlatform
     public function getCheckDeclarationSQL(array $definition) : string
     {
         $constraints = [];
-        foreach ($definition as $field => $def) {
+        foreach ($definition as $def) {
             if (is_string($def)) {
                 $constraints[] = 'CHECK (' . $def . ')';
             } else {
                 if (isset($def['min'])) {
-                    $constraints[] = 'CHECK (' . $field . ' >= ' . $def['min'] . ')';
+                    $constraints[] = 'CHECK (' . $def['name'] . ' >= ' . $def['min'] . ')';
                 }
 
                 if (! isset($def['max'])) {
                     continue;
                 }
 
-                $constraints[] = 'CHECK (' . $field . ' <= ' . $def['max'] . ')';
+                $constraints[] = 'CHECK (' . $def['name'] . ' <= ' . $def['max'] . ')';
             }
         }
 

lib/Doctrine/DBAL/Platforms/OraclePlatform.php

@@ -367,17 +367,16 @@ class OraclePlatform extends AbstractPlatform
         $options['indexes'] = [];
         $sql                = parent::_getCreateTableSQL($tableName, $columns, $options);
 
-        foreach ($columns as $name => $column) {
+        foreach ($columns as $column) {
             if (isset($column['sequence'])) {
                 $sql[] = $this->getCreateSequenceSQL($column['sequence']);
             }
 
-            if (! isset($column['autoincrement']) || ! $column['autoincrement'] &&
-               (! isset($column['autoinc']) || ! $column['autoinc'])) {
+            if (empty($column['autoincrement'])) {
                 continue;
             }
 
-            $sql = array_merge($sql, $this->getCreateAutoincrementSql($name, $tableName));
+            $sql = array_merge($sql, $this->getCreateAutoincrementSql($column['name'], $tableName));
         }
 
         if (isset($indexes) && ! empty($indexes)) {

lib/Doctrine/DBAL/Platforms/SqlitePlatform.php

@@ -368,8 +368,10 @@ class SqlitePlatform extends AbstractPlatform
         $keyColumns = array_unique(array_values($options['primary']));
 
         foreach ($keyColumns as $keyColumn) {
-            if (! empty($columns[$keyColumn]['autoincrement'])) {
-                return '';
+            foreach ($columns as $column) {
+                if ($column['name'] === $keyColumn && ! empty($column['autoincrement'])) {
+                    return '';
+                }
             }
         }
 

tests/Doctrine/Tests/DBAL/Functional/TemporaryTableTest.php

@@ -44,8 +44,15 @@ class TemporaryTableTest extends DbalFunctionalTestCase
         }
 
         $platform          = $this->connection->getDatabasePlatform();
-        $columnDefinitions = ['id' => ['type' => Type::getType('integer'), 'notnull' => true]];
-        $tempTable         = $platform->getTemporaryTableName('my_temporary');
+        $columnDefinitions = [
+            [
+                'name' => 'id',
+                'type' => Type::getType('integer'),
+                'notnull' => true,
+            ],
+        ];
+
+        $tempTable = $platform->getTemporaryTableName('my_temporary');
 
         $createTempTableSQL = $platform->getCreateTemporaryTableSnippetSQL() . ' ' . $tempTable . ' ('
                 . $platform->getColumnDeclarationListSQL($columnDefinitions) . ')';
@@ -79,8 +86,15 @@ class TemporaryTableTest extends DbalFunctionalTestCase
         }
 
         $platform          = $this->connection->getDatabasePlatform();
-        $columnDefinitions = ['id' => ['type' => Type::getType('integer'), 'notnull' => true]];
-        $tempTable         = $platform->getTemporaryTableName('my_temporary');
+        $columnDefinitions = [
+            [
+                'name' => 'id',
+                'type' => Type::getType('integer'),
+                'notnull' => true,
+            ],
+        ];
+
+        $tempTable = $platform->getTemporaryTableName('my_temporary');
 
         $createTempTableSQL = $platform->getCreateTemporaryTableSnippetSQL() . ' ' . $tempTable . ' ('
                 . $platform->getColumnDeclarationListSQL($columnDefinitions) . ')';