Issue #3159113 by daffie, saxenaakansha30, quietone: PostgreSQL Duplicate...

Issue #3159113 by daffie, saxenaakansha30, quietone: PostgreSQL Duplicate table error for alter index queries

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

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],
         ],
       ],
     ];

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.

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));
+  }
+
 }

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();