From fdacae95f33c1504f93fb2f592d649adcbe80b59 Mon Sep 17 00:00:00 2001
From: Lee Rowlands <lee.rowlands@previousnext.com.au>
Date: Fri, 22 Dec 2023 13:31:17 +1000
Subject: [PATCH] Issue #3159113 by daffie, saxenaakansha30, quietone:
PostgreSQL Duplicate table error for alter index queries
---
core/lib/Drupal/Core/Database/Schema.php | 6 +-
.../Unit/MigrationConfigurationTraitTest.php | 36 ++++++------
.../src/Driver/Database/pgsql/Schema.php | 10 +++-
.../tests/src/Kernel/pgsql/SchemaTest.php | 56 +++++++++++++++++++
.../src/Driver/Database/sqlite/Schema.php | 4 +-
5 files changed, 87 insertions(+), 25 deletions(-)
diff --git a/core/lib/Drupal/Core/Database/Schema.php b/core/lib/Drupal/Core/Database/Schema.php
index 9153cd9ef8f0..9530c8fbf86b 100644
--- a/core/lib/Drupal/Core/Database/Schema.php
+++ b/core/lib/Drupal/Core/Database/Schema.php
@@ -161,12 +161,14 @@ protected function buildTableNameCondition($table_name, $operator = '=', $add_pr
*
* @param $table
* The name of the table in drupal (no prefixing).
+ * @param bool $add_prefix
+ * Boolean to indicate whether the table name needs to be prefixed.
*
* @return bool
* TRUE if the given table exists, otherwise FALSE.
*/
- public function tableExists($table) {
- $condition = $this->buildTableNameCondition($table);
+ public function tableExists($table, bool $add_prefix = TRUE) {
+ $condition = $this->buildTableNameCondition($table, '=', $add_prefix);
$condition->compile($this->connection, $this);
// Normally, we would heartily discourage the use of string
// concatenation for conditionals like this however, we
diff --git a/core/modules/migrate_drupal/tests/src/Unit/MigrationConfigurationTraitTest.php b/core/modules/migrate_drupal/tests/src/Unit/MigrationConfigurationTraitTest.php
index 4ebbfc47d196..3e6e052147f1 100644
--- a/core/modules/migrate_drupal/tests/src/Unit/MigrationConfigurationTraitTest.php
+++ b/core/modules/migrate_drupal/tests/src/Unit/MigrationConfigurationTraitTest.php
@@ -62,8 +62,8 @@ public function providerTestGetLegacyDrupalVersion() {
'schema_version' => '1678',
'exception' => NULL,
'table_map' => [
- ['system', TRUE],
- ['key_value', FALSE],
+ ['system', TRUE, TRUE],
+ ['key_value', TRUE, FALSE],
],
],
'D6' => [
@@ -71,8 +71,8 @@ public function providerTestGetLegacyDrupalVersion() {
'schema_version' => '6057',
'exception' => NULL,
'table_map' => [
- ['system', TRUE],
- ['key_value', FALSE],
+ ['system', TRUE, TRUE],
+ ['key_value', TRUE, FALSE],
],
],
'D7' => [
@@ -80,8 +80,8 @@ public function providerTestGetLegacyDrupalVersion() {
'schema_version' => '7065',
'exception' => NULL,
'table_map' => [
- ['system', TRUE],
- ['key_value', FALSE],
+ ['system', TRUE, TRUE],
+ ['key_value', TRUE, FALSE],
],
],
'D8' => [
@@ -89,8 +89,8 @@ public function providerTestGetLegacyDrupalVersion() {
'schema_version' => serialize('8976'),
'exception' => NULL,
'table_map' => [
- ['system', FALSE],
- ['key_value', TRUE],
+ ['system', TRUE, FALSE],
+ ['key_value', TRUE, TRUE],
],
],
'D9' => [
@@ -98,8 +98,8 @@ public function providerTestGetLegacyDrupalVersion() {
'schema_version' => serialize('9270'),
'exception' => NULL,
'table_map' => [
- ['system', FALSE],
- ['key_value', TRUE],
+ ['system', TRUE, FALSE],
+ ['key_value', TRUE, TRUE],
],
],
'Not drupal' => [
@@ -107,8 +107,8 @@ public function providerTestGetLegacyDrupalVersion() {
'schema_version' => "not drupal I guess",
'exception' => NULL,
'table_map' => [
- ['system', FALSE],
- ['key_value', FALSE],
+ ['system', TRUE, FALSE],
+ ['key_value', TRUE, FALSE],
],
],
'D5 almost' => [
@@ -116,8 +116,8 @@ public function providerTestGetLegacyDrupalVersion() {
'schema_version' => '123',
'exception' => NULL,
'table_map' => [
- ['system', TRUE],
- ['key_value', FALSE],
+ ['system', TRUE, TRUE],
+ ['key_value', TRUE, FALSE],
],
],
'D5/6/7 Exception' => [
@@ -125,8 +125,8 @@ public function providerTestGetLegacyDrupalVersion() {
'schema_version' => NULL,
'exception' => new DatabaseExceptionWrapper(),
'table_map' => [
- ['system', TRUE],
- ['key_value', FALSE],
+ ['system', TRUE, TRUE],
+ ['key_value', TRUE, FALSE],
],
],
'D8/9 Exception' => [
@@ -134,8 +134,8 @@ public function providerTestGetLegacyDrupalVersion() {
'schema_version' => NULL,
'exception' => new DatabaseExceptionWrapper(),
'table_map' => [
- ['system', FALSE],
- ['key_value', TRUE],
+ ['system', TRUE, FALSE],
+ ['key_value', TRUE, TRUE],
],
],
];
diff --git a/core/modules/pgsql/src/Driver/Database/pgsql/Schema.php b/core/modules/pgsql/src/Driver/Database/pgsql/Schema.php
index 62515e8caff6..b7e05f1cc18d 100644
--- a/core/modules/pgsql/src/Driver/Database/pgsql/Schema.php
+++ b/core/modules/pgsql/src/Driver/Database/pgsql/Schema.php
@@ -506,8 +506,8 @@ protected function createPrimaryKeySql($fields) {
/**
* {@inheritdoc}
*/
- public function tableExists($table) {
- $prefixInfo = $this->getPrefixInfo($table, TRUE);
+ public function tableExists($table, $add_prefix = TRUE) {
+ $prefixInfo = $this->getPrefixInfo($table, $add_prefix);
return (bool) $this->connection->query("SELECT 1 FROM pg_tables WHERE schemaname = :schema AND tablename = :table", [':schema' => $prefixInfo['schema'], ':table' => $prefixInfo['table']])->fetchField();
}
@@ -586,7 +586,11 @@ public function renameTable($table, $new_name) {
preg_match('/^' . preg_quote($table_name) . '__(.*)__' . preg_quote($index_type) . '/', $index->indexname, $matches);
$index_name = $matches[1];
}
- $this->connection->query('ALTER INDEX "' . $this->defaultSchema . '"."' . $index->indexname . '" RENAME TO ' . $this->ensureIdentifiersLength($new_name, $index_name, $index_type));
+ // The renaming of an index will fail when the there exists an table with
+ // the same name as the renamed index.
+ if (!$this->tableExists($this->ensureIdentifiersLength($new_name, $index_name, $index_type), FALSE)) {
+ $this->connection->query('ALTER INDEX "' . $this->defaultSchema . '"."' . $index->indexname . '" RENAME TO ' . $this->ensureIdentifiersLength($new_name, $index_name, $index_type));
+ }
}
// Ensure the new table name does not include schema syntax.
diff --git a/core/modules/pgsql/tests/src/Kernel/pgsql/SchemaTest.php b/core/modules/pgsql/tests/src/Kernel/pgsql/SchemaTest.php
index bbf05ab0abf8..2de31456cbe8 100644
--- a/core/modules/pgsql/tests/src/Kernel/pgsql/SchemaTest.php
+++ b/core/modules/pgsql/tests/src/Kernel/pgsql/SchemaTest.php
@@ -307,4 +307,60 @@ public function testPgsqlSequences(): void {
}
+ /**
+ * Tests the method tableExists().
+ */
+ public function testTableExists() {
+ $table_name = 'test_table';
+ $table_specification = [
+ 'fields' => [
+ 'id' => [
+ 'type' => 'int',
+ 'default' => NULL,
+ ],
+ ],
+ ];
+ $this->schema->createTable($table_name, $table_specification);
+ $prefixed_table_name = $this->connection->getPrefix($table_name) . $table_name;
+
+ // Three different calls to the method Schema::tableExists() with an
+ // unprefixed table name.
+ $this->assertTrue($this->schema->tableExists($table_name));
+ $this->assertTrue($this->schema->tableExists($table_name, TRUE));
+ $this->assertFalse($this->schema->tableExists($table_name, FALSE));
+
+ // Three different calls to the method Schema::tableExists() with a
+ // prefixed table name.
+ $this->assertFalse($this->schema->tableExists($prefixed_table_name));
+ $this->assertFalse($this->schema->tableExists($prefixed_table_name, TRUE));
+ $this->assertTrue($this->schema->tableExists($prefixed_table_name, FALSE));
+ }
+
+ /**
+ * Tests renaming a table where the new index name is equal to the table name.
+ */
+ public function testRenameTableWithNewIndexNameEqualsTableName() {
+ // Special table names for colliding with the PostgreSQL new index name.
+ $table_name_old = 'some_new_table_name__id__idx';
+ $table_name_new = 'some_new_table_name';
+ $table_specification = [
+ 'fields' => [
+ 'id' => [
+ 'type' => 'int',
+ 'default' => NULL,
+ ],
+ ],
+ 'indexes' => [
+ 'id' => ['id'],
+ ],
+ ];
+ $this->schema->createTable($table_name_old, $table_specification);
+
+ // Renaming the table can fail for PostgreSQL, when a new index name is
+ // equal to the old table name.
+ $this->schema->renameTable($table_name_old, $table_name_new);
+
+ $this->assertTrue($this->schema->tableExists($table_name_new));
+ }
+
}
diff --git a/core/modules/sqlite/src/Driver/Database/sqlite/Schema.php b/core/modules/sqlite/src/Driver/Database/sqlite/Schema.php
index eb836eb45363..64bdffa8c804 100644
--- a/core/modules/sqlite/src/Driver/Database/sqlite/Schema.php
+++ b/core/modules/sqlite/src/Driver/Database/sqlite/Schema.php
@@ -28,8 +28,8 @@ class Schema extends DatabaseSchema {
/**
* {@inheritdoc}
*/
- public function tableExists($table) {
- $info = $this->getPrefixInfo($table);
+ public function tableExists($table, $add_prefix = TRUE) {
+ $info = $this->getPrefixInfo($table, $add_prefix);
// Don't use {} around sqlite_master table.
return (bool) $this->connection->query('SELECT 1 FROM [' . $info['schema'] . '].sqlite_master WHERE type = :type AND name = :name', [':type' => 'table', ':name' => $info['table']])->fetchField();
--
GitLab