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

Getting rid of the column name index
doctrine · Jun 4, 2019 · 0c4aa7e · 0c4aa7e
2 parents c5d938e + 94d2a4a
commit 0c4aa7e
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 27 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -1,5 +1,9 @@
 # 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.

diff --git a/lib/Doctrine/DBAL/Event/SchemaCreateTableEventArgs.php b/lib/Doctrine/DBAL/Event/SchemaCreateTableEventArgs.php
@@ -9,14 +9,15 @@
 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 @@ public function getTable() : Table
     }
 
     /**
-     * @return array<string, array<string, mixed>>
+     * @return array<int, array<string, mixed>>
      */
     public function getColumns() : array
     {

diff --git a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php
@@ -1345,10 +1345,7 @@ public function getCreateTableSQL(Table $table, int $createFlags = self::CREATE_
                 $columnData['primary'] = true;
             }
 
-            $columnName = $columnData['name'];
-            assert(is_string($columnName));
-
-            $columns[$columnName] = $columnData;
+            $columns[] = $columnData;
         }
 
         if ($this->_eventManager !== null && $this->_eventManager->hasListeners(Events::onSchemaCreateTable)) {
@@ -1859,9 +1856,9 @@ protected function _getAlterTableIndexForeignKeySQL(TableDiff $diff) : array
     /**
      * 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:
      *
@@ -1887,8 +1884,8 @@ public function getColumnDeclarationListSQL(array $fields) : string
     {
         $queryFields = [];
 
-        foreach ($fields as $fieldName => $field) {
-            $queryFields[] = $this->getColumnDeclarationSQL($fieldName, $field);
+        foreach ($fields as $field) {
+            $queryFields[] = $this->getColumnDeclarationSQL($field['name'], $field);
         }
 
         return implode(', ', $queryFields);
@@ -2034,19 +2031,19 @@ public function getDefaultValueDeclarationSQL(array $field) : string
     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'] . ')';
             }
         }
 

diff --git a/lib/Doctrine/DBAL/Platforms/OraclePlatform.php b/lib/Doctrine/DBAL/Platforms/OraclePlatform.php
@@ -367,17 +367,16 @@ protected function _getCreateTableSQL(string $tableName, array $columns, array $
         $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)) {

diff --git a/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php b/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php
@@ -360,8 +360,10 @@ private function getNonAutoincrementPrimaryKeyDefinition(array $columns, array $
         $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 '';
+                }
             }
         }
 

diff --git a/tests/Doctrine/Tests/DBAL/Functional/TemporaryTableTest.php b/tests/Doctrine/Tests/DBAL/Functional/TemporaryTableTest.php
@@ -44,8 +44,15 @@ public function testDropTemporaryTableNotAutoCommitTransaction() : void
         }
 
         $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 @@ public function testCreateTemporaryTableNotAutoCommitTransaction() : void
         }
 
         $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) . ')';