doctrine/dbal

AbstractMySQLPlatform::buildTableOptions() uses "collate", but everywhere else DBAL uses "collation"

mlocati opened this issue · 4 comments

Feature Request

Q A
New Feature yes
RFC no
BC Break no

Summary

DBAL uses almost everywhere collation to identify the collation, except in AbstractMySQLPlatform::buildTableOptions() where collate` is used:

$ grep -rE "['\"](collate|collation)['\"]" .
./src/Platforms/AbstractMySQLPlatform.php:        if (! isset($options['collate'])) {
./src/Platforms/AbstractMySQLPlatform.php:            $options['collate'] = $options['charset'] . '_unicode_ci';
./src/Platforms/AbstractMySQLPlatform.php:        $tableOptions[] = $this->getColumnCollationDeclarationSQL($options['collate']);
./src/Platforms/AbstractPlatform.php:            $collation = ! empty($column['collation']) ?
./src/Platforms/AbstractPlatform.php:                ' ' . $this->getColumnCollationDeclarationSQL($column['collation']) : '';
./src/Platforms/MySQL/Comparator.php:            'collation' => null,
./src/Platforms/SQLite/Comparator.php:            if (! isset($options['collation']) || strcasecmp($options['collation'], 'binary') !== 0) {
./src/Platforms/SQLite/Comparator.php:            unset($options['collation']);
./src/Platforms/SQLServer/Comparator.php:            if (! isset($options['collation']) || $options['collation'] !== $this->databaseCollation) {
./src/Platforms/SQLServer/Comparator.php:            unset($options['collation']);
./src/Platforms/SQLServerPlatform.php:            $collation = ! empty($column['collation']) ?
./src/Platforms/SQLServerPlatform.php:                ' ' . $this->getColumnCollationDeclarationSQL($column['collation']) : '';
./src/Schema/MySQLSchemaManager.php:        if (isset($tableColumn['collation'])) {
./src/Schema/MySQLSchemaManager.php:            $column->setPlatformOption('collation', $tableColumn['collation']);
./src/Schema/MySQLSchemaManager.php:            $table->addOption('collation', $tableOptions['TABLE_COLLATION']);
./src/Schema/PostgreSQLSchemaManager.php:        if (isset($tableColumn['collation']) && ! empty($tableColumn['collation'])) {
./src/Schema/PostgreSQLSchemaManager.php:            $column->setPlatformOption('collation', $tableColumn['collation']);
./src/Schema/SqliteSchemaManager.php:                    'collation',
./src/Schema/SQLServerSchemaManager.php:        if (isset($tableColumn['collation']) && $tableColumn['collation'] !== 'NULL') {
./src/Schema/SQLServerSchemaManager.php:            $column->setPlatformOption('collation', $tableColumn['collation']);

For example, MySQLSchemaManager::listTableDetails() sets the collation key in the table options. And creating a new table with the same collation as an existing table requires to copy the value of the collation key of the existing table options to the collate key of the new table options.

In order to fix this in a backward compatible way we could change the following code of AbstractMySQLPlatform::buildTableOptions():

        if (! isset($options['collate'])) {
            $options['collate'] = $options['charset'] . '_unicode_ci';
        }

to

        if (! isset($options['collate'])) {
            if (isset($options['collation'])) {
                $options['collate'] = $options['collation'];
            } else {
                $options['collate'] = $options['charset'] . '_unicode_ci';
            }
        }

Both the existing and the proposed APIs are error-prone: it is easy to make a typo in the option and waste time debugging your code. I've been there myself.

A proper API should use some better-defined structures for platform-specific options than array<string,mixed>. For instance, class MySQLTableOptions which could be instantiated from the metadata and would validate it.

The options array is passed around, so this does not seem doable without a BC-break or without converting the structure back to an array immediately after building it:

$eventArgs = new SchemaCreateTableEventArgs($table, $columns, $options, $this);

$sql = $this->_getCreateTableSQL($tableName, $columns, $options);

Possible steps forward:

  • fixing this in 3.3.x
  • deprecating collate in favor of collation in 3.4.x
  • using more precise phpdoc
  • using objects in 4.0.x

As it appears in #5243 (comment), the existing behavior of the MySQL platform expecting "collate" is more of a bug rather than something that needs improvement.

deprecating collate in favor of collation in 3.4.x

We can introduce the support for "collation" in 3.3.x and deprecate "collate" in 3.4.x.

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.