[TASK] dbal: Add support for ALTER TABLE ADD/DROP KEY 97/40397/10
authorMorton Jonuschat <m.jonuschat@mojocode.de>
Wed, 17 Jun 2015 19:38:19 +0000 (21:38 +0200)
committerAnja Leichsenring <aleichsenring@ab-softlab.de>
Thu, 9 Jul 2015 20:31:46 +0000 (22:31 +0200)
Add support for creating and removing indices to DBAL. The SQL is
generated using the respective ADOdb methods, so adding/removing
PRIMARY keys is not possible.

MySQL prefix lengths will be removed from columns in an index
definition unless the DBMS specifically enables support for this
feature. To ensure that index columns get properly quoted by
ADOdb the field names need to be wrapped in backticks.

Due to the common requirement that index names must be unique within a
database/schema the requested index name will be prepended with a
unique table name identifier.

Resolves: #67445
Resolves: #67531
Releases: master
Change-Id: I274be8df3078867309dde0d5771853c67903719d
Reviewed-on: http://review.typo3.org/40397
Reviewed-by: Stephan Großberndt <stephan@grossberndt.de>
Tested-by: Stephan Großberndt <stephan@grossberndt.de>
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
typo3/sysext/core/Documentation/Changelog/master/Important-67445-DBALSupportForALTERTABLEADDDROPKEYAdded.rst [new file with mode: 0644]
typo3/sysext/dbal/Classes/Database/DatabaseConnection.php
typo3/sysext/dbal/Classes/Database/Specifics/AbstractSpecifics.php
typo3/sysext/dbal/Classes/Database/SqlParser.php
typo3/sysext/dbal/Tests/Unit/Database/DatabaseConnectionPostgresqlTest.php
typo3/sysext/dbal/Tests/Unit/Database/SqlParserTest.php

