Fix

KodaiD · KodaiD · commit a63f5d8cf7d3 · 2025-09-08T08:11:59.000+09:00
diff --git a/core/src/integration-test/java/com/scalar/db/storage/cosmos/ConsensusCommitAdminIntegrationTestWithCosmos.java b/core/src/integration-test/java/com/scalar/db/storage/cosmos/ConsensusCommitAdminIntegrationTestWithCosmos.java
@@ -62,5 +62,9 @@ public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentExceptio
 
   @Override
   @Disabled("Cosmos DB does not support renaming columns")
-  public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {}
+  public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly() {}
+
+  @Override
+  @Disabled("Cosmos DB does not support renaming columns")
+  public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() {}
 }
diff --git a/core/src/integration-test/java/com/scalar/db/storage/cosmos/CosmosAdminCaseSensitivityIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/cosmos/CosmosAdminCaseSensitivityIntegrationTest.java
@@ -62,5 +62,9 @@ public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentExceptio
 
   @Override
   @Disabled("Cosmos DB does not support renaming columns")
-  public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {}
+  public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly() {}
+
+  @Override
+  @Disabled("Cosmos DB does not support renaming columns")
+  public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() {}
 }
diff --git a/core/src/integration-test/java/com/scalar/db/storage/cosmos/CosmosAdminIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/cosmos/CosmosAdminIntegrationTest.java
@@ -61,5 +61,9 @@ public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentExceptio
 
   @Override
   @Disabled("Cosmos DB does not support renaming columns")
-  public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {}
+  public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly() {}
+
+  @Override
+  @Disabled("Cosmos DB does not support renaming columns")
+  public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() {}
 }
diff --git a/core/src/integration-test/java/com/scalar/db/storage/cosmos/SingleCrudOperationTransactionAdminIntegrationTestWithCosmos.java b/core/src/integration-test/java/com/scalar/db/storage/cosmos/SingleCrudOperationTransactionAdminIntegrationTestWithCosmos.java
@@ -56,5 +56,9 @@ public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentExceptio
 
   @Override
   @Disabled("Cosmos DB does not support renaming columns")
-  public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {}
+  public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly() {}
+
+  @Override
+  @Disabled("Cosmos DB does not support renaming columns")
+  public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() {}
 }
diff --git a/core/src/integration-test/java/com/scalar/db/storage/dynamo/ConsensusCommitAdminIntegrationTestWithDynamo.java b/core/src/integration-test/java/com/scalar/db/storage/dynamo/ConsensusCommitAdminIntegrationTestWithDynamo.java
@@ -67,5 +67,9 @@ public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentExceptio
 
   @Override
   @Disabled("DynamoDB does not support renaming columns")
-  public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {}
+  public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly() {}
+
+  @Override
+  @Disabled("DynamoDB does not support renaming columns")
+  public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() {}
 }
diff --git a/core/src/integration-test/java/com/scalar/db/storage/dynamo/DynamoAdminCaseSensitivityIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/dynamo/DynamoAdminCaseSensitivityIntegrationTest.java
@@ -67,5 +67,9 @@ public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentExceptio
 
   @Override
   @Disabled("DynamoDB does not support renaming columns")
-  public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {}
+  public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly() {}
+
+  @Override
+  @Disabled("DynamoDB does not support renaming columns")
+  public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() {}
 }
diff --git a/core/src/integration-test/java/com/scalar/db/storage/dynamo/DynamoAdminIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/dynamo/DynamoAdminIntegrationTest.java
@@ -66,5 +66,9 @@ public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentExceptio
 
   @Override
   @Disabled("DynamoDB does not support renaming columns")
-  public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {}
+  public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly() {}
+
+  @Override
+  @Disabled("DynamoDB does not support renaming columns")
+  public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() {}
 }
diff --git a/core/src/integration-test/java/com/scalar/db/storage/dynamo/SingleCrudOperationTransactionAdminIntegrationTestWithDynamo.java b/core/src/integration-test/java/com/scalar/db/storage/dynamo/SingleCrudOperationTransactionAdminIntegrationTestWithDynamo.java
@@ -61,5 +61,9 @@ public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentExceptio
 
   @Override
   @Disabled("DynamoDB does not support renaming columns")
-  public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {}
+  public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly() {}
+
+  @Override
+  @Disabled("DynamoDB does not support renaming columns")
+  public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() {}
 }
diff --git a/core/src/main/java/com/scalar/db/common/CheckedDistributedStorageAdmin.java b/core/src/main/java/com/scalar/db/common/CheckedDistributedStorageAdmin.java
@@ -353,14 +353,6 @@ public void renameColumn(
               ScalarDbUtils.getFullTableName(namespace, table), newColumnName));
     }
 
