From 032089afbe2455ba8c1e1df397d71c7dc9236c8f Mon Sep 17 00:00:00 2001 From: Lee Rowlands Date: Fri, 31 Mar 2023 18:15:16 +1000 Subject: [PATCH] Issue #1148856 by drunken monkey, stefan.r, bzrudi71, xatoo, Ben Coleman, jyotisankar, daffie, mondrake, andypost, Damien Tournoud, alexpott: Postgres schema doesn't support keylength on a unique index --- .../lib/Drupal/Core/Database/database.api.php | 3 + .../src/Driver/Database/pgsql/Schema.php | 14 +- .../SchemaUniquePrefixedKeysIndexTest.php | 135 ++++++++++++++++++ 3 files changed, 148 insertions(+), 4 deletions(-) create mode 100644 core/tests/Drupal/KernelTests/Core/Database/SchemaUniquePrefixedKeysIndexTest.php diff --git a/core/lib/Drupal/Core/Database/database.api.php b/core/lib/Drupal/Core/Database/database.api.php index ab704108873..e4e5a0b9be0 100644 --- a/core/lib/Drupal/Core/Database/database.api.php +++ b/core/lib/Drupal/Core/Database/database.api.php @@ -340,6 +340,9 @@ use Drupal\Core\Database\Query\SelectInterface; * * A key column specifier is either a string naming a column or an array of two * elements, column name and length, specifying a prefix of the named column. + * Note that some DBMS drivers may opt to ignore the prefix length configuration + * and still use the whole field value for the key. Code should therefore not + * rely on this functionality. * * As an example, this is the schema definition for the 'users_data' table. It * shows five fields ('uid', 'module', 'name', 'value', and 'serialized'), the diff --git a/core/modules/pgsql/src/Driver/Database/pgsql/Schema.php b/core/modules/pgsql/src/Driver/Database/pgsql/Schema.php index 4843e076f08..ed852c622ba 100644 --- a/core/modules/pgsql/src/Driver/Database/pgsql/Schema.php +++ b/core/modules/pgsql/src/Driver/Database/pgsql/Schema.php @@ -286,7 +286,11 @@ EOD; } if (isset($table['unique keys']) && is_array($table['unique keys'])) { foreach ($table['unique keys'] as $key_name => $key) { - $sql_keys[] = 'CONSTRAINT ' . $this->ensureIdentifiersLength($name, $key_name, 'key') . ' UNIQUE (' . implode(', ', $key) . ')'; + // Use the createPrimaryKeySql(), which already discards any prefix + // lengths passed as part of the key column specifiers. (Postgres + // doesn't support setting a prefix length for PRIMARY or UNIQUE + // indices.) + $sql_keys[] = 'CONSTRAINT ' . $this->ensureIdentifiersLength($name, $key_name, 'key') . ' UNIQUE (' . $this->createPrimaryKeySql($key) . ')'; } } @@ -471,7 +475,7 @@ EOD; } /** - * Create the SQL expression for primary keys. + * Create the SQL expression for primary and unique keys. * * Postgresql does not support key length. It does support fillfactor, but * that requires a separate database lookup for each column in the key. The @@ -794,8 +798,10 @@ EOD; throw new SchemaObjectExistsException("Cannot add unique key '$name' to table '$table': unique key already exists."); } - $fields = array_map([$this->connection, 'escapeField'], $fields); - $this->connection->query('ALTER TABLE {' . $table . '} ADD CONSTRAINT ' . $this->ensureIdentifiersLength($table, $name, 'key') . ' UNIQUE (' . implode(',', $fields) . ')'); + // Use the createPrimaryKeySql(), which already discards any prefix lengths + // passed as part of the key column specifiers. (Postgres doesn't support + // setting a prefix length for PRIMARY or UNIQUE indices.) + $this->connection->query('ALTER TABLE {' . $table . '} ADD CONSTRAINT ' . $this->ensureIdentifiersLength($table, $name, 'key') . ' UNIQUE (' . $this->createPrimaryKeySql($fields) . ')'); $this->resetTableInformation($table); } diff --git a/core/tests/Drupal/KernelTests/Core/Database/SchemaUniquePrefixedKeysIndexTest.php b/core/tests/Drupal/KernelTests/Core/Database/SchemaUniquePrefixedKeysIndexTest.php new file mode 100644 index 00000000000..b21543c13cd --- /dev/null +++ b/core/tests/Drupal/KernelTests/Core/Database/SchemaUniquePrefixedKeysIndexTest.php @@ -0,0 +1,135 @@ +connection->schema()->createTable('test_unique', [ + 'fields' => [ + 'field' => [ + 'type' => 'varchar', + 'length' => 50, + ], + ], + 'unique keys' => [ + 'field' => [['field', 10]], + ], + ]); + + $this->checkUniqueConstraintException('test_unique', 'field'); + } + + /** + * Tests adding a UNIQUE key to an existing table. + * + * @covers ::addUniqueKey + */ + public function testAddUniqueKey(): void { + $this->connection->schema() + ->addUniqueKey('test_people', 'job', [['job', 10]]); + + $this->checkUniqueConstraintException('test_people', 'job'); + } + + /** + * Tests adding a new field with UNIQUE key. + * + * @covers ::addField + */ + public function testAddField(): void { + $field_spec = [ + 'type' => 'varchar', + 'length' => 50, + ]; + $keys_spec = [ + 'unique keys' => [ + 'field' => [['field', 10]], + ], + ]; + $this->connection->schema() + ->addField('test', 'field', $field_spec, $keys_spec); + + $this->checkUniqueConstraintException('test', 'field'); + } + + /** + * Tests changing a field to add a UNIQUE key. + * + * @covers ::changeField + */ + public function testChangeField(): void { + $field_spec = [ + 'description' => "The person's job", + 'type' => 'varchar_ascii', + 'length' => 50, + 'not null' => TRUE, + 'default' => '', + ]; + $keys_spec = [ + 'unique keys' => [ + 'job' => [['job', 10]], + ], + ]; + $this->connection->schema() + ->changeField('test_people', 'job', 'job', $field_spec, $keys_spec); + + $this->checkUniqueConstraintException('test_people', 'job'); + } + + /** + * Verifies that inserting the same value/prefix twice causes an exception. + * + * @param string $table + * The table to insert into. + * @param string $column + * The column on that table that has a UNIQUE index. If prefix lengths are + * accepted for UNIQUE keys on the current database, the prefix length for + * the field is expected to be set to 10 characters. + */ + protected function checkUniqueConstraintException(string $table, string $column): void { + $this->connection->insert($table) + ->fields([ + $column => '1234567890 foo', + ]) + ->execute(); + + $this->expectException(DatabaseException::class); + $value = '1234567890 ' . ($this->supportsPrefixLength() ? 'bar' : 'foo'); + $this->connection->insert($table) + ->fields([ + $column => $value, + ]) + ->execute(); + } + + /** + * Determines whether the current database supports prefix lengths for keys. + * + * The basic syntax of passing an array (field, prefix length) as a key column + * specifier must always be accepted by the driver. However, due to technical + * limitations, some drivers may choose to ignore them. + * + * @return bool + * TRUE if the current database (driver) will conform to the prefix length + * specified as part of a key column specifier, FALSE if it will be ignored. + */ + protected function supportsPrefixLength(): bool { + return $this->connection->driver() === 'mysql'; + } + +}