diff --git a/typo3/sysext/core/Documentation/Changelog/master/Important-67445-DBALSupportForALTERTABLEADDDROPKEYAdded.rst b/typo3/sysext/core/Documentation/Changelog/master/Important-67445-DBALSupportForALTERTABLEADDDROPKEYAdded.rst
new file mode 100644 (file)
index 0000000..d5759b9
--- /dev/null
@@ -0,0 +1,21 @@
+===================================================================
+Important: #67445 - DBAL support for ALTER TABLE ADD/DROP KEY added
+===================================================================
+
+Description
+===========
+
+The prefix used to build the name of indexes in a database/schema has
+been changed. The prefix is used to ensure that an index name is unique
+within a database/schema.
+
+Formerly the requested index name was prepended with the table name to
+which the index was added. In some cases this results in index names that
+exceeded the valid identifier length on all DBMS except MS SQL Server.
+The silent truncation of these identifiers results in non-unique names or
+index names that can not be matched to the original name.
+
+With TYPO3 7.4 the prefix used for index names has been changed to
+a unique constant length prefix. Due to this all non-primary indexes need
+to be dropped and re-created with a new name. The changes to the database
+will be performed by the Upgrade Wizard in the Install Tool.
index 048d5fe..bfb8564 100644 (file)
@@ -2718,7 +2718,7 @@ class DatabaseConnection extends \TYPO3\CMS\Core\Database\DatabaseConnection {
                                        foreach ($keyRows as $k => $theKey) {
                                                $theKey['Table'] = $tableName;
                                                $theKey['Non_unique'] = (int)(!$theKey['unique']);
-                                               $theKey['Key_name'] = str_replace($tableName . '_', '', $k);
+                                               $theKey['Key_name'] = str_replace(hash('crc32b', $tableName) . '_', '', $k);
                                                // the following are probably not needed anyway...
                                                $theKey['Collation'] = '';
                                                $theKey['Cardinality'] = '';
index 74e49e4..cc85743 100644 (file)
@@ -25,6 +25,7 @@ abstract class AbstractSpecifics {
        const TABLE_MAXLENGTH = 'table_maxlength';
        const FIELD_MAXLENGTH = 'field_maxlength';
        const LIST_MAXEXPRESSIONS = 'list_maxexpressions';
+       const PARTIAL_STRING_INDEX = 'partial_string_index';
 
        /**
         * Contains the specifics of a DBMS.
index 912cda1..0c234df 100644 (file)
@@ -287,10 +287,10 @@ class SqlParser extends \TYPO3\CMS\Core\Database\SqlParser {
                                                        }
                                                } elseif ($kN === 'UNIQUE') {
                                                        foreach ($kCfg as $n => $field) {
-                                                               $indexKeys = array_merge($indexKeys, $this->databaseConnection->handlerInstance[$this->databaseConnection->handler_getFromTableList($components['TABLE'])]->DataDictionary->CreateIndexSQL($n, $components['TABLE'], $field, array('UNIQUE')));
+                                                               $indexKeys = array_merge($indexKeys, $this->compileCREATEINDEX($n, $components['TABLE'], $field, array('UNIQUE')));
                                                        }
                                                } else {
-                                                       $indexKeys = array_merge($indexKeys, $this->databaseConnection->handlerInstance[$this->databaseConnection->handler_getFromTableList($components['TABLE'])]->DataDictionary->CreateIndexSQL($components['TABLE'] . '_' . $kN, $components['TABLE'], $kCfg));
+                                                       $indexKeys = array_merge($indexKeys, $this->compileCREATEINDEX($kN, $components['TABLE'], $kCfg));
                                                }
                                        }
                                }
@@ -331,13 +331,17 @@ class SqlParser extends \TYPO3\CMS\Core\Database\SqlParser {
                                        case 'DROP':
 
                                        case 'DROPKEY':
+                                               $query = $this->compileDROPINDEX($components['KEY'], $components['TABLE']);
                                                break;
-                                       case 'ADDKEY':
-
-                                       case 'ADDPRIMARYKEY':
 
+                                       case 'ADDKEY':
+                                               $query = $this->compileCREATEINDEX($components['KEY'], $components['TABLE'], $components['fields']);
+                                               break;
                                        case 'ADDUNIQUE':
-                                               $query .= ' (' . implode(',', $components['fields']) . ')';
+                                               $query = $this->compileCREATEINDEX($components['KEY'], $components['TABLE'], $components['fields'], array('UNIQUE'));
+                                               break;
+                                       case 'ADDPRIMARYKEY':
+                                               // @todo ???
                                                break;
                                        case 'DEFAULTCHARACTERSET':
 
@@ -351,6 +355,56 @@ class SqlParser extends \TYPO3\CMS\Core\Database\SqlParser {
        }
 
        /**
+        * Compiles CREATE INDEX statements from component information
+        *
+        * MySQL only needs uniqueness of index names per table, but many DBMS require uniqueness of index names per schema.
+        * The table name is hashed and prepended to the index name to make sure index names are unique.
+        *
+        * @param string $indexName
+        * @param string $tableName
+        * @param array $indexFields
+        * @param array $indexOptions
+        * @return array
+        * @see compileALTERTABLE()
+        */
+       public function compileCREATEINDEX($indexName, $tableName, $indexFields, $indexOptions = array()) {
+               $indexIdentifier = $this->databaseConnection->quoteName(hash('crc32b', $tableName) . '_' . $indexName, NULL, TRUE);
+               $dbmsSpecifics = $this->databaseConnection->getSpecifics();
+               $keepFieldLengths = $dbmsSpecifics->specificExists(Specifics\AbstractSpecifics::PARTIAL_STRING_INDEX) && $dbmsSpecifics->getSpecific(Specifics\AbstractSpecifics::PARTIAL_STRING_INDEX);
+
+               foreach ($indexFields as $key => $fieldName) {
+                       if (!$keepFieldLengths) {
+                               $fieldName = preg_replace('/\A([^\(]+)(\(\d+\))/', '\\1', $fieldName);
+                       }
+                       // Quote the fieldName in backticks with escaping, ADOdb will replace the backticks with the correct quoting
+                       $indexFields[$key] = '`' . str_replace('`', '``', $fieldName) . '`';
+               }
+
+               return $this->databaseConnection->handlerInstance[$this->databaseConnection->handler_getFromTableList($tableName)]->DataDictionary->CreateIndexSQL(
+                       $indexIdentifier, $this->databaseConnection->quoteName($tableName, NULL, TRUE), $indexFields, $indexOptions
+               );
+       }
+
+       /**
+        * Compiles DROP INDEX statements from component information
+        *
+        * MySQL only needs uniqueness of index names per table, but many DBMS require uniqueness of index names per schema.
+        * The table name is hashed and prepended to the index name to make sure index names are unique.
+        *
+        * @param $indexName
+        * @param $tableName
+        * @return array
+        * @see compileALTERTABLE()
+        */
+       public function compileDROPINDEX($indexName, $tableName) {
+               $indexIdentifier = $this->databaseConnection->quoteName(hash('crc32b', $tableName) . '_' . $indexName, NULL, TRUE);
+
+               return $this->databaseConnection->handlerInstance[$this->databaseConnection->handler_getFromTableList($tableName)]->DataDictionary->DropIndexSQL(
+                       $indexIdentifier, $this->databaseConnection->quoteName($tableName)
+               );
+       }
+
+       /**
         * Compile field definition
         *
         * @param array $fieldCfg Field definition parts
index f6a039e..fedffc0 100644 (file)
@@ -144,4 +144,32 @@ class DatabaseConnectionPostgresqlTest extends AbstractTestCase {
                $this->assertEquals($expected, $this->cleanSql($result));
        }
 
+       /**
+        * @test
+        * @see http://forge.typo3.org/issues/67445
+        */
+       public function alterTableAddKeyStatementIsRemappedToCreateIndex() {
+               $parseString = 'ALTER TABLE sys_collection ADD KEY parent (pid,deleted)';
+               $components = $this->subject->SQLparser->_callRef('parseALTERTABLE', $parseString);
+               $this->assertInternalType('array', $components);
+
+               $result = $this->subject->SQLparser->_callRef('compileALTERTABLE', $components);
+               $expected = array('CREATE INDEX "dd81ee97_parent" ON "sys_collection" ("pid", "deleted")');
+               $this->assertSame($expected, $this->cleanSql($result));
+       }
+
+       /**
+        * @test
+        * @see http://forge.typo3.org/issues/67445
+        */
+       public function canParseAlterTableDropKeyStatement() {
+               $parseString = 'ALTER TABLE sys_collection DROP KEY parent';
+               $components = $this->subject->SQLparser->_callRef('parseALTERTABLE', $parseString);
+               $this->assertInternalType('array', $components);
+
+               $result = $this->subject->SQLparser->_callRef('compileALTERTABLE', $components);
+               $expected = array('DROP INDEX "dd81ee97_parent"');
+               $this->assertSame($expected, $this->cleanSql($result));
+       }
+
 }
index 65563fb..b530931 100644 (file)
@@ -299,6 +299,34 @@ class SqlParserTest extends AbstractTestCase {
 
        /**
         * @test
+        * @see http://forge.typo3.org/issues/67445
+        */
+       public function canParseAlterTableAddKeyStatement() {
+               $parseString = 'ALTER TABLE sys_collection ADD KEY parent (pid,deleted)';
+               $components = $this->subject->_callRef('parseALTERTABLE', $parseString);
+               $this->assertInternalType('array', $components);
+
+               $result = $this->subject->_callRef('compileALTERTABLE', $components);
+               $expected = 'ALTER TABLE sys_collection ADD KEY parent (pid,deleted)';
+               $this->assertSame($expected, $this->cleanSql($result));
+       }
+
+       /**
+        * @test
+        * @see http://forge.typo3.org/issues/67445
+        */
+       public function canParseAlterTableDropKeyStatement() {
+               $parseString = 'ALTER TABLE sys_collection DROP KEY parent';
+               $components = $this->subject->_callRef('parseALTERTABLE', $parseString);
+               $this->assertInternalType('array', $components);
+
+               $result = $this->subject->_callRef('compileALTERTABLE', $components);
+               $expected = 'ALTER TABLE sys_collection DROP KEY parent';
+               $this->assertSame($expected, $this->cleanSql($result));
+       }
+
+       /**
+        * @test
         * @see http://forge.typo3.org/issues/23087
         */
        public function canParseFindInSetStatement() {
@@ -712,4 +740,4 @@ class SqlParserTest extends AbstractTestCase {
                $this->assertEquals($expected, $this->cleanSql($result));
        }
 
-}
\ No newline at end of file
+}