-    if (tableMetadata.getPartitionKeyNames().contains(oldColumnName)
-        || tableMetadata.getClusteringKeyNames().contains(oldColumnName)
-        || tableMetadata.getSecondaryIndexNames().contains(oldColumnName)) {
-      throw new IllegalArgumentException(
-          CoreError.RENAME_PRIMARY_KEY_COLUMN_NOT_SUPPORTED.buildMessage(
-              ScalarDbUtils.getFullTableName(namespace, table), oldColumnName));
-    }
-
     try {
       admin.renameColumn(namespace, table, oldColumnName, newColumnName);
     } catch (ExecutionException e) {
diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java b/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java
@@ -891,15 +891,34 @@ public void renameColumn(
     try {
       TableMetadata currentTableMetadata = getTableMetadata(namespace, table);
       DataType columnType = currentTableMetadata.getColumnDataType(oldColumnName);
-      TableMetadata updatedTableMetadata =
+      TableMetadata.Builder tableMetadataBuilder =
           TableMetadata.newBuilder(currentTableMetadata)
               .removeColumn(oldColumnName)
-              .addColumn(newColumnName, columnType)
-              .build();
+              .addColumn(newColumnName, columnType);
+      if (currentTableMetadata.getPartitionKeyNames().contains(oldColumnName)) {
+        tableMetadataBuilder.removePartitionKey(oldColumnName);
+        tableMetadataBuilder.addPartitionKey(newColumnName);
+      } else if (currentTableMetadata.getClusteringKeyNames().contains(oldColumnName)) {
+        Ordering.Order order = currentTableMetadata.getClusteringOrder(oldColumnName);
+        tableMetadataBuilder.removeClusteringKey(oldColumnName);
+        tableMetadataBuilder.addClusteringKey(newColumnName, order);
+      } else if (currentTableMetadata.getSecondaryIndexNames().contains(oldColumnName)) {
+        tableMetadataBuilder.removeSecondaryIndex(oldColumnName);
+        tableMetadataBuilder.addSecondaryIndex(newColumnName);
+      }
+      TableMetadata updatedTableMetadata = tableMetadataBuilder.build();
       String renameColumnStatement =
           rdbEngine.renameColumnSql(namespace, table, oldColumnName, newColumnName);
       try (Connection connection = dataSource.getConnection()) {
         execute(connection, renameColumnStatement);
+        if (currentTableMetadata.getSecondaryIndexNames().contains(oldColumnName)) {
+          if (rdbEngine instanceof RdbEngineSqlite) {
+            dropIndex(namespace, table, oldColumnName);
+            createIndex(connection, namespace, table, newColumnName, false);
+          } else {
+            renameIndex(connection, namespace, table, oldColumnName, newColumnName);
+          }
+        }
         addTableMetadata(connection, namespace, table, updatedTableMetadata, false, true);
       }
     } catch (SQLException e) {
@@ -968,6 +987,19 @@ private void dropIndex(Connection connection, String schema, String table, Strin
     execute(connection, sql);
   }
 
+  private void renameIndex(
+      Connection connection,
+      String schema,
+      String table,
+      String oldIndexedColumn,
+      String newIndexedColumn)
+      throws SQLException {
+    String oldIndexName = getIndexName(schema, table, oldIndexedColumn);
+    String newIndexName = getIndexName(schema, table, newIndexedColumn);
+    String sql = rdbEngine.renameIndexSql(schema, table, oldIndexName, newIndexName);
+    execute(connection, sql);
+  }
+
   private String getIndexName(String schema, String table, String indexedColumn) {
     return String.join("_", INDEX_NAME_PREFIX, schema, table, indexedColumn);
   }
diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineDb2.java b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineDb2.java
@@ -292,6 +292,17 @@ public String dropIndexSql(String schema, String table, String indexName) {
     return "DROP INDEX " + enclose(schema) + "." + enclose(indexName);
   }
 
+  @Override
+  public String renameIndexSql(
+      String schema, String table, String oldIndexName, String newIndexName) {
+    return "RENAME INDEX "
+        + enclose(schema)
+        + "."
+        + enclose(oldIndexName)
+        + " TO "
+        + enclose(newIndexName);
+  }
+
   @Override
   public boolean isDuplicateIndexError(SQLException e) {
     // Even though the "create index if exists ..." syntax does not exist,
diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineMysql.java b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineMysql.java
@@ -135,6 +135,17 @@ public String dropIndexSql(String schema, String table, String indexName) {
     return "DROP INDEX " + enclose(indexName) + " ON " + encloseFullTableName(schema, table);
   }
 
+  @Override
+  public String renameIndexSql(
+      String schema, String table, String oldIndexName, String newIndexName) {
+    return "ALTER TABLE "
+        + encloseFullTableName(schema, table)
+        + " RENAME INDEX "
+        + enclose(oldIndexName)
+        + " TO "
+        + enclose(newIndexName);
+  }
+
   @Override
   public String enclose(String name) {
     return "`" + name + "`";
diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineOracle.java b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineOracle.java
@@ -163,6 +163,17 @@ public String dropIndexSql(String schema, String table, String indexName) {
     return "DROP INDEX " + enclose(schema) + "." + enclose(indexName);
   }
 
+  @Override
+  public String renameIndexSql(
+      String schema, String table, String oldIndexName, String newIndexName) {
+    return "ALTER INDEX "
+        + enclose(schema)
+        + "."
+        + enclose(oldIndexName)
+        + " RENAME TO "
+        + enclose(newIndexName);
+  }
+
   @Override
   public String enclose(String name) {
     return "\"" + name + "\"";
diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEnginePostgresql.java b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEnginePostgresql.java
@@ -132,6 +132,17 @@ public String dropIndexSql(String schema, String table, String indexName) {
     return "DROP INDEX " + enclose(schema) + "." + enclose(indexName);
   }
 
+  @Override
+  public String renameIndexSql(
+      String schema, String table, String oldIndexName, String newIndexName) {
+    return "ALTER INDEX "
+        + enclose(schema)
+        + "."
+        + enclose(oldIndexName)
+        + " RENAME TO "
+        + enclose(newIndexName);
+  }
+
   @Override
   public boolean isDuplicateTableError(SQLException e) {
     if (e.getSQLState() == null) {
diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlServer.java b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlServer.java
@@ -130,6 +130,18 @@ public String dropIndexSql(String schema, String table, String indexName) {
     return "DROP INDEX " + enclose(indexName) + " ON " + encloseFullTableName(schema, table);
   }
 
+  @Override
+  public String renameIndexSql(
+      String schema, String table, String oldIndexName, String newIndexName) {
+    return "EXEC sp_rename '"
+        + encloseFullTableName(schema, table)
+        + "."
+        + enclose(oldIndexName)
+        + "', '"
+        + newIndexName
+        + "', 'INDEX'";
+  }
+
   @Override
   public boolean isDuplicateTableError(SQLException e) {
     // 2714: There is already an object named '%.*ls' in the database.
diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlite.java b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlite.java
@@ -266,6 +266,13 @@ public String dropIndexSql(String schema, String table, String indexName) {
     return "DROP INDEX " + enclose(indexName);
   }
 
+  @Override
+  public String renameIndexSql(
+      String schema, String table, String oldIndexName, String newIndexName) {
+    // SQLite does not support renaming an index
+    return null;
+  }
+
   @Override
   public String enclose(String name) {
     return "\"" + name + "\"";
diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java
@@ -134,6 +134,8 @@ default String createIndexSql(
 
   String dropIndexSql(String schema, String table, String indexName);
 
+  String renameIndexSql(String schema, String table, String oldIndexName, String newIndexName);
+
   /**
    * Enclose the target (schema, table or column) to use reserved words and special characters.
    *
diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdmin.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdmin.java
@@ -265,11 +265,16 @@ public void renameColumn(
       throw new IllegalArgumentException(
           CoreError.TABLE_NOT_FOUND.buildMessage(ScalarDbUtils.getFullTableName(namespace, table)));
     }
-    String oldBeforeColumnName = getBeforeImageColumnName(oldColumnName, tableMetadata);
-    String newBeforeColumnName = getBeforeImageColumnName(newColumnName, tableMetadata);
-
-    admin.renameColumn(namespace, table, oldColumnName, newColumnName);
-    admin.renameColumn(namespace, table, oldBeforeColumnName, newBeforeColumnName);
+    if (tableMetadata.getPartitionKeyNames().contains(oldColumnName)
+        || tableMetadata.getClusteringKeyNames().contains(oldColumnName)) {
+      admin.renameColumn(namespace, table, oldColumnName, newColumnName);
+    } else {
+      String oldBeforeColumnName = getBeforeImageColumnName(oldColumnName, tableMetadata);
+      String newBeforeColumnName = getBeforeImageColumnName(newColumnName, tableMetadata);
+
+      admin.renameColumn(namespace, table, oldColumnName, newColumnName);
+      admin.renameColumn(namespace, table, oldBeforeColumnName, newBeforeColumnName);
+    }
   }
 
   @Override
diff --git a/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java b/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java
diff --git a/integration-test/src/main/java/com/scalar/db/api/DistributedTransactionAdminIntegrationTestBase.java b/integration-test/src/main/java/com/scalar/db/api/DistributedTransactionAdminIntegrationTestBase.java
