From 087a6e107b57b99b46eee2fb2f57abeb0a3509e1 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Fri, 29 Aug 2025 14:17:26 +0900 Subject: [PATCH 01/15] Add rename column --- ...sCommitAdminIntegrationTestWithCosmos.java | 17 ++ ...osAdminCaseSensitivityIntegrationTest.java | 17 ++ .../cosmos/CosmosAdminIntegrationTest.java | 17 ++ ...sactionAdminIntegrationTestWithCosmos.java | 17 ++ ...sCommitAdminIntegrationTestWithDynamo.java | 17 ++ ...moAdminCaseSensitivityIntegrationTest.java | 17 ++ .../dynamo/DynamoAdminIntegrationTest.java | 17 ++ .../DynamoAdminPermissionIntegrationTest.java | 5 + ...sactionAdminIntegrationTestWithDynamo.java | 17 ++ .../main/java/com/scalar/db/api/Admin.java | 15 ++ .../CheckedDistributedStorageAdmin.java | 40 +++++ .../java/com/scalar/db/common/CoreError.java | 18 ++ .../DecoratedDistributedTransactionAdmin.java | 7 + .../com/scalar/db/service/AdminService.java | 7 + .../db/storage/cassandra/CassandraAdmin.java | 23 +++ .../scalar/db/storage/cosmos/CosmosAdmin.java | 7 + .../scalar/db/storage/dynamo/DynamoAdmin.java | 8 + .../com/scalar/db/storage/jdbc/JdbcAdmin.java | 27 +++ .../db/storage/jdbc/RdbEngineSqlServer.java | 12 ++ .../db/storage/jdbc/RdbEngineStrategy.java | 10 ++ .../multistorage/MultiStorageAdmin.java | 7 + .../consensuscommit/ConsensusCommitAdmin.java | 18 ++ .../jdbc/JdbcTransactionAdmin.java | 7 + .../SingleCrudOperationTransactionAdmin.java | 7 + .../storage/cassandra/CassandraAdminTest.java | 20 +++ .../db/storage/cosmos/CosmosAdminTest.java | 3 + .../storage/dynamo/DynamoAdminTestBase.java | 2 + .../scalar/db/storage/jdbc/JdbcAdminTest.java | 154 ++++++++++++++++++ .../multistorage/MultiStorageAdminTest.java | 64 ++++++++ .../ConsensusCommitAdminTestBase.java | 29 ++++ .../jdbc/JdbcTransactionAdminTest.java | 15 ++ ...ngleCrudOperationTransactionAdminTest.java | 15 ++ ...ibutedStorageAdminIntegrationTestBase.java | 66 ++++++++ ...ageAdminPermissionIntegrationTestBase.java | 10 ++ ...edTransactionAdminIntegrationTestBase.java | 60 +++++++ 35 files changed, 792 insertions(+) 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 index b372b419fb..96dc3db2b5 100644 --- 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 @@ -4,6 +4,7 @@ import com.scalar.db.util.AdminTestUtils; import java.util.Map; import java.util.Properties; +import org.junit.jupiter.api.Disabled; public class ConsensusCommitAdminIntegrationTestWithCosmos extends ConsensusCommitAdminIntegrationTestBase { @@ -22,4 +23,20 @@ protected Map getCreationOptions() { protected AdminTestUtils getAdminTestUtils(String testName) { return new CosmosAdminTestUtils(getProperties(testName)); } + + @Override + @Disabled("Cosmos DB does not support renaming columns") + public void renameColumn_ShouldRenameColumnCorrectly() {} + + @Override + @Disabled("Cosmos DB does not support renaming columns") + public void renameColumn_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("Cosmos DB does not support renaming columns") + public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("Cosmos DB does not support renaming columns") + public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} } 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 index 4048c82c9c..c2561370e9 100644 --- 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 @@ -4,6 +4,7 @@ import com.scalar.db.util.AdminTestUtils; import java.util.Map; import java.util.Properties; +import org.junit.jupiter.api.Disabled; public class CosmosAdminCaseSensitivityIntegrationTest extends DistributedStorageAdminCaseSensitivityIntegrationTestBase { @@ -22,4 +23,20 @@ protected Map getCreationOptions() { protected AdminTestUtils getAdminTestUtils(String testName) { return new CosmosAdminTestUtils(getProperties(testName)); } + + @Override + @Disabled("Cosmos DB does not support renaming columns") + public void renameColumn_ShouldRenameColumnCorrectly() {} + + @Override + @Disabled("Cosmos DB does not support renaming columns") + public void renameColumn_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("Cosmos DB does not support renaming columns") + public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("Cosmos DB does not support renaming columns") + public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} } 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 index 03c2fbf59a..7b278582b0 100644 --- 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 @@ -4,6 +4,7 @@ import com.scalar.db.util.AdminTestUtils; import java.util.Map; import java.util.Properties; +import org.junit.jupiter.api.Disabled; public class CosmosAdminIntegrationTest extends DistributedStorageAdminIntegrationTestBase { @@ -21,4 +22,20 @@ protected Map getCreationOptions() { protected AdminTestUtils getAdminTestUtils(String testName) { return new CosmosAdminTestUtils(getProperties(testName)); } + + @Override + @Disabled("Cosmos DB does not support renaming columns") + public void renameColumn_ShouldRenameColumnCorrectly() {} + + @Override + @Disabled("Cosmos DB does not support renaming columns") + public void renameColumn_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("Cosmos DB does not support renaming columns") + public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("Cosmos DB does not support renaming columns") + public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} } 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 index e9b37a9ea4..32f94bc609 100644 --- 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 @@ -3,6 +3,7 @@ import com.scalar.db.transaction.singlecrudoperation.SingleCrudOperationTransactionAdminIntegrationTestBase; import java.util.Map; import java.util.Properties; +import org.junit.jupiter.api.Disabled; public class SingleCrudOperationTransactionAdminIntegrationTestWithCosmos extends SingleCrudOperationTransactionAdminIntegrationTestBase { @@ -16,4 +17,20 @@ protected Properties getProps(String testName) { protected Map getCreationOptions() { return CosmosEnv.getCreationOptions(); } + + @Override + @Disabled("Cosmos DB does not support renaming columns") + public void renameColumn_ShouldRenameColumnCorrectly() {} + + @Override + @Disabled("Cosmos DB does not support renaming columns") + public void renameColumn_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("Cosmos DB does not support renaming columns") + public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("Cosmos DB does not support renaming columns") + public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} } 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 index 0269fc8f11..9baa5195cd 100644 --- 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 @@ -4,6 +4,7 @@ import com.scalar.db.util.AdminTestUtils; import java.util.Map; import java.util.Properties; +import org.junit.jupiter.api.Disabled; public class ConsensusCommitAdminIntegrationTestWithDynamo extends ConsensusCommitAdminIntegrationTestBase { @@ -27,4 +28,20 @@ protected boolean isIndexOnBooleanColumnSupported() { protected AdminTestUtils getAdminTestUtils(String testName) { return new DynamoAdminTestUtils(getProperties(testName)); } + + @Override + @Disabled("DynamoDB does not support renaming columns") + public void renameColumn_ShouldRenameColumnCorrectly() {} + + @Override + @Disabled("DynamoDB does not support renaming columns") + public void renameColumn_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("DynamoDB does not support renaming columns") + public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("DynamoDB does not support renaming columns") + public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} } 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 index 94b7fa1d21..f8abbbe279 100644 --- 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 @@ -4,6 +4,7 @@ import com.scalar.db.util.AdminTestUtils; import java.util.Map; import java.util.Properties; +import org.junit.jupiter.api.Disabled; public class DynamoAdminCaseSensitivityIntegrationTest extends DistributedStorageAdminCaseSensitivityIntegrationTestBase { @@ -27,4 +28,20 @@ protected boolean isIndexOnBooleanColumnSupported() { protected AdminTestUtils getAdminTestUtils(String testName) { return new DynamoAdminTestUtils(getProperties(testName)); } + + @Override + @Disabled("DynamoDB does not support renaming columns") + public void renameColumn_ShouldRenameColumnCorrectly() {} + + @Override + @Disabled("DynamoDB does not support renaming columns") + public void renameColumn_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("DynamoDB does not support renaming columns") + public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("DynamoDB does not support renaming columns") + public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} } 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 index 938c9f0a20..a9f133dd8b 100644 --- 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 @@ -4,6 +4,7 @@ import com.scalar.db.util.AdminTestUtils; import java.util.Map; import java.util.Properties; +import org.junit.jupiter.api.Disabled; public class DynamoAdminIntegrationTest extends DistributedStorageAdminIntegrationTestBase { @@ -26,4 +27,20 @@ protected boolean isIndexOnBooleanColumnSupported() { protected AdminTestUtils getAdminTestUtils(String testName) { return new DynamoAdminTestUtils(getProperties(testName)); } + + @Override + @Disabled("DynamoDB does not support renaming columns") + public void renameColumn_ShouldRenameColumnCorrectly() {} + + @Override + @Disabled("DynamoDB does not support renaming columns") + public void renameColumn_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("DynamoDB does not support renaming columns") + public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("DynamoDB does not support renaming columns") + public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} } diff --git a/core/src/integration-test/java/com/scalar/db/storage/dynamo/DynamoAdminPermissionIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/dynamo/DynamoAdminPermissionIntegrationTest.java index b2fcd6aa5e..231579f000 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/dynamo/DynamoAdminPermissionIntegrationTest.java +++ b/core/src/integration-test/java/com/scalar/db/storage/dynamo/DynamoAdminPermissionIntegrationTest.java @@ -59,4 +59,9 @@ public void addRawColumnToTable_WithSufficientPermission_ShouldSucceed() {} @Override @Disabled("Import-related functionality is not supported in DynamoDB") public void importTable_WithSufficientPermission_ShouldSucceed() {} + + @Test + @Override + @Disabled("DynamoDB does not support renaming columns") + public void renameColumn_WithSufficientPermission_ShouldSucceed() {} } 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 index 7fa7d490e7..fc37121917 100644 --- 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 @@ -3,6 +3,7 @@ import com.scalar.db.transaction.singlecrudoperation.SingleCrudOperationTransactionAdminIntegrationTestBase; import java.util.Map; import java.util.Properties; +import org.junit.jupiter.api.Disabled; public class SingleCrudOperationTransactionAdminIntegrationTestWithDynamo extends SingleCrudOperationTransactionAdminIntegrationTestBase { @@ -21,4 +22,20 @@ protected Map getCreationOptions() { protected boolean isIndexOnBooleanColumnSupported() { return false; } + + @Override + @Disabled("DynamoDB does not support renaming columns") + public void renameColumn_ShouldRenameColumnCorrectly() {} + + @Override + @Disabled("DynamoDB does not support renaming columns") + public void renameColumn_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("DynamoDB does not support renaming columns") + public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("DynamoDB does not support renaming columns") + public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} } diff --git a/core/src/main/java/com/scalar/db/api/Admin.java b/core/src/main/java/com/scalar/db/api/Admin.java index 937c959aa0..4432597321 100644 --- a/core/src/main/java/com/scalar/db/api/Admin.java +++ b/core/src/main/java/com/scalar/db/api/Admin.java @@ -441,6 +441,21 @@ default void addNewColumnToTable( } } + /** + * Renames an existing column of an existing table. + * + * @param namespace the table namespace + * @param table the table name + * @param oldColumnName the current name of the column to rename + * @param newColumnName the new name of the column + * @throws IllegalArgumentException if the table or the old column does not exist, the new column + * already exists, or the column is a partition key column, clustering key column, or is + * indexed + * @throws ExecutionException if the operation fails + */ + void renameColumn(String namespace, String table, String oldColumnName, String newColumnName) + throws ExecutionException; + /** * Imports an existing table that is not managed by ScalarDB. * diff --git a/core/src/main/java/com/scalar/db/common/CheckedDistributedStorageAdmin.java b/core/src/main/java/com/scalar/db/common/CheckedDistributedStorageAdmin.java index 0786f02456..ef49867335 100644 --- a/core/src/main/java/com/scalar/db/common/CheckedDistributedStorageAdmin.java +++ b/core/src/main/java/com/scalar/db/common/CheckedDistributedStorageAdmin.java @@ -295,6 +295,46 @@ public void addNewColumnToTable( } } + @Override + public void renameColumn( + String namespace, String table, String oldColumnName, String newColumnName) + throws ExecutionException { + TableMetadata tableMetadata = getTableMetadata(namespace, table); + if (tableMetadata == null) { + throw new IllegalArgumentException( + CoreError.TABLE_NOT_FOUND.buildMessage(ScalarDbUtils.getFullTableName(namespace, table))); + } + + if (!tableMetadata.getColumnNames().contains(oldColumnName)) { + throw new IllegalArgumentException( + CoreError.COLUMN_NOT_FOUND2.buildMessage( + ScalarDbUtils.getFullTableName(namespace, table), oldColumnName)); + } + + if (tableMetadata.getColumnNames().contains(newColumnName)) { + throw new IllegalArgumentException( + CoreError.COLUMN_ALREADY_EXISTS.buildMessage( + ScalarDbUtils.getFullTableName(namespace, table), newColumnName)); + } + + if (tableMetadata.getPartitionKeyNames().contains(oldColumnName) + || tableMetadata.getClusteringKeyNames().contains(oldColumnName) + || tableMetadata.getSecondaryIndexNames().contains(oldColumnName)) { + throw new IllegalArgumentException( + CoreError.COLUMN_SPECIFIED_AS_PRIMARY_KEY_OR_INDEX_KEY.buildMessage( + ScalarDbUtils.getFullTableName(namespace, table), oldColumnName)); + } + + try { + admin.renameColumn(namespace, table, oldColumnName, newColumnName); + } catch (ExecutionException e) { + throw new ExecutionException( + CoreError.RENAMING_COLUMN_FAILED.buildMessage( + ScalarDbUtils.getFullTableName(namespace, table), oldColumnName, newColumnName), + e); + } + } + @Override public Set getNamespaceNames() throws ExecutionException { try { diff --git a/core/src/main/java/com/scalar/db/common/CoreError.java b/core/src/main/java/com/scalar/db/common/CoreError.java index b4e5fbe682..8c6d4668ad 100644 --- a/core/src/main/java/com/scalar/db/common/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/CoreError.java @@ -678,6 +678,18 @@ public enum CoreError implements ScalarDbError { "Mutations across multiple storages are not allowed. Mutations: %s", "", ""), + COLUMN_SPECIFIED_AS_PRIMARY_KEY_OR_INDEX_KEY( + Category.USER_ERROR, + "0216", + "The column %s is specified as a primary key or an index key", + "", + ""), + COSMOS_RENAME_COLUMN_NOT_SUPPORTED( + Category.USER_ERROR, + "0217", + "Rename column functionality is not supported in Cosmos DB", + "", + ""), // // Errors for the concurrency error category @@ -960,6 +972,12 @@ public enum CoreError implements ScalarDbError { Category.INTERNAL_ERROR, "0057", "Recovering records failed. Details: %s", "", ""), CONSENSUS_COMMIT_COMMITTING_RECORDS_FAILED( Category.INTERNAL_ERROR, "0058", "Committing records failed. Details: %s", "", ""), + RENAMING_COLUMN_FAILED( + Category.INTERNAL_ERROR, + "0059", + "Renaming the column failed. Table: %s; Old column name: %s; New column name: %s", + "", + ""), // // Errors for the unknown transaction status error category diff --git a/core/src/main/java/com/scalar/db/common/DecoratedDistributedTransactionAdmin.java b/core/src/main/java/com/scalar/db/common/DecoratedDistributedTransactionAdmin.java index 29849b2536..fb215d5bcd 100644 --- a/core/src/main/java/com/scalar/db/common/DecoratedDistributedTransactionAdmin.java +++ b/core/src/main/java/com/scalar/db/common/DecoratedDistributedTransactionAdmin.java @@ -187,6 +187,13 @@ public void addNewColumnToTable( distributedTransactionAdmin.addNewColumnToTable(namespace, table, columnName, columnType); } + @Override + public void renameColumn( + String namespace, String table, String oldColumnName, String newColumnName) + throws ExecutionException { + distributedTransactionAdmin.renameColumn(namespace, table, oldColumnName, newColumnName); + } + @Override public void addNewColumnToTable( String namespace, String table, String columnName, DataType columnType, boolean encrypted) diff --git a/core/src/main/java/com/scalar/db/service/AdminService.java b/core/src/main/java/com/scalar/db/service/AdminService.java index 18806b1ac5..adca3ff7a2 100644 --- a/core/src/main/java/com/scalar/db/service/AdminService.java +++ b/core/src/main/java/com/scalar/db/service/AdminService.java @@ -94,6 +94,13 @@ public void addNewColumnToTable( admin.addNewColumnToTable(namespace, table, columnName, columnType); } + @Override + public void renameColumn( + String namespace, String table, String oldColumnName, String newColumnName) + throws ExecutionException { + admin.renameColumn(namespace, table, oldColumnName, newColumnName); + } + @Override public TableMetadata getImportTableMetadata( String namespace, String table, Map overrideColumnsType) diff --git a/core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java b/core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java index 1d85327eea..744e90490d 100644 --- a/core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java +++ b/core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java @@ -415,6 +415,29 @@ columnName, getFullTableName(namespace, table)), } } + @Override + public void renameColumn( + String namespace, String table, String oldColumnName, String newColumnName) + throws ExecutionException { + try { + String alterTableQuery = + SchemaBuilder.alterTable(quoteIfNecessary(namespace), quoteIfNecessary(table)) + .renameColumn(quoteIfNecessary(oldColumnName)) + .to(quoteIfNecessary(newColumnName)) + .getQueryString(); + + clusterManager.getSession().execute(alterTableQuery); + } catch (IllegalArgumentException e) { + throw e; + } catch (RuntimeException e) { + throw new ExecutionException( + String.format( + "Renaming the %s column to %s in the %s table failed", + oldColumnName, newColumnName, getFullTableName(namespace, table)), + e); + } + } + @Override public Set getNamespaceNames() throws ExecutionException { try { diff --git a/core/src/main/java/com/scalar/db/storage/cosmos/CosmosAdmin.java b/core/src/main/java/com/scalar/db/storage/cosmos/CosmosAdmin.java index ff1c2f3cff..49a4813773 100644 --- a/core/src/main/java/com/scalar/db/storage/cosmos/CosmosAdmin.java +++ b/core/src/main/java/com/scalar/db/storage/cosmos/CosmosAdmin.java @@ -644,6 +644,13 @@ columnName, getFullTableName(namespace, table)), } } + @Override + public void renameColumn( + String namespace, String table, String oldColumnName, String newColumnName) { + throw new UnsupportedOperationException( + CoreError.COSMOS_RENAME_COLUMN_NOT_SUPPORTED.buildMessage()); + } + @Override public TableMetadata getImportTableMetadata( String namespace, String table, Map overrideColumnsType) { diff --git a/core/src/main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java b/core/src/main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java index 5a0b621170..6c0796a0c6 100644 --- a/core/src/main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java +++ b/core/src/main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java @@ -1440,6 +1440,14 @@ columnName, getFullTableName(namespace, table)), } } + @Override + public void renameColumn( + String namespace, String table, String oldColumnName, String newColumnName) + throws ExecutionException { + throw new UnsupportedOperationException( + "Rename column functionality is not supported in DynamoDB"); + } + @Override public TableMetadata getImportTableMetadata( String namespace, String table, Map overrideColumnsType) { 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 index f578b3b5c9..2362185ab8 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java @@ -861,6 +861,33 @@ columnName, getFullTableName(namespace, table)), } } + @Override + public void renameColumn( + String namespace, String table, String oldColumnName, String newColumnName) + throws ExecutionException { + try { + TableMetadata currentTableMetadata = getTableMetadata(namespace, table); + DataType columnType = currentTableMetadata.getColumnDataType(oldColumnName); + TableMetadata updatedTableMetadata = + TableMetadata.newBuilder(currentTableMetadata) + .removeColumn(oldColumnName) + .addColumn(newColumnName, columnType) + .build(); + String renameColumnStatement = + rdbEngine.renameColumnSql(namespace, table, oldColumnName, newColumnName); + try (Connection connection = dataSource.getConnection()) { + execute(connection, renameColumnStatement); + addTableMetadata(connection, namespace, table, updatedTableMetadata, false, true); + } + } catch (SQLException e) { + throw new ExecutionException( + String.format( + "Renaming the %s column to %s in the %s table failed", + oldColumnName, newColumnName, getFullTableName(namespace, table)), + e); + } + } + @Override public void addRawColumnToTable( String namespace, String table, String columnName, DataType columnType) 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 index 948e3b30d5..db88f08d19 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlServer.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlServer.java @@ -101,6 +101,18 @@ public void dropNamespaceTranslateSQLException(SQLException e, String namespace) throw new ExecutionException("Dropping the schema failed: " + namespace, e); } + @Override + public String renameColumnSql( + String namespace, String table, String oldColumnName, String newColumnName) { + return "EXEC sp_rename '" + + encloseFullTableName(namespace, table) + + "." + + enclose(oldColumnName) + + "', '" + + newColumnName + + "', 'COLUMN'"; + } + @Override public String alterColumnTypeSql( String namespace, String table, String columnName, String columnType) { 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 index 39edb7ec60..de0ffa2ea9 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java @@ -98,6 +98,16 @@ default String truncateTableSql(String namespace, String table) { void dropNamespaceTranslateSQLException(SQLException e, String namespace) throws ExecutionException; + default String renameColumnSql( + String namespace, String table, String oldColumnName, String newColumnName) { + return "ALTER TABLE " + + encloseFullTableName(namespace, table) + + " RENAME COLUMN " + + enclose(oldColumnName) + + " TO " + + enclose(newColumnName); + } + String alterColumnTypeSql(String namespace, String table, String columnName, String columnType); String tableExistsInternalTableCheckSql(String fullTableName); diff --git a/core/src/main/java/com/scalar/db/storage/multistorage/MultiStorageAdmin.java b/core/src/main/java/com/scalar/db/storage/multistorage/MultiStorageAdmin.java index 5c7c4f9a79..762a0bd449 100644 --- a/core/src/main/java/com/scalar/db/storage/multistorage/MultiStorageAdmin.java +++ b/core/src/main/java/com/scalar/db/storage/multistorage/MultiStorageAdmin.java @@ -205,6 +205,13 @@ public void addNewColumnToTable( getAdmin(namespace, table).addNewColumnToTable(namespace, table, columnName, columnType); } + @Override + public void renameColumn( + String namespace, String table, String oldColumnName, String newColumnName) + throws ExecutionException { + getAdmin(namespace, table).renameColumn(namespace, table, oldColumnName, newColumnName); + } + @Override public TableMetadata getImportTableMetadata( String namespace, String table, Map overrideColumnsType) 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 index d9e5d6c313..43e334745b 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdmin.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdmin.java @@ -238,6 +238,24 @@ public void addNewColumnToTable( admin.addNewColumnToTable(namespace, table, beforeColumnName, columnType); } + @Override + public void renameColumn( + String namespace, String table, String oldColumnName, String newColumnName) + throws ExecutionException { + checkNamespace(namespace); + + TableMetadata tableMetadata = getTableMetadata(namespace, table); + if (tableMetadata == null) { + 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); + } + @Override public Set getNamespaceNames() throws ExecutionException { return admin.getNamespaceNames().stream() diff --git a/core/src/main/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdmin.java b/core/src/main/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdmin.java index 76f060903e..d5924d8379 100644 --- a/core/src/main/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdmin.java +++ b/core/src/main/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdmin.java @@ -160,6 +160,13 @@ public void addNewColumnToTable( jdbcAdmin.addNewColumnToTable(namespace, table, columnName, columnType); } + @Override + public void renameColumn( + String namespace, String table, String oldColumnName, String newColumnName) + throws ExecutionException { + jdbcAdmin.renameColumn(namespace, table, oldColumnName, newColumnName); + } + @Override public Set getNamespaceNames() throws ExecutionException { return jdbcAdmin.getNamespaceNames(); diff --git a/core/src/main/java/com/scalar/db/transaction/singlecrudoperation/SingleCrudOperationTransactionAdmin.java b/core/src/main/java/com/scalar/db/transaction/singlecrudoperation/SingleCrudOperationTransactionAdmin.java index a941639f1f..cb3bb2239d 100644 --- a/core/src/main/java/com/scalar/db/transaction/singlecrudoperation/SingleCrudOperationTransactionAdmin.java +++ b/core/src/main/java/com/scalar/db/transaction/singlecrudoperation/SingleCrudOperationTransactionAdmin.java @@ -109,6 +109,13 @@ public void addNewColumnToTable( distributedStorageAdmin.addNewColumnToTable(namespace, table, columnName, columnType); } + @Override + public void renameColumn( + String namespace, String table, String oldColumnName, String newColumnName) + throws ExecutionException { + distributedStorageAdmin.renameColumn(namespace, table, oldColumnName, newColumnName); + } + @Override public Set getNamespaceNames() throws ExecutionException { return distributedStorageAdmin.getNamespaceNames(); diff --git a/core/src/test/java/com/scalar/db/storage/cassandra/CassandraAdminTest.java b/core/src/test/java/com/scalar/db/storage/cassandra/CassandraAdminTest.java index c8b7717adf..25df43f782 100644 --- a/core/src/test/java/com/scalar/db/storage/cassandra/CassandraAdminTest.java +++ b/core/src/test/java/com/scalar/db/storage/cassandra/CassandraAdminTest.java @@ -865,6 +865,26 @@ public void addNewColumnToTable_ShouldWorkProperly() throws ExecutionException { verify(cassandraSession).execute(alterTableQuery); } + @Test + public void renameColumn_ShouldWorkProperly() throws ExecutionException { + // Arrange + String namespace = "sample_ns"; + String table = "tbl"; + String oldColumnName = "c2"; + String newColumnName = "c3"; + + // Act + cassandraAdmin.renameColumn(namespace, table, oldColumnName, newColumnName); + + // Assert + String alterTableQuery = + SchemaBuilder.alterTable(namespace, table) + .renameColumn(oldColumnName) + .to(newColumnName) + .getQueryString(); + verify(cassandraSession).execute(alterTableQuery); + } + @Test public void getNamespacesNames_WithNonExistingKeyspaces_ShouldReturnEmptySet() throws ExecutionException { diff --git a/core/src/test/java/com/scalar/db/storage/cosmos/CosmosAdminTest.java b/core/src/test/java/com/scalar/db/storage/cosmos/CosmosAdminTest.java index 7506e98abf..c58150c444 100644 --- a/core/src/test/java/com/scalar/db/storage/cosmos/CosmosAdminTest.java +++ b/core/src/test/java/com/scalar/db/storage/cosmos/CosmosAdminTest.java @@ -1149,11 +1149,14 @@ public void unsupportedOperations_ShouldThrowUnsupportedException() { () -> admin.importTable( namespace, table, Collections.emptyMap(), Collections.emptyMap())); + Throwable thrown4 = + catchThrowable(() -> admin.renameColumn(namespace, table, column, "newCol")); // Assert assertThat(thrown1).isInstanceOf(UnsupportedOperationException.class); assertThat(thrown2).isInstanceOf(UnsupportedOperationException.class); assertThat(thrown3).isInstanceOf(UnsupportedOperationException.class); + assertThat(thrown4).isInstanceOf(UnsupportedOperationException.class); } @Test diff --git a/core/src/test/java/com/scalar/db/storage/dynamo/DynamoAdminTestBase.java b/core/src/test/java/com/scalar/db/storage/dynamo/DynamoAdminTestBase.java index c208d85c43..1dcf5f81c6 100644 --- a/core/src/test/java/com/scalar/db/storage/dynamo/DynamoAdminTestBase.java +++ b/core/src/test/java/com/scalar/db/storage/dynamo/DynamoAdminTestBase.java @@ -1717,11 +1717,13 @@ public void unsupportedOperations_ShouldThrowUnsupportedException() { () -> admin.importTable( NAMESPACE, TABLE, Collections.emptyMap(), Collections.emptyMap())); + Throwable thrown4 = catchThrowable(() -> admin.renameColumn(NAMESPACE, TABLE, "c1", "c2")); // Assert assertThat(thrown1).isInstanceOf(UnsupportedOperationException.class); assertThat(thrown2).isInstanceOf(UnsupportedOperationException.class); assertThat(thrown3).isInstanceOf(UnsupportedOperationException.class); + assertThat(thrown4).isInstanceOf(UnsupportedOperationException.class); } @Test diff --git a/core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java b/core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java index ee7480a99a..3bee32b29c 100644 --- a/core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java +++ b/core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java @@ -2939,6 +2939,160 @@ private void addNewColumnToTable_ForX_ShouldWorkProperly( } } + @Test + public void renameColumn_ForMysql_ShouldWorkProperly() throws SQLException, ExecutionException { + renameColumn_ForX_ShouldWorkProperly( + RdbEngine.MYSQL, + "SELECT `column_name`,`data_type`,`key_type`,`clustering_order`,`indexed` FROM `" + + METADATA_SCHEMA + + "`.`metadata` WHERE `full_table_name`=? ORDER BY `ordinal_position` ASC", + "ALTER TABLE `ns`.`table` RENAME COLUMN `c2` TO `c3`", + "DELETE FROM `" + METADATA_SCHEMA + "`.`metadata` WHERE `full_table_name` = 'ns.table'", + "INSERT INTO `" + + METADATA_SCHEMA + + "`.`metadata` VALUES ('ns.table','c1','TEXT','PARTITION',NULL,false,1)", + "INSERT INTO `" + + METADATA_SCHEMA + + "`.`metadata` VALUES ('ns.table','c3','INT',NULL,NULL,false,2)"); + } + + @Test + public void renameColumn_ForOracle_ShouldWorkProperly() throws SQLException, ExecutionException { + renameColumn_ForX_ShouldWorkProperly( + RdbEngine.ORACLE, + "SELECT \"column_name\",\"data_type\",\"key_type\",\"clustering_order\",\"indexed\" FROM \"" + + METADATA_SCHEMA + + "\".\"metadata\" WHERE \"full_table_name\"=? ORDER BY \"ordinal_position\" ASC", + "ALTER TABLE \"ns\".\"table\" RENAME COLUMN \"c2\" TO \"c3\"", + "DELETE FROM \"" + + METADATA_SCHEMA + + "\".\"metadata\" WHERE \"full_table_name\" = 'ns.table'", + "INSERT INTO \"" + + METADATA_SCHEMA + + "\".\"metadata\" VALUES ('ns.table','c1','TEXT','PARTITION',NULL,0,1)", + "INSERT INTO \"" + + METADATA_SCHEMA + + "\".\"metadata\" VALUES ('ns.table','c3','INT',NULL,NULL,0,2)"); + } + + @Test + public void renameColumn_ForPostgresql_ShouldWorkProperly() + throws SQLException, ExecutionException { + renameColumn_ForX_ShouldWorkProperly( + RdbEngine.POSTGRESQL, + "SELECT \"column_name\",\"data_type\",\"key_type\",\"clustering_order\",\"indexed\" FROM \"" + + METADATA_SCHEMA + + "\".\"metadata\" WHERE \"full_table_name\"=? ORDER BY \"ordinal_position\" ASC", + "ALTER TABLE \"ns\".\"table\" RENAME COLUMN \"c2\" TO \"c3\"", + "DELETE FROM \"" + + METADATA_SCHEMA + + "\".\"metadata\" WHERE \"full_table_name\" = 'ns.table'", + "INSERT INTO \"" + + METADATA_SCHEMA + + "\".\"metadata\" VALUES ('ns.table','c1','TEXT','PARTITION',NULL,false,1)", + "INSERT INTO \"" + + METADATA_SCHEMA + + "\".\"metadata\" VALUES ('ns.table','c3','INT',NULL,NULL,false,2)"); + } + + @Test + public void renameColumn_ForSqlServer_ShouldWorkProperly() + throws SQLException, ExecutionException { + renameColumn_ForX_ShouldWorkProperly( + RdbEngine.SQL_SERVER, + "SELECT [column_name],[data_type],[key_type],[clustering_order],[indexed] FROM [" + + METADATA_SCHEMA + + "].[metadata] WHERE [full_table_name]=? ORDER BY [ordinal_position] ASC", + "EXEC sp_rename '[ns].[table].[c2]', 'c3', 'COLUMN'", + "DELETE FROM [" + METADATA_SCHEMA + "].[metadata] WHERE [full_table_name] = 'ns.table'", + "INSERT INTO [" + + METADATA_SCHEMA + + "].[metadata] VALUES ('ns.table','c1','TEXT','PARTITION',NULL,0,1)", + "INSERT INTO [" + + METADATA_SCHEMA + + "].[metadata] VALUES ('ns.table','c3','INT',NULL,NULL,0,2)"); + } + + @Test + public void renameColumn_ForSqlite_ShouldWorkProperly() throws SQLException, ExecutionException { + renameColumn_ForX_ShouldWorkProperly( + RdbEngine.SQLITE, + "SELECT \"column_name\",\"data_type\",\"key_type\",\"clustering_order\",\"indexed\" FROM \"" + + METADATA_SCHEMA + + "$metadata\" WHERE \"full_table_name\"=? ORDER BY \"ordinal_position\" ASC", + "ALTER TABLE \"ns$table\" RENAME COLUMN \"c2\" TO \"c3\"", + "DELETE FROM \"" + METADATA_SCHEMA + "$metadata\" WHERE \"full_table_name\" = 'ns.table'", + "INSERT INTO \"" + + METADATA_SCHEMA + + "$metadata\" VALUES ('ns.table','c1','TEXT','PARTITION',NULL,FALSE,1)", + "INSERT INTO \"" + + METADATA_SCHEMA + + "$metadata\" VALUES ('ns.table','c3','INT',NULL,NULL,FALSE,2)"); + } + + @Test + public void renameColumn_ForDb2_ShouldWorkProperly() throws SQLException, ExecutionException { + renameColumn_ForX_ShouldWorkProperly( + RdbEngine.DB2, + "SELECT \"column_name\",\"data_type\",\"key_type\",\"clustering_order\",\"indexed\" FROM \"" + + METADATA_SCHEMA + + "\".\"metadata\" WHERE \"full_table_name\"=? ORDER BY \"ordinal_position\" ASC", + "ALTER TABLE \"ns\".\"table\" RENAME COLUMN \"c2\" TO \"c3\"", + "DELETE FROM \"" + + METADATA_SCHEMA + + "\".\"metadata\" WHERE \"full_table_name\" = 'ns.table'", + "INSERT INTO \"" + + METADATA_SCHEMA + + "\".\"metadata\" VALUES ('ns.table','c1','TEXT','PARTITION',NULL,false,1)", + "INSERT INTO \"" + + METADATA_SCHEMA + + "\".\"metadata\" VALUES ('ns.table','c3','INT',NULL,NULL,false,2)"); + } + + private void renameColumn_ForX_ShouldWorkProperly( + RdbEngine rdbEngine, String expectedGetMetadataStatement, String... expectedSqlStatements) + throws SQLException, ExecutionException { + // Arrange + String namespace = "ns"; + String table = "table"; + String columnName1 = "c1"; + String columnName2 = "c2"; + String columnName3 = "c3"; + + PreparedStatement selectStatement = mock(PreparedStatement.class); + ResultSet resultSet = + mockResultSet( + new SelectAllFromMetadataTableResultSetMocker.Row( + columnName1, DataType.TEXT.toString(), "PARTITION", null, false), + new SelectAllFromMetadataTableResultSetMocker.Row( + columnName2, DataType.INT.toString(), null, null, false)); + when(selectStatement.executeQuery()).thenReturn(resultSet); + + when(connection.prepareStatement(any())).thenReturn(selectStatement); + List expectedStatements = new ArrayList<>(); + for (int i = 0; i < expectedSqlStatements.length; i++) { + Statement expectedStatement = mock(Statement.class); + expectedStatements.add(expectedStatement); + } + when(connection.createStatement()) + .thenReturn( + expectedStatements.get(0), + expectedStatements.subList(1, expectedStatements.size()).toArray(new Statement[0])); + + when(dataSource.getConnection()).thenReturn(connection); + JdbcAdmin admin = createJdbcAdminFor(rdbEngine); + + // Act + admin.renameColumn(namespace, table, columnName2, columnName3); + + // Assert + verify(selectStatement).setString(1, getFullTableName(namespace, table)); + verify(connection).prepareStatement(expectedGetMetadataStatement); + for (int i = 0; i < expectedSqlStatements.length; i++) { + verify(expectedStatements.get(i)).execute(expectedSqlStatements[i]); + } + } + @Test public void getNamespaceNames_forMysql_ShouldReturnNamespaceNames() throws Exception { getNamespaceNames_forX_ShouldReturnNamespaceNames( diff --git a/core/src/test/java/com/scalar/db/storage/multistorage/MultiStorageAdminTest.java b/core/src/test/java/com/scalar/db/storage/multistorage/MultiStorageAdminTest.java index 2a74bf0a87..f7b2f2339a 100644 --- a/core/src/test/java/com/scalar/db/storage/multistorage/MultiStorageAdminTest.java +++ b/core/src/test/java/com/scalar/db/storage/multistorage/MultiStorageAdminTest.java @@ -499,6 +499,70 @@ public void addNewColumnToTable_ForTable1InNamespace2_ShouldCallAddNewColumnOfAd verify(admin2).addNewColumnToTable(namespace, table, column, dataType); } + @Test + public void renameColumn_ForTable1InNamespace1_ShouldCallRenameColumnOfAdmin1() + throws ExecutionException { + // Arrange + String namespace = NAMESPACE1; + String table = TABLE1; + String column1 = "c1"; + String column2 = "c2"; + + // Act + multiStorageAdmin.renameColumn(namespace, table, column1, column2); + + // Assert + verify(admin1).renameColumn(namespace, table, column1, column2); + } + + @Test + public void renameColumn_ForTable2InNamespace1_ShouldShouldCallRenameColumnOfAdmin2() + throws ExecutionException { + // Arrange + String namespace = NAMESPACE1; + String table = TABLE2; + String column1 = "c1"; + String column2 = "c2"; + + // Act + multiStorageAdmin.renameColumn(namespace, table, column1, column2); + + // Assert + verify(admin2).renameColumn(namespace, table, column1, column2); + } + + @Test + public void renameColumn_ForTable3InNamespace1_ShouldCallRenameColumnOfDefaultAdmin() + throws ExecutionException { + // Arrange + String namespace = NAMESPACE1; + String table = TABLE3; + String column1 = "c1"; + String column2 = "c2"; + + // Act + multiStorageAdmin.renameColumn(namespace, table, column1, column2); + + // Assert + verify(admin3).renameColumn(namespace, table, column1, column2); + } + + @Test + public void renameColumn_ForTable1InNamespace2_ShouldCallRenameColumnOfAdmin2() + throws ExecutionException { + // Arrange + String namespace = NAMESPACE2; + String table = TABLE1; + String column1 = "c1"; + String column2 = "c2"; + + // Act + multiStorageAdmin.renameColumn(namespace, table, column1, column2); + + // Assert + verify(admin2).renameColumn(namespace, table, column1, column2); + } + @Test public void getNamespaceNames_WithExistingNamespacesNotInMapping_ShouldReturnExistingNamespacesInMappingAndFromDefaultAdmin() diff --git a/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminTestBase.java b/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminTestBase.java index 696ea3c5d8..cbe1f4f89c 100644 --- a/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminTestBase.java +++ b/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminTestBase.java @@ -602,6 +602,35 @@ public void addNewColumnToTable_WithEncrypted_ShouldThrowUnsupportedOperationExc .isInstanceOf(UnsupportedOperationException.class); } + @Test + public void renameColumn_ShouldCallJdbcAdminProperly() throws ExecutionException { + // Arrange + String existingColumnName = "col2"; + String newColumnName = "col3"; + TableMetadata tableMetadata = + TableMetadata.newBuilder() + .addColumn("col1", DataType.INT) + .addColumn(existingColumnName, DataType.INT) + .addPartitionKey("col1") + .build(); + when(distributedStorageAdmin.getTableMetadata(any(), any())) + .thenReturn(ConsensusCommitUtils.buildTransactionTableMetadata(tableMetadata)); + + // Act + admin.renameColumn(NAMESPACE, TABLE, existingColumnName, newColumnName); + + // Assert + verify(distributedStorageAdmin).getTableMetadata(NAMESPACE, TABLE); + verify(distributedStorageAdmin) + .renameColumn(NAMESPACE, TABLE, existingColumnName, newColumnName); + verify(distributedStorageAdmin) + .renameColumn( + NAMESPACE, + TABLE, + Attribute.BEFORE_PREFIX + existingColumnName, + Attribute.BEFORE_PREFIX + newColumnName); + } + @Test public void importTable_ShouldCallStorageAdminProperly() throws ExecutionException { // Arrange diff --git a/core/src/test/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdminTest.java b/core/src/test/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdminTest.java index 8e0f9f077c..4c69a11379 100644 --- a/core/src/test/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdminTest.java +++ b/core/src/test/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdminTest.java @@ -228,6 +228,21 @@ public void addNewColumnToTable_ShouldCallJdbcAdminProperly() throws ExecutionEx verify(jdbcAdmin).addNewColumnToTable(namespace, table, column, dataType); } + @Test + public void renameColumn_ShouldCallJdbcAdminProperly() throws ExecutionException { + // Arrange + String namespace = "ns"; + String table = "tbl"; + String columnName1 = "c1"; + String columnName2 = "c2"; + + // Act + admin.renameColumn(namespace, table, columnName1, columnName2); + + // Assert + verify(jdbcAdmin).renameColumn(namespace, table, columnName1, columnName2); + } + @Test public void importTable_ShouldCallJdbcAdminProperly() throws ExecutionException { // Arrange diff --git a/core/src/test/java/com/scalar/db/transaction/singlecrudoperation/SingleCrudOperationTransactionAdminTest.java b/core/src/test/java/com/scalar/db/transaction/singlecrudoperation/SingleCrudOperationTransactionAdminTest.java index b108f5d801..5df7ffa0b9 100644 --- a/core/src/test/java/com/scalar/db/transaction/singlecrudoperation/SingleCrudOperationTransactionAdminTest.java +++ b/core/src/test/java/com/scalar/db/transaction/singlecrudoperation/SingleCrudOperationTransactionAdminTest.java @@ -233,6 +233,21 @@ public void addNewColumnToTable_ShouldCallDistributedStorageAdminProperly() verify(distributedStorageAdmin).addNewColumnToTable(namespace, table, column, dataType); } + @Test + public void renameColumn_ShouldCallDistributedStorageAdminProperly() throws ExecutionException { + // Arrange + String namespace = "ns"; + String table = "tbl"; + String columnName1 = "c1"; + String columnName2 = "c2"; + + // Act + admin.renameColumn(namespace, table, columnName1, columnName2); + + // Assert + verify(distributedStorageAdmin).renameColumn(namespace, table, columnName1, columnName2); + } + @Test public void importTable_ShouldCallDistributedStorageAdminProperly() throws ExecutionException { // Arrange 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 index 0798c86ffa..8afe9eb1fe 100644 --- a/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java @@ -977,6 +977,35 @@ public void addNewColumnToTable_AddColumnForEachExistingDataType_ShouldAddNewCol } } + @Test + public void renameColumn_ShouldRenameColumnCorrectly() throws ExecutionException { + try { + // Arrange + Map options = getCreationOptions(); + TableMetadata currentTableMetadata = + TableMetadata.newBuilder() + .addColumn(getColumnName1(), DataType.INT) + .addColumn(getColumnName2(), DataType.INT) + .addPartitionKey(getColumnName1()) + .build(); + admin.createTable(namespace1, getTable4(), currentTableMetadata, options); + + // Act + admin.renameColumn(namespace1, getTable4(), getColumnName2(), getColumnName3()); + + // Assert + TableMetadata expectedTableMetadata = + TableMetadata.newBuilder() + .addColumn(getColumnName1(), DataType.INT) + .addColumn(getColumnName3(), DataType.INT) + .addPartitionKey(getColumnName1()) + .build(); + assertThat(admin.getTableMetadata(namespace1, getTable4())).isEqualTo(expectedTableMetadata); + } finally { + admin.dropTable(namespace1, getTable4(), true); + } + } + @Test public void getNamespaceNames_ShouldReturnCreatedNamespaces() throws ExecutionException { // Arrange @@ -1010,6 +1039,43 @@ public void addNewColumnToTable_ForAlreadyExistingColumn_ShouldThrowIllegalArgum .isInstanceOf(IllegalArgumentException.class); } + @Test + public void renameColumn_ForNonExistingTable_ShouldThrowIllegalArgumentException() { + // Arrange + + // Act Assert + assertThatThrownBy( + () -> admin.renameColumn(namespace1, getTable4(), getColumnName2(), getColumnName3())) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentException() { + // Arrange + + // Act Assert + assertThatThrownBy( + () -> + admin.renameColumn(namespace1, getTable1(), "nonExistingColumn", getColumnName3())) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() { + // Arrange + + // Act Assert + assertThatThrownBy( + () -> admin.renameColumn(namespace1, getTable1(), getColumnName1(), "newColumnName")) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy( + () -> admin.renameColumn(namespace1, getTable1(), getColumnName3(), "newColumnName")) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy( + () -> admin.renameColumn(namespace1, getTable1(), getColumnName5(), "newColumnName")) + .isInstanceOf(IllegalArgumentException.class); + } + @Test public void upgrade_WhenMetadataTableExistsButNotNamespacesTable_ShouldCreateNamespacesTableAndImportExistingNamespaces() diff --git a/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminPermissionIntegrationTestBase.java b/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminPermissionIntegrationTestBase.java index e81af475f6..ac0115bacf 100644 --- a/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminPermissionIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminPermissionIntegrationTestBase.java @@ -308,6 +308,16 @@ public void addNewColumnToTable_WithSufficientPermission_ShouldSucceed() .doesNotThrowAnyException(); } + @Test + public void renameColumn_WithSufficientPermission_ShouldSucceed() throws ExecutionException { + // Arrange + createNamespaceByRoot(); + createTableByRoot(); + // Act Assert + assertThatCode(() -> adminForNormalUser.renameColumn(NAMESPACE, TABLE, COL_NAME3, NEW_COL_NAME)) + .doesNotThrowAnyException(); + } + @Test public void importTable_WithSufficientPermission_ShouldSucceed() throws Exception { // Arrange 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 index bc040e9f0a..51e9582dcf 100644 --- a/integration-test/src/main/java/com/scalar/db/api/DistributedTransactionAdminIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/api/DistributedTransactionAdminIntegrationTestBase.java @@ -886,6 +886,66 @@ public void addNewColumnToTable_ForAlreadyExistingColumn_ShouldThrowIllegalArgum .isInstanceOf(IllegalArgumentException.class); } + @Test + public void renameColumn_ShouldRenameColumnCorrectly() throws ExecutionException { + try { + // Arrange + Map options = getCreationOptions(); + TableMetadata currentTableMetadata = + TableMetadata.newBuilder() + .addColumn("c1", DataType.INT) + .addColumn("c2", DataType.INT) + .addPartitionKey("c1") + .build(); + admin.createTable(namespace1, TABLE4, currentTableMetadata, options); + + // Act + admin.renameColumn(namespace1, TABLE4, "c2", "c3"); + + // Assert + TableMetadata expectedTableMetadata = + TableMetadata.newBuilder() + .addColumn("c1", DataType.INT) + .addColumn("c3", DataType.INT) + .addPartitionKey("c1") + .build(); + assertThat(admin.getTableMetadata(namespace1, TABLE4)).isEqualTo(expectedTableMetadata); + } finally { + admin.dropTable(namespace1, TABLE4, true); + } + } + + @Test + public void renameColumn_ForNonExistingTable_ShouldThrowIllegalArgumentException() { + // Arrange + + // Act Assert + assertThatThrownBy(() -> admin.renameColumn(namespace1, TABLE4, "c2", "c3")) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentException() { + // Arrange + + // Act Assert + assertThatThrownBy(() -> admin.renameColumn(namespace1, TABLE1, "nonExistingColumn", "c3")) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() { + // Arrange + + // Act Assert + assertThatThrownBy(() -> admin.renameColumn(namespace1, TABLE1, "c1", "newColumnName")) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> admin.renameColumn(namespace1, TABLE1, "c3", "newColumnName")) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> admin.renameColumn(namespace1, TABLE1, "c5", "newColumnName")) + .isInstanceOf(IllegalArgumentException.class); + } + @Test public void createCoordinatorTables_ShouldCreateCoordinatorTablesCorrectly() throws ExecutionException { From 730a7633b620bca6faab2522afc90745a8295217 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Fri, 5 Sep 2025 19:06:09 +0900 Subject: [PATCH 02/15] Reformat --- ...onsensusCommitAdminIntegrationTestWithCosmos.java | 2 +- .../CosmosAdminCaseSensitivityIntegrationTest.java | 12 ++++++------ .../storage/cosmos/CosmosAdminIntegrationTest.java | 12 ++++++------ ...ionTransactionAdminIntegrationTestWithCosmos.java | 2 +- ...onsensusCommitAdminIntegrationTestWithDynamo.java | 2 +- .../DynamoAdminCaseSensitivityIntegrationTest.java | 12 ++++++------ .../storage/dynamo/DynamoAdminIntegrationTest.java | 12 ++++++------ ...ionTransactionAdminIntegrationTestWithDynamo.java | 2 +- 8 files changed, 28 insertions(+), 28 deletions(-) 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 index 058274b202..831fdb5012 100644 --- 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 @@ -40,8 +40,8 @@ public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentE @Disabled("Cosmos DB does not support dropping columns") public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() {} - @Disabled("Cosmos DB does not support dropping columns") @Override + @Disabled("Cosmos DB does not support dropping columns") public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() {} @Override 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 index d0629a6e3d..b5c7a3bfa5 100644 --- 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 @@ -24,28 +24,28 @@ protected AdminTestUtils getAdminTestUtils(String testName) { return new CosmosAdminTestUtils(getProperties(testName)); } - @Disabled("Cosmos DB does not support dropping columns") @Override + @Disabled("Cosmos DB does not support dropping columns") public void dropColumnFromTable_DropColumnForEachExistingDataType_ShouldDropColumnsCorrectly() {} - @Disabled("Cosmos DB does not support dropping columns") @Override + @Disabled("Cosmos DB does not support dropping columns") public void dropColumnFromTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} - @Disabled("Cosmos DB does not support dropping columns") @Override + @Disabled("Cosmos DB does not support dropping columns") public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} - @Disabled("Cosmos DB does not support dropping columns") @Override + @Disabled("Cosmos DB does not support dropping columns") public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() {} - @Disabled("Cosmos DB does not support dropping columns") @Override + @Disabled("Cosmos DB does not support dropping columns") public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() {} - @Disabled("Cosmos DB does not support dropping columns") @Override + @Disabled("Cosmos DB does not support dropping columns") public void dropColumnFromTable_IfExists_ForNonExistingColumn_ShouldNotThrowAnyException() {} @Override 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 index bd7b02a077..4df9aac231 100644 --- 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 @@ -23,28 +23,28 @@ protected AdminTestUtils getAdminTestUtils(String testName) { return new CosmosAdminTestUtils(getProperties(testName)); } - @Disabled("Cosmos DB does not support dropping columns") @Override + @Disabled("Cosmos DB does not support dropping columns") public void dropColumnFromTable_DropColumnForEachExistingDataType_ShouldDropColumnsCorrectly() {} - @Disabled("Cosmos DB does not support dropping columns") @Override + @Disabled("Cosmos DB does not support dropping columns") public void dropColumnFromTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} - @Disabled("Cosmos DB does not support dropping columns") @Override + @Disabled("Cosmos DB does not support dropping columns") public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} - @Disabled("Cosmos DB does not support dropping columns") @Override + @Disabled("Cosmos DB does not support dropping columns") public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() {} - @Disabled("Cosmos DB does not support dropping columns") @Override + @Disabled("Cosmos DB does not support dropping columns") public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() {} - @Disabled("Cosmos DB does not support dropping columns") @Override + @Disabled("Cosmos DB does not support dropping columns") public void dropColumnFromTable_IfExists_ForNonExistingColumn_ShouldNotThrowAnyException() {} @Override 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 index adb02492ec..9ae29ee56a 100644 --- 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 @@ -34,8 +34,8 @@ public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentE @Disabled("Cosmos DB does not support dropping columns") public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() {} - @Disabled("Cosmos DB does not support dropping columns") @Override + @Disabled("Cosmos DB does not support dropping columns") public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() {} @Override 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 index b67c4cea99..992ee59a31 100644 --- 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 @@ -45,8 +45,8 @@ public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentE @Disabled("DynamoDB does not support dropping columns") public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() {} - @Disabled("DynamoDB does not support dropping columns") @Override + @Disabled("DynamoDB does not support dropping columns") public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() {} @Override 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 index 91884e82b2..606066aca6 100644 --- 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 @@ -29,28 +29,28 @@ protected AdminTestUtils getAdminTestUtils(String testName) { return new DynamoAdminTestUtils(getProperties(testName)); } - @Disabled("DynamoDB does not support dropping columns") @Override + @Disabled("DynamoDB does not support dropping columns") public void dropColumnFromTable_DropColumnForEachExistingDataType_ShouldDropColumnsCorrectly() {} - @Disabled("DynamoDB does not support dropping columns") @Override + @Disabled("DynamoDB does not support dropping columns") public void dropColumnFromTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} - @Disabled("DynamoDB does not support dropping columns") @Override + @Disabled("DynamoDB does not support dropping columns") public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} - @Disabled("DynamoDB does not support dropping columns") @Override + @Disabled("DynamoDB does not support dropping columns") public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() {} - @Disabled("DynamoDB does not support dropping columns") @Override + @Disabled("DynamoDB does not support dropping columns") public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() {} - @Disabled("DynamoDB does not support dropping columns") @Override + @Disabled("DynamoDB does not support dropping columns") public void dropColumnFromTable_IfExists_ForNonExistingColumn_ShouldNotThrowAnyException() {} @Override 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 index f0f15207b9..e3d85801ed 100644 --- 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 @@ -28,28 +28,28 @@ protected AdminTestUtils getAdminTestUtils(String testName) { return new DynamoAdminTestUtils(getProperties(testName)); } - @Disabled("DynamoDB does not support dropping columns") @Override + @Disabled("DynamoDB does not support dropping columns") public void dropColumnFromTable_DropColumnForEachExistingDataType_ShouldDropColumnsCorrectly() {} - @Disabled("DynamoDB does not support dropping columns") @Override + @Disabled("DynamoDB does not support dropping columns") public void dropColumnFromTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} - @Disabled("DynamoDB does not support dropping columns") @Override + @Disabled("DynamoDB does not support dropping columns") public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} - @Disabled("DynamoDB does not support dropping columns") @Override + @Disabled("DynamoDB does not support dropping columns") public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() {} - @Disabled("DynamoDB does not support dropping columns") @Override + @Disabled("DynamoDB does not support dropping columns") public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() {} - @Disabled("DynamoDB does not support dropping columns") @Override + @Disabled("DynamoDB does not support dropping columns") public void dropColumnFromTable_IfExists_ForNonExistingColumn_ShouldNotThrowAnyException() {} @Override 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 index e661b499b7..3d401d9afb 100644 --- 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 @@ -39,8 +39,8 @@ public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentE @Disabled("DynamoDB does not support dropping columns") public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() {} - @Disabled("DynamoDB does not support dropping columns") @Override + @Disabled("DynamoDB does not support dropping columns") public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() {} @Override From a63f5d8cf7d39ab400e6d926fc661b2a03276ae4 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Mon, 8 Sep 2025 08:11:59 +0900 Subject: [PATCH 03/15] Fix --- ...sCommitAdminIntegrationTestWithCosmos.java | 6 +- ...osAdminCaseSensitivityIntegrationTest.java | 6 +- .../cosmos/CosmosAdminIntegrationTest.java | 6 +- ...sactionAdminIntegrationTestWithCosmos.java | 6 +- ...sCommitAdminIntegrationTestWithDynamo.java | 6 +- ...moAdminCaseSensitivityIntegrationTest.java | 6 +- .../dynamo/DynamoAdminIntegrationTest.java | 6 +- ...sactionAdminIntegrationTestWithDynamo.java | 6 +- .../CheckedDistributedStorageAdmin.java | 8 -- .../com/scalar/db/storage/jdbc/JdbcAdmin.java | 38 +++++- .../scalar/db/storage/jdbc/RdbEngineDb2.java | 11 ++ .../db/storage/jdbc/RdbEngineMysql.java | 11 ++ .../db/storage/jdbc/RdbEngineOracle.java | 11 ++ .../db/storage/jdbc/RdbEnginePostgresql.java | 11 ++ .../db/storage/jdbc/RdbEngineSqlServer.java | 12 ++ .../db/storage/jdbc/RdbEngineSqlite.java | 7 + .../db/storage/jdbc/RdbEngineStrategy.java | 2 + .../consensuscommit/ConsensusCommitAdmin.java | 15 ++- ...ibutedStorageAdminIntegrationTestBase.java | 127 +++++++++++++----- ...edTransactionAdminIntegrationTestBase.java | 76 +++++++++-- 20 files changed, 308 insertions(+), 69 deletions(-) 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 index 831fdb5012..c164a406c5 100644 --- 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 index b5c7a3bfa5..f0b4f67010 100644 --- 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 index 4df9aac231..1b44f8a619 100644 --- 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 index 9ae29ee56a..e8c66ac747 100644 --- 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 index 992ee59a31..f9e91e6f48 100644 --- 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 index 606066aca6..d04d2eb196 100644 --- 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 index e3d85801ed..ec8c49b585 100644 --- 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 index 3d401d9afb..e92a5e1888 100644 --- 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 index 3efb719e81..299ee77fc3 100644 --- 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 index 677ce338d3..693a9bc95e 100644 --- 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 index b68a3fad61..430ac9d218 100644 --- 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 index 930fa5eaa2..2cf657c4c8 100644 --- 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 index 8b90648992..16434c4cbe 100644 --- 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 index 8b015a0da0..7d831c93e6 100644 --- 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 index db88f08d19..0823f68299 100644 --- 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 index 90dea3d78d..db7820fed1 100644 --- 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 index 849529ff3b..9270c2b1c0 100644 --- 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 index 03013537e0..e25ec4eb97 100644 --- 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 index 2d862c03d2..f08bc0f9da 100644 --- a/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java @@ -1105,40 +1105,16 @@ public void addNewColumnToTable_ForAlreadyExistingColumn_ShouldThrowIllegalArgum } @Test - public void renameColumn_ForNonExistingTable_ShouldThrowIllegalArgumentException() { - // Arrange - - // Act Assert - assertThatThrownBy( - () -> admin.renameColumn(namespace1, getTable4(), getColumnName2(), getColumnName3())) - .isInstanceOf(IllegalArgumentException.class); - } - - @Test - public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentException() { + public void + addNewColumnToTable_IfNotExists_ForAlreadyExistingColumn_ShouldNotThrowAnyException() { // Arrange // Act Assert - assertThatThrownBy( + assertThatCode( () -> - admin.renameColumn(namespace1, getTable1(), "nonExistingColumn", getColumnName3())) - .isInstanceOf(IllegalArgumentException.class); - } - - @Test - public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() { - // Arrange - - // Act Assert - assertThatThrownBy( - () -> admin.renameColumn(namespace1, getTable1(), getColumnName1(), "newColumnName")) - .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy( - () -> admin.renameColumn(namespace1, getTable1(), getColumnName3(), "newColumnName")) - .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy( - () -> admin.renameColumn(namespace1, getTable1(), getColumnName5(), "newColumnName")) - .isInstanceOf(IllegalArgumentException.class); + admin.addNewColumnToTable( + namespace1, getTable1(), getColumnName7(), DataType.TEXT, false, true)) + .doesNotThrowAnyException(); } @Test @@ -1215,16 +1191,95 @@ public void dropColumnFromTable_IfExists_ForNonExistingColumn_ShouldNotThrowAnyE } @Test - public void - addNewColumnToTable_IfNotExists_ForAlreadyExistingColumn_ShouldNotThrowAnyException() { + public void renameColumn_ForNonExistingTable_ShouldThrowIllegalArgumentException() { // Arrange // Act Assert - assertThatCode( + assertThatThrownBy( + () -> admin.renameColumn(namespace1, getTable4(), getColumnName2(), getColumnName3())) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentException() { + // Arrange + + // Act Assert + assertThatThrownBy( () -> - admin.addNewColumnToTable( - namespace1, getTable1(), getColumnName7(), DataType.TEXT, false, true)) - .doesNotThrowAnyException(); + admin.renameColumn(namespace1, getTable1(), "nonExistingColumn", getColumnName3())) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly() + throws ExecutionException { + try { + // Arrange + Map options = getCreationOptions(); + TableMetadata currentTableMetadata = + TableMetadata.newBuilder() + .addColumn(getColumnName1(), DataType.INT) + .addColumn(getColumnName2(), DataType.INT) + .addColumn(getColumnName3(), DataType.TEXT) + .addPartitionKey(getColumnName1()) + .addClusteringKey(getColumnName2()) + .build(); + admin.createTable(namespace1, getTable4(), currentTableMetadata, options); + + // Act + admin.renameColumn(namespace1, getTable4(), getColumnName1(), getColumnName4()); + admin.renameColumn(namespace1, getTable4(), getColumnName2(), getColumnName5()); + + // Assert + TableMetadata expectedTableMetadata = + TableMetadata.newBuilder() + .addColumn(getColumnName4(), DataType.INT) + .addColumn(getColumnName5(), DataType.INT) + .addColumn(getColumnName3(), DataType.TEXT) + .addPartitionKey(getColumnName4()) + .addClusteringKey(getColumnName5()) + .build(); + assertThat(admin.getTableMetadata(namespace1, getTable4())).isEqualTo(expectedTableMetadata); + } finally { + admin.dropTable(namespace1, getTable4(), true); + } + } + + @Test + public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() + throws ExecutionException { + try { + // Arrange + Map options = getCreationOptions(); + TableMetadata currentTableMetadata = + TableMetadata.newBuilder() + .addColumn(getColumnName1(), DataType.INT) + .addColumn(getColumnName2(), DataType.INT) + .addColumn(getColumnName3(), DataType.TEXT) + .addPartitionKey(getColumnName1()) + .addSecondaryIndex(getColumnName3()) + .build(); + admin.createTable(namespace1, getTable4(), currentTableMetadata, options); + + // Act + admin.renameColumn(namespace1, getTable4(), getColumnName3(), getColumnName4()); + + // Assert + TableMetadata expectedTableMetadata = + TableMetadata.newBuilder() + .addColumn(getColumnName1(), DataType.INT) + .addColumn(getColumnName2(), DataType.INT) + .addColumn(getColumnName4(), DataType.TEXT) + .addPartitionKey(getColumnName1()) + .addSecondaryIndex(getColumnName4()) + .build(); + assertThat(admin.getTableMetadata(namespace1, getTable4())).isEqualTo(expectedTableMetadata); + assertThat(admin.indexExists(namespace1, getTable4(), getColumnName3())).isFalse(); + assertThat(admin.indexExists(namespace1, getTable4(), getColumnName4())).isTrue(); + } finally { + admin.dropTable(namespace1, getTable4(), true); + } } @Test 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 index a6b3555942..46bcc54339 100644 --- a/integration-test/src/main/java/com/scalar/db/api/DistributedTransactionAdminIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/api/DistributedTransactionAdminIntegrationTestBase.java @@ -1080,16 +1080,74 @@ public void renameColumn_ForNonExistingColumn_ShouldThrowIllegalArgumentExceptio } @Test - public void renameColumn_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() { - // Arrange + public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly() + throws ExecutionException { + try { + // Arrange + Map options = getCreationOptions(); + TableMetadata currentTableMetadata = + TableMetadata.newBuilder() + .addColumn("c1", DataType.INT) + .addColumn("c2", DataType.INT) + .addColumn("c3", DataType.TEXT) + .addPartitionKey("c1") + .addClusteringKey("c2") + .build(); + admin.createTable(namespace1, TABLE4, currentTableMetadata, options); - // Act Assert - assertThatThrownBy(() -> admin.renameColumn(namespace1, TABLE1, "c1", "newColumnName")) - .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> admin.renameColumn(namespace1, TABLE1, "c3", "newColumnName")) - .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> admin.renameColumn(namespace1, TABLE1, "c5", "newColumnName")) - .isInstanceOf(IllegalArgumentException.class); + // Act + admin.renameColumn(namespace1, TABLE4, "c1", "c4"); + admin.renameColumn(namespace1, TABLE4, "c2", "c5"); + + // Assert + TableMetadata expectedTableMetadata = + TableMetadata.newBuilder() + .addColumn("c4", DataType.INT) + .addColumn("c5", DataType.INT) + .addColumn("c3", DataType.TEXT) + .addPartitionKey("c4") + .addClusteringKey("c5") + .build(); + assertThat(admin.getTableMetadata(namespace1, TABLE4)).isEqualTo(expectedTableMetadata); + } finally { + admin.dropTable(namespace1, TABLE4, true); + } + } + + @Test + public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() + throws ExecutionException { + try { + // Arrange + Map options = getCreationOptions(); + TableMetadata currentTableMetadata = + TableMetadata.newBuilder() + .addColumn("c1", DataType.INT) + .addColumn("c2", DataType.INT) + .addColumn("c3", DataType.TEXT) + .addPartitionKey("c1") + .addSecondaryIndex("c3") + .build(); + admin.createTable(namespace1, TABLE4, currentTableMetadata, options); + + // Act + admin.renameColumn(namespace1, TABLE4, "c3", "c4"); + + // Assert + TableMetadata expectedTableMetadata = + TableMetadata.newBuilder() + .addColumn("c1", DataType.INT) + .addColumn("c2", DataType.INT) + .addColumn("c4", DataType.TEXT) + .addPartitionKey("c1") + .addSecondaryIndex("c4") + .build(); + assertThat(admin.getTableMetadata(namespace1, TABLE4)).isEqualTo(expectedTableMetadata); + assertThat(admin.indexExists(namespace1, TABLE4, "c3")).isFalse(); + assertThat(admin.indexExists(namespace1, TABLE4, "c4")).isTrue(); + } finally { + admin.dropTable(namespace1, TABLE4, true); + } } @Test From c03c49e594ea937708e454f1566094b23a4f7321 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Mon, 8 Sep 2025 11:10:58 +0900 Subject: [PATCH 04/15] Fix to handle an issue where Cassandra does not support renaming non-primary key columns --- ...raAdminCaseSensitivityIntegrationTest.java | 9 ++++++++ .../CassandraAdminIntegrationTest.java | 9 ++++++++ ...ssandraAdminPermissionIntegrationTest.java | 15 +++++++++++++ ...mmitAdminIntegrationTestWithCassandra.java | 9 ++++++++ ...tionAdminIntegrationTestWithCassandra.java | 9 ++++++++ .../java/com/scalar/db/common/CoreError.java | 6 +++++ ...ageAdminPermissionIntegrationTestBase.java | 22 +++++++++---------- 7 files changed, 68 insertions(+), 11 deletions(-) diff --git a/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminCaseSensitivityIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminCaseSensitivityIntegrationTest.java index d27e057274..e0a8b111e2 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminCaseSensitivityIntegrationTest.java +++ b/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminCaseSensitivityIntegrationTest.java @@ -5,6 +5,7 @@ import java.util.Collections; import java.util.Map; import java.util.Properties; +import org.junit.jupiter.api.Disabled; public class CassandraAdminCaseSensitivityIntegrationTest extends DistributedStorageAdminCaseSensitivityIntegrationTestBase { @@ -27,4 +28,12 @@ protected AdminTestUtils getAdminTestUtils(String testName) { protected boolean isTimestampTypeSupported() { return false; } + + @Override + @Disabled("Renaming non-primary key columns is not supported in Cassandra") + public void renameColumn_ShouldRenameColumnCorrectly() {} + + @Override + @Disabled("Renaming non-primary key columns is not supported in Cassandra") + public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() {} } diff --git a/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminIntegrationTest.java index 119b2ca2a3..46c8fe9be1 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminIntegrationTest.java +++ b/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminIntegrationTest.java @@ -5,6 +5,7 @@ import java.util.Collections; import java.util.Map; import java.util.Properties; +import org.junit.jupiter.api.Disabled; public class CassandraAdminIntegrationTest extends DistributedStorageAdminIntegrationTestBase { @Override @@ -26,4 +27,12 @@ protected AdminTestUtils getAdminTestUtils(String testName) { protected boolean isTimestampTypeSupported() { return false; } + + @Override + @Disabled("Renaming non-primary key columns is not supported in Cassandra") + public void renameColumn_ShouldRenameColumnCorrectly() {} + + @Override + @Disabled("Renaming non-primary key columns is not supported in Cassandra") + public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() {} } diff --git a/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminPermissionIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminPermissionIntegrationTest.java index e54ad7052a..c98376f192 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminPermissionIntegrationTest.java +++ b/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminPermissionIntegrationTest.java @@ -2,9 +2,11 @@ import static com.scalar.db.storage.cassandra.CassandraPermissionTestUtils.MAX_RETRY_COUNT; import static com.scalar.db.storage.cassandra.CassandraPermissionTestUtils.SLEEP_BETWEEN_RETRIES_SECONDS; +import static org.assertj.core.api.Assertions.assertThatCode; import com.google.common.util.concurrent.Uninterruptibles; import com.scalar.db.api.DistributedStorageAdminPermissionIntegrationTestBase; +import com.scalar.db.exception.storage.ExecutionException; import com.scalar.db.util.AdminTestUtils; import com.scalar.db.util.PermissionTestUtils; import java.util.Collections; @@ -138,4 +140,17 @@ public void addRawColumnToTable_WithSufficientPermission_ShouldSucceed() {} @Override @Disabled("Import-related functionality is not supported in Cassandra") public void importTable_WithSufficientPermission_ShouldSucceed() {} + + @Test + @Override + public void renameColumn_WithSufficientPermission_ShouldSucceed() throws ExecutionException { + // Arrange + createNamespaceByRoot(); + createTableByRoot(); + + // Act Assert + // Cassandra does not support renaming non-primary key columns + assertThatCode(() -> adminForNormalUser.renameColumn(NAMESPACE, TABLE, COL_NAME1, NEW_COL_NAME)) + .doesNotThrowAnyException(); + } } diff --git a/core/src/integration-test/java/com/scalar/db/storage/cassandra/ConsensusCommitAdminIntegrationTestWithCassandra.java b/core/src/integration-test/java/com/scalar/db/storage/cassandra/ConsensusCommitAdminIntegrationTestWithCassandra.java index b0eb1c4bab..b8b8b1d12e 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/cassandra/ConsensusCommitAdminIntegrationTestWithCassandra.java +++ b/core/src/integration-test/java/com/scalar/db/storage/cassandra/ConsensusCommitAdminIntegrationTestWithCassandra.java @@ -5,6 +5,7 @@ import java.util.Collections; import java.util.Map; import java.util.Properties; +import org.junit.jupiter.api.Disabled; public class ConsensusCommitAdminIntegrationTestWithCassandra extends ConsensusCommitAdminIntegrationTestBase { @@ -27,4 +28,12 @@ protected AdminTestUtils getAdminTestUtils(String testName) { protected boolean isTimestampTypeSupported() { return false; } + + @Override + @Disabled("Renaming non-primary key columns is not supported in Cassandra") + public void renameColumn_ShouldRenameColumnCorrectly() {} + + @Override + @Disabled("Renaming non-primary key columns is not supported in Cassandra") + public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() {} } diff --git a/core/src/integration-test/java/com/scalar/db/storage/cassandra/SingleCrudOperationTransactionAdminIntegrationTestWithCassandra.java b/core/src/integration-test/java/com/scalar/db/storage/cassandra/SingleCrudOperationTransactionAdminIntegrationTestWithCassandra.java index 4669045a58..05b48a0af8 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/cassandra/SingleCrudOperationTransactionAdminIntegrationTestWithCassandra.java +++ b/core/src/integration-test/java/com/scalar/db/storage/cassandra/SingleCrudOperationTransactionAdminIntegrationTestWithCassandra.java @@ -4,6 +4,7 @@ import java.util.Collections; import java.util.Map; import java.util.Properties; +import org.junit.jupiter.api.Disabled; public class SingleCrudOperationTransactionAdminIntegrationTestWithCassandra extends SingleCrudOperationTransactionAdminIntegrationTestBase { @@ -22,4 +23,12 @@ protected Map getCreationOptions() { protected boolean isTimestampTypeSupported() { return false; } + + @Override + @Disabled("Renaming non-primary key columns is not supported in Cassandra") + public void renameColumn_ShouldRenameColumnCorrectly() {} + + @Override + @Disabled("Renaming non-primary key columns is not supported in Cassandra") + public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() {} } diff --git a/core/src/main/java/com/scalar/db/common/CoreError.java b/core/src/main/java/com/scalar/db/common/CoreError.java index c8050b20fa..d4b067db05 100644 --- a/core/src/main/java/com/scalar/db/common/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/CoreError.java @@ -706,6 +706,12 @@ public enum CoreError implements ScalarDbError { ""), DYNAMO_RENAME_COLUMN_NOT_SUPPORTED( Category.USER_ERROR, "0221", "DynamoDB does not support the renaming column feature", "", ""), + CASSANDRA_RENAME_NON_PRIMARY_KEY_COLUMN_NOT_SUPPORTED( + Category.USER_ERROR, + "0222", + "Cassandra does not support renaming non-primary key columns", + "", + ""), // // Errors for the concurrency error category diff --git a/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminPermissionIntegrationTestBase.java b/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminPermissionIntegrationTestBase.java index 1684d76a37..8b83527dd5 100644 --- a/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminPermissionIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminPermissionIntegrationTestBase.java @@ -28,13 +28,13 @@ public abstract class DistributedStorageAdminPermissionIntegrationTestBase { private static final Logger logger = LoggerFactory.getLogger(DistributedStorageAdminPermissionIntegrationTestBase.class); - private static final String COL_NAME1 = "c1"; - private static final String COL_NAME2 = "c2"; - private static final String COL_NAME3 = "c3"; - private static final String COL_NAME4 = "c4"; - private static final String RAW_COL_NAME = "raw_col"; - private static final String NEW_COL_NAME = "new_col"; - private static final TableMetadata TABLE_METADATA = + protected static final String COL_NAME1 = "c1"; + protected static final String COL_NAME2 = "c2"; + protected static final String COL_NAME3 = "c3"; + protected static final String COL_NAME4 = "c4"; + protected static final String RAW_COL_NAME = "raw_col"; + protected static final String NEW_COL_NAME = "new_col"; + protected static final TableMetadata TABLE_METADATA = TableMetadata.newBuilder() .addColumn(COL_NAME1, DataType.INT) .addColumn(COL_NAME2, DataType.TEXT) @@ -45,8 +45,8 @@ public abstract class DistributedStorageAdminPermissionIntegrationTestBase { .addSecondaryIndex(COL_NAME4) .build(); - private DistributedStorageAdmin adminForRootUser; - private DistributedStorageAdmin adminForNormalUser; + protected DistributedStorageAdmin adminForRootUser; + protected DistributedStorageAdmin adminForNormalUser; private String normalUserName; private String normalUserPassword; @@ -424,12 +424,12 @@ protected void sleepBetweenTests() { // Default do nothing } - private void createNamespaceByRoot() throws ExecutionException { + protected void createNamespaceByRoot() throws ExecutionException { adminForRootUser.createNamespace(NAMESPACE, getCreationOptions()); waitForNamespaceCreation(); } - private void createTableByRoot() throws ExecutionException { + protected void createTableByRoot() throws ExecutionException { adminForRootUser.createTable(NAMESPACE, TABLE, TABLE_METADATA, getCreationOptions()); waitForTableCreation(); } From 67a8ed666a6e8acca1f2cfe43a45178815467f29 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Mon, 8 Sep 2025 11:12:06 +0900 Subject: [PATCH 05/15] Fix to handle an issue where MySQL 5.7 does not support RENAME COLUMN statement --- .../com/scalar/db/storage/jdbc/JdbcAdmin.java | 7 ++++++- .../scalar/db/storage/jdbc/RdbEngineMysql.java | 17 +++++++++++++++++ .../db/storage/jdbc/RdbEngineSqlServer.java | 6 +++++- .../db/storage/jdbc/RdbEngineStrategy.java | 6 +++++- .../scalar/db/storage/jdbc/JdbcAdminTest.java | 2 +- 5 files changed, 34 insertions(+), 4 deletions(-) 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 index 693a9bc95e..9f8b729dad 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java @@ -908,7 +908,12 @@ public void renameColumn( } TableMetadata updatedTableMetadata = tableMetadataBuilder.build(); String renameColumnStatement = - rdbEngine.renameColumnSql(namespace, table, oldColumnName, newColumnName); + rdbEngine.renameColumnSql( + namespace, + table, + oldColumnName, + newColumnName, + getVendorDbColumnType(updatedTableMetadata, newColumnName)); try (Connection connection = dataSource.getConnection()) { execute(connection, renameColumnStatement); if (currentTableMetadata.getSecondaryIndexNames().contains(oldColumnName)) { 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 index 2cf657c4c8..cce13b4965 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineMysql.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineMysql.java @@ -114,6 +114,23 @@ public void dropNamespaceTranslateSQLException(SQLException e, String namespace) throw new ExecutionException("Dropping the schema failed: " + namespace, e); } + @Override + public String renameColumnSql( + String namespace, + String table, + String oldColumnName, + String newColumnName, + String columnType) { + return "ALTER TABLE " + + encloseFullTableName(namespace, table) + + " CHANGE COLUMN " + + enclose(oldColumnName) + + " " + + enclose(newColumnName) + + " " + + columnType; + } + @Override public String alterColumnTypeSql( String namespace, String table, String columnName, String columnType) { 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 index 0823f68299..d10c310bde 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlServer.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlServer.java @@ -103,7 +103,11 @@ public void dropNamespaceTranslateSQLException(SQLException e, String namespace) @Override public String renameColumnSql( - String namespace, String table, String oldColumnName, String newColumnName) { + String namespace, + String table, + String oldColumnName, + String newColumnName, + String columnType) { return "EXEC sp_rename '" + encloseFullTableName(namespace, table) + "." 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 index 9270c2b1c0..d229cb26df 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java @@ -108,7 +108,11 @@ default String[] dropColumnSql(String namespace, String table, String columnName } default String renameColumnSql( - String namespace, String table, String oldColumnName, String newColumnName) { + String namespace, + String table, + String oldColumnName, + String newColumnName, + String columnType) { return "ALTER TABLE " + encloseFullTableName(namespace, table) + " RENAME COLUMN " diff --git a/core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java b/core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java index 827910f66c..ab22b91d7a 100644 --- a/core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java +++ b/core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java @@ -3086,7 +3086,7 @@ public void renameColumn_ForMysql_ShouldWorkProperly() throws SQLException, Exec "SELECT `column_name`,`data_type`,`key_type`,`clustering_order`,`indexed` FROM `" + METADATA_SCHEMA + "`.`metadata` WHERE `full_table_name`=? ORDER BY `ordinal_position` ASC", - "ALTER TABLE `ns`.`table` RENAME COLUMN `c2` TO `c3`", + "ALTER TABLE `ns`.`table` CHANGE COLUMN `c2` `c3` INT", "DELETE FROM `" + METADATA_SCHEMA + "`.`metadata` WHERE `full_table_name` = 'ns.table'", "INSERT INTO `" + METADATA_SCHEMA From bf367a6ee662a36162bf93dada835e7754665d50 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Mon, 8 Sep 2025 19:00:30 +0900 Subject: [PATCH 06/15] Fix to handle an issue where Db2 does not support renaming non-primary or index key columns --- ...tAdminIntegrationTestWithJdbcDatabase.java | 24 +++++++++++++++++++ ...bcAdminCaseSensitivityIntegrationTest.java | 24 +++++++++++++++++++ .../jdbc/JdbcAdminIntegrationTest.java | 24 +++++++++++++++++++ .../com/scalar/db/storage/jdbc/JdbcEnv.java | 4 ++++ ...nAdminIntegrationTestWithJdbcDatabase.java | 24 +++++++++++++++++++ .../JdbcTransactionAdminIntegrationTest.java | 22 +++++++++++++++++ .../main/java/com/scalar/db/api/Admin.java | 5 ++-- .../java/com/scalar/db/common/CoreError.java | 2 +- .../db/storage/cassandra/CassandraAdmin.java | 7 ++++++ .../com/scalar/db/storage/jdbc/JdbcAdmin.java | 15 ++++++------ .../scalar/db/storage/jdbc/RdbEngineDb2.java | 22 ++++++++++------- .../db/storage/jdbc/RdbEngineMysql.java | 22 ++++++++++------- .../db/storage/jdbc/RdbEngineOracle.java | 22 ++++++++++------- .../db/storage/jdbc/RdbEnginePostgresql.java | 22 ++++++++++------- .../db/storage/jdbc/RdbEngineSqlServer.java | 24 ++++++++++++------- .../db/storage/jdbc/RdbEngineSqlite.java | 13 +++++++--- .../db/storage/jdbc/RdbEngineStrategy.java | 7 +++++- 17 files changed, 226 insertions(+), 57 deletions(-) diff --git a/core/src/integration-test/java/com/scalar/db/storage/jdbc/ConsensusCommitAdminIntegrationTestWithJdbcDatabase.java b/core/src/integration-test/java/com/scalar/db/storage/jdbc/ConsensusCommitAdminIntegrationTestWithJdbcDatabase.java index 7078581c2e..1667b67122 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/jdbc/ConsensusCommitAdminIntegrationTestWithJdbcDatabase.java +++ b/core/src/integration-test/java/com/scalar/db/storage/jdbc/ConsensusCommitAdminIntegrationTestWithJdbcDatabase.java @@ -1,9 +1,12 @@ package com.scalar.db.storage.jdbc; import com.scalar.db.config.DatabaseConfig; +import com.scalar.db.exception.storage.ExecutionException; import com.scalar.db.transaction.consensuscommit.ConsensusCommitAdminIntegrationTestBase; import com.scalar.db.util.AdminTestUtils; import java.util.Properties; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledIf; public class ConsensusCommitAdminIntegrationTestWithJdbcDatabase extends ConsensusCommitAdminIntegrationTestBase { @@ -29,4 +32,25 @@ protected boolean isCreateIndexOnTextAndBlobColumnsEnabled() { // So we disable these tests until the issue with the Db2 community edition is resolved. return !JdbcTestUtils.isDb2(rdbEngine); } + + @SuppressWarnings("unused") + private boolean isDb2() { + return JdbcEnv.isDb2(); + } + + @Test + @Override + @DisabledIf("isDb2") + public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly() + throws ExecutionException { + super.renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly(); + } + + @Test + @Override + @DisabledIf("isDb2") + public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() + throws ExecutionException { + super.renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly(); + } } diff --git a/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminCaseSensitivityIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminCaseSensitivityIntegrationTest.java index 34d20d1906..376c83a78d 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminCaseSensitivityIntegrationTest.java +++ b/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminCaseSensitivityIntegrationTest.java @@ -2,8 +2,11 @@ import com.scalar.db.api.DistributedStorageAdminCaseSensitivityIntegrationTestBase; import com.scalar.db.config.DatabaseConfig; +import com.scalar.db.exception.storage.ExecutionException; import com.scalar.db.util.AdminTestUtils; import java.util.Properties; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledIf; public class JdbcAdminCaseSensitivityIntegrationTest extends DistributedStorageAdminCaseSensitivityIntegrationTestBase { @@ -28,4 +31,25 @@ protected boolean isCreateIndexOnTextAndBlobColumnsEnabled() { // So we disable these tests until the issue is resolved. return !JdbcTestUtils.isDb2(rdbEngine); } + + @SuppressWarnings("unused") + private boolean isDb2() { + return JdbcEnv.isDb2(); + } + + @Test + @Override + @DisabledIf("isDb2") + public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly() + throws ExecutionException { + super.renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly(); + } + + @Test + @Override + @DisabledIf("isDb2") + public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() + throws ExecutionException { + super.renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly(); + } } diff --git a/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminIntegrationTest.java index dffdbddf5a..9c7fe1dc28 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminIntegrationTest.java +++ b/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminIntegrationTest.java @@ -2,8 +2,11 @@ import com.scalar.db.api.DistributedStorageAdminIntegrationTestBase; import com.scalar.db.config.DatabaseConfig; +import com.scalar.db.exception.storage.ExecutionException; import com.scalar.db.util.AdminTestUtils; import java.util.Properties; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledIf; public class JdbcAdminIntegrationTest extends DistributedStorageAdminIntegrationTestBase { private RdbEngineStrategy rdbEngine; @@ -27,4 +30,25 @@ protected boolean isCreateIndexOnTextAndBlobColumnsEnabled() { // So we disable these tests until the issue is resolved. return !JdbcTestUtils.isDb2(rdbEngine); } + + @SuppressWarnings("unused") + private boolean isDb2() { + return JdbcEnv.isDb2(); + } + + @Test + @Override + @DisabledIf("isDb2") + public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly() + throws ExecutionException { + super.renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly(); + } + + @Test + @Override + @DisabledIf("isDb2") + public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() + throws ExecutionException { + super.renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly(); + } } diff --git a/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcEnv.java b/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcEnv.java index 3c7ab0eeed..a977a2346c 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcEnv.java +++ b/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcEnv.java @@ -54,4 +54,8 @@ public static Properties getPropertiesForNormalUser(String testName) { public static boolean isSqlite() { return System.getProperty(PROP_JDBC_URL, DEFAULT_JDBC_URL).startsWith("jdbc:sqlite:"); } + + public static boolean isDb2() { + return System.getProperty(PROP_JDBC_URL, DEFAULT_JDBC_URL).startsWith("jdbc:db2:"); + } } diff --git a/core/src/integration-test/java/com/scalar/db/storage/jdbc/SingleCrudOperationTransactionAdminIntegrationTestWithJdbcDatabase.java b/core/src/integration-test/java/com/scalar/db/storage/jdbc/SingleCrudOperationTransactionAdminIntegrationTestWithJdbcDatabase.java index 78385ebd68..f14d0e618b 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/jdbc/SingleCrudOperationTransactionAdminIntegrationTestWithJdbcDatabase.java +++ b/core/src/integration-test/java/com/scalar/db/storage/jdbc/SingleCrudOperationTransactionAdminIntegrationTestWithJdbcDatabase.java @@ -1,9 +1,12 @@ package com.scalar.db.storage.jdbc; import com.scalar.db.config.DatabaseConfig; +import com.scalar.db.exception.storage.ExecutionException; import com.scalar.db.transaction.singlecrudoperation.SingleCrudOperationTransactionAdminIntegrationTestBase; import com.scalar.db.util.AdminTestUtils; import java.util.Properties; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledIf; public class SingleCrudOperationTransactionAdminIntegrationTestWithJdbcDatabase extends SingleCrudOperationTransactionAdminIntegrationTestBase { @@ -29,4 +32,25 @@ protected boolean isCreateIndexOnTextAndBlobColumnsEnabled() { // So we disable these tests until the issue with the Db2 community edition is resolved. return !JdbcTestUtils.isDb2(rdbEngine); } + + @SuppressWarnings("unused") + private boolean isDb2() { + return JdbcEnv.isDb2(); + } + + @Test + @Override + @DisabledIf("isDb2") + public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly() + throws ExecutionException { + super.renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly(); + } + + @Test + @Override + @DisabledIf("isDb2") + public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() + throws ExecutionException { + super.renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly(); + } } diff --git a/core/src/integration-test/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdminIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdminIntegrationTest.java index 1a73a8d28a..cc83e15dd0 100644 --- a/core/src/integration-test/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdminIntegrationTest.java +++ b/core/src/integration-test/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdminIntegrationTest.java @@ -12,6 +12,7 @@ import java.util.Properties; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledIf; public class JdbcTransactionAdminIntegrationTest extends DistributedTransactionAdminIntegrationTestBase { @@ -102,4 +103,25 @@ protected boolean isCreateIndexOnTextAndBlobColumnsEnabled() { // So we disable these tests until the issue with the Db2 community edition is resolved. return !JdbcTestUtils.isDb2(rdbEngine); } + + @SuppressWarnings("unused") + private boolean isDb2() { + return JdbcEnv.isDb2(); + } + + @Test + @Override + @DisabledIf("isDb2") + public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly() + throws ExecutionException { + super.renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly(); + } + + @Test + @Override + @DisabledIf("isDb2") + public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() + throws ExecutionException { + super.renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly(); + } } diff --git a/core/src/main/java/com/scalar/db/api/Admin.java b/core/src/main/java/com/scalar/db/api/Admin.java index acaa0ff89e..729853d06a 100644 --- a/core/src/main/java/com/scalar/db/api/Admin.java +++ b/core/src/main/java/com/scalar/db/api/Admin.java @@ -527,9 +527,8 @@ default void dropColumnFromTable( * @param table the table name * @param oldColumnName the current name of the column to rename * @param newColumnName the new name of the column - * @throws IllegalArgumentException if the table or the old column does not exist, the new column - * already exists, or the column is a partition key column, clustering key column, or is - * indexed + * @throws IllegalArgumentException if the table or the old column does not exist or the new + * column already exists * @throws ExecutionException if the operation fails */ void renameColumn(String namespace, String table, String oldColumnName, String newColumnName) diff --git a/core/src/main/java/com/scalar/db/common/CoreError.java b/core/src/main/java/com/scalar/db/common/CoreError.java index d4b067db05..0154115a12 100644 --- a/core/src/main/java/com/scalar/db/common/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/CoreError.java @@ -709,7 +709,7 @@ public enum CoreError implements ScalarDbError { CASSANDRA_RENAME_NON_PRIMARY_KEY_COLUMN_NOT_SUPPORTED( Category.USER_ERROR, "0222", - "Cassandra does not support renaming non-primary key columns", + "Cassandra does not support renaming the non-primary key column feature", "", ""), diff --git a/core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java b/core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java index 859b2bba17..fbb9b7ac79 100644 --- a/core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java +++ b/core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java @@ -441,6 +441,13 @@ public void renameColumn( String namespace, String table, String oldColumnName, String newColumnName) throws ExecutionException { try { + TableMetadata tableMetadata = getTableMetadata(namespace, table); + assert tableMetadata != null; + if (!tableMetadata.getPartitionKeyNames().contains(oldColumnName) + || !tableMetadata.getClusteringKeyNames().contains(oldColumnName)) { + throw new IllegalArgumentException( + CoreError.CASSANDRA_RENAME_NON_PRIMARY_KEY_COLUMN_NOT_SUPPORTED.buildMessage()); + } String alterTableQuery = SchemaBuilder.alterTable(quoteIfNecessary(namespace), quoteIfNecessary(table)) .renameColumn(quoteIfNecessary(oldColumnName)) 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 index 9f8b729dad..06e2f1af26 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java @@ -890,6 +890,7 @@ public void renameColumn( throws ExecutionException { try { TableMetadata currentTableMetadata = getTableMetadata(namespace, table); + assert currentTableMetadata != null; DataType columnType = currentTableMetadata.getColumnDataType(oldColumnName); TableMetadata.Builder tableMetadataBuilder = TableMetadata.newBuilder(currentTableMetadata) @@ -917,12 +918,7 @@ public void renameColumn( 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); - } + renameIndex(connection, namespace, table, oldColumnName, newColumnName); } addTableMetadata(connection, namespace, table, updatedTableMetadata, false, true); } @@ -1001,8 +997,11 @@ private void renameIndex( 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); + String[] sqls = + rdbEngine.renameIndexSqls(schema, table, oldIndexName, newIndexName, newIndexedColumn); + for (String sql : sqls) { + execute(connection, sql); + } } private String getIndexName(String schema, String table, String 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 index 430ac9d218..63cf5aba26 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineDb2.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineDb2.java @@ -293,14 +293,20 @@ public String dropIndexSql(String schema, String table, String indexName) { } @Override - public String renameIndexSql( - String schema, String table, String oldIndexName, String newIndexName) { - return "RENAME INDEX " - + enclose(schema) - + "." - + enclose(oldIndexName) - + " TO " - + enclose(newIndexName); + public String[] renameIndexSqls( + String schema, + String table, + String oldIndexName, + String newIndexName, + String newIndexedColumn) { + return new String[] { + "RENAME INDEX " + + enclose(schema) + + "." + + enclose(oldIndexName) + + " TO " + + enclose(newIndexName) + }; } @Override 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 index cce13b4965..a8c35b5f57 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineMysql.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineMysql.java @@ -153,14 +153,20 @@ public String dropIndexSql(String schema, String table, String indexName) { } @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); + public String[] renameIndexSqls( + String schema, + String table, + String oldIndexName, + String newIndexName, + String newIndexedColumn) { + return new String[] { + "ALTER TABLE " + + encloseFullTableName(schema, table) + + " RENAME INDEX " + + enclose(oldIndexName) + + " TO " + + enclose(newIndexName) + }; } @Override 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 index 16434c4cbe..6b16df9684 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineOracle.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineOracle.java @@ -164,14 +164,20 @@ public String dropIndexSql(String schema, String table, String indexName) { } @Override - public String renameIndexSql( - String schema, String table, String oldIndexName, String newIndexName) { - return "ALTER INDEX " - + enclose(schema) - + "." - + enclose(oldIndexName) - + " RENAME TO " - + enclose(newIndexName); + public String[] renameIndexSqls( + String schema, + String table, + String oldIndexName, + String newIndexName, + String newIndexedColumn) { + return new String[] { + "ALTER INDEX " + + enclose(schema) + + "." + + enclose(oldIndexName) + + " RENAME TO " + + enclose(newIndexName) + }; } @Override 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 index 7d831c93e6..fb76b2d48f 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEnginePostgresql.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEnginePostgresql.java @@ -133,14 +133,20 @@ public String dropIndexSql(String schema, String table, String indexName) { } @Override - public String renameIndexSql( - String schema, String table, String oldIndexName, String newIndexName) { - return "ALTER INDEX " - + enclose(schema) - + "." - + enclose(oldIndexName) - + " RENAME TO " - + enclose(newIndexName); + public String[] renameIndexSqls( + String schema, + String table, + String oldIndexName, + String newIndexName, + String newIndexedColumn) { + return new String[] { + "ALTER INDEX " + + enclose(schema) + + "." + + enclose(oldIndexName) + + " RENAME TO " + + enclose(newIndexName) + }; } @Override 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 index d10c310bde..6bec004bba 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlServer.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlServer.java @@ -135,15 +135,21 @@ public String dropIndexSql(String schema, String table, String indexName) { } @Override - public String renameIndexSql( - String schema, String table, String oldIndexName, String newIndexName) { - return "EXEC sp_rename '" - + encloseFullTableName(schema, table) - + "." - + enclose(oldIndexName) - + "', '" - + newIndexName - + "', 'INDEX'"; + public String[] renameIndexSqls( + String schema, + String table, + String oldIndexName, + String newIndexName, + String newIndexedColumn) { + return new String[] { + "EXEC sp_rename '" + + encloseFullTableName(schema, table) + + "." + + enclose(oldIndexName) + + "', '" + + newIndexName + + "', 'INDEX'" + }; } @Override 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 index db7820fed1..4d04335b74 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlite.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlite.java @@ -267,10 +267,17 @@ public String dropIndexSql(String schema, String table, String indexName) { } @Override - public String renameIndexSql( - String schema, String table, String oldIndexName, String newIndexName) { + public String[] renameIndexSqls( + String schema, + String table, + String oldIndexName, + String newIndexName, + String newIndexedColumn) { // SQLite does not support renaming an index - return null; + return new String[] { + dropIndexSql(schema, table, oldIndexName), + createIndexSql(schema, table, newIndexName, newIndexedColumn) + }; } @Override 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 index d229cb26df..ea9bc1c812 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java @@ -138,7 +138,12 @@ default String createIndexSql( String dropIndexSql(String schema, String table, String indexName); - String renameIndexSql(String schema, String table, String oldIndexName, String newIndexName); + String[] renameIndexSqls( + String schema, + String table, + String oldIndexName, + String newIndexName, + String newIndexedColumn); /** * Enclose the target (schema, table or column) to use reserved words and special characters. From 0213f9d1cf8b3b9821b2014902a83c50aeb95b41 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Mon, 8 Sep 2025 22:11:24 +0900 Subject: [PATCH 07/15] Fix bug in Cassandra --- .../db/storage/cassandra/CassandraAdmin.java | 2 +- .../db/storage/cassandra/CassandraAdminTest.java | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java b/core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java index fbb9b7ac79..bbb74d9837 100644 --- a/core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java +++ b/core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java @@ -444,7 +444,7 @@ public void renameColumn( TableMetadata tableMetadata = getTableMetadata(namespace, table); assert tableMetadata != null; if (!tableMetadata.getPartitionKeyNames().contains(oldColumnName) - || !tableMetadata.getClusteringKeyNames().contains(oldColumnName)) { + && !tableMetadata.getClusteringKeyNames().contains(oldColumnName)) { throw new IllegalArgumentException( CoreError.CASSANDRA_RENAME_NON_PRIMARY_KEY_COLUMN_NOT_SUPPORTED.buildMessage()); } diff --git a/core/src/test/java/com/scalar/db/storage/cassandra/CassandraAdminTest.java b/core/src/test/java/com/scalar/db/storage/cassandra/CassandraAdminTest.java index 6d6a66a0da..b1fdca6eff 100644 --- a/core/src/test/java/com/scalar/db/storage/cassandra/CassandraAdminTest.java +++ b/core/src/test/java/com/scalar/db/storage/cassandra/CassandraAdminTest.java @@ -897,8 +897,18 @@ public void renameColumn_ShouldWorkProperly() throws ExecutionException { // Arrange String namespace = "sample_ns"; String table = "tbl"; - String oldColumnName = "c2"; - String newColumnName = "c3"; + String oldColumnName = "c1"; + String newColumnName = "c2"; + com.datastax.driver.core.TableMetadata tableMetadata = + mock(com.datastax.driver.core.TableMetadata.class); + ColumnMetadata c1 = mock(ColumnMetadata.class); + when(c1.getName()).thenReturn(oldColumnName); + when(c1.getType()).thenReturn(com.datastax.driver.core.DataType.text()); + when(tableMetadata.getPartitionKey()).thenReturn(Collections.singletonList(c1)); + when(tableMetadata.getClusteringColumns()).thenReturn(Collections.emptyList()); + when(tableMetadata.getIndexes()).thenReturn(Collections.emptyList()); + when(tableMetadata.getColumns()).thenReturn(Collections.singletonList(c1)); + when(clusterManager.getMetadata(any(), any())).thenReturn(tableMetadata); // Act cassandraAdmin.renameColumn(namespace, table, oldColumnName, newColumnName); From 7a396bb73b2c2f53d72521c049a5497eb0f68a78 Mon Sep 17 00:00:00 2001 From: Kodai Doki <52027276+KodaiD@users.noreply.github.com> Date: Tue, 9 Sep 2025 14:19:47 +0900 Subject: [PATCH 08/15] Apply suggestions from code review Co-authored-by: Hiroyuki Yamada --- core/src/main/java/com/scalar/db/common/CoreError.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/com/scalar/db/common/CoreError.java b/core/src/main/java/com/scalar/db/common/CoreError.java index 0154115a12..d4b067db05 100644 --- a/core/src/main/java/com/scalar/db/common/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/CoreError.java @@ -709,7 +709,7 @@ public enum CoreError implements ScalarDbError { CASSANDRA_RENAME_NON_PRIMARY_KEY_COLUMN_NOT_SUPPORTED( Category.USER_ERROR, "0222", - "Cassandra does not support renaming the non-primary key column feature", + "Cassandra does not support renaming non-primary key columns", "", ""), From 448b3d461f87b32d2a01bef601e07baaf09bbc19 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Tue, 9 Sep 2025 15:37:40 +0900 Subject: [PATCH 09/15] Fix to handle rename in metadata correctly --- .../java/com/scalar/db/api/TableMetadata.java | 93 +++++++++++++++++++ .../com/scalar/db/storage/jdbc/JdbcAdmin.java | 15 +-- .../com/scalar/db/api/TableMetadataTest.java | 76 +++++++++++++++ ...ibutedStorageAdminIntegrationTestBase.java | 20 ++-- ...edTransactionAdminIntegrationTestBase.java | 20 ++-- 5 files changed, 201 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/com/scalar/db/api/TableMetadata.java b/core/src/main/java/com/scalar/db/api/TableMetadata.java index b243f87d6c..f1dae0a509 100644 --- a/core/src/main/java/com/scalar/db/api/TableMetadata.java +++ b/core/src/main/java/com/scalar/db/api/TableMetadata.java @@ -251,6 +251,33 @@ public Builder removeColumn(String name) { return this; } + /** + * Renames a column from {@code oldName} to {@code newName}. + * + * @param oldName a column name to be renamed + * @param newName a new column name + * @return a builder instance + */ + public Builder renameColumn(String oldName, String newName) { + if (columns.containsKey(oldName)) { + LinkedHashMap newColumns = new LinkedHashMap<>(); + for (Map.Entry entry : columns.entrySet()) { + if (entry.getKey().equals(oldName)) { + newColumns.put(newName, entry.getValue()); + } else { + newColumns.put(entry.getKey(), entry.getValue()); + } + } + columns.clear(); + columns.putAll(newColumns); + } + if (encryptedColumnNames.contains(oldName)) { + encryptedColumnNames.remove(oldName); + encryptedColumnNames.add(newName); + } + return this; + } + /** * Adds a partition-key column with the specified name. * @@ -273,6 +300,29 @@ public Builder removePartitionKey(String name) { return this; } + /** + * Renames a partition-key column from {@code oldName} to {@code newName}. + * + * @param oldName a column name to be renamed + * @param newName a new column name + * @return a builder instance + */ + public Builder renamePartitionKey(String oldName, String newName) { + if (partitionKeyNames.contains(oldName)) { + LinkedHashSet newPartitionKeyNames = new LinkedHashSet<>(); + for (String name : partitionKeyNames) { + if (name.equals(oldName)) { + newPartitionKeyNames.add(newName); + } else { + newPartitionKeyNames.add(name); + } + } + partitionKeyNames.clear(); + partitionKeyNames.addAll(newPartitionKeyNames); + } + return this; + } + /** * Adds a clustering-key column with the specified name and the default order (ASC). * @@ -309,6 +359,34 @@ public Builder removeClusteringKey(String name) { return this; } + /** + * Renames a clustering-key column from {@code oldName} to {@code newName}. + * + * @param oldName a column name to be renamed + * @param newName a new column name + * @return a builder instance + */ + public Builder renameClusteringKey(String oldName, String newName) { + if (clusteringKeyNames.contains(oldName)) { + LinkedHashSet newClusteringKeyNames = new LinkedHashSet<>(); + LinkedHashMap newClusteringOrders = new LinkedHashMap<>(); + for (String name : clusteringKeyNames) { + if (name.equals(oldName)) { + newClusteringKeyNames.add(newName); + newClusteringOrders.put(newName, clusteringOrders.get(oldName)); + } else { + newClusteringKeyNames.add(name); + newClusteringOrders.put(name, clusteringOrders.get(name)); + } + } + clusteringKeyNames.clear(); + clusteringOrders.clear(); + clusteringKeyNames.addAll(newClusteringKeyNames); + clusteringOrders.putAll(newClusteringOrders); + } + return this; + } + /** * Adds a secondary-index column with the specified name. * @@ -331,6 +409,21 @@ public Builder removeSecondaryIndex(String name) { return this; } + /** + * Renames a secondary-index column from {@code oldName} to {@code newName}. + * + * @param oldName a column name to be renamed + * @param newName a new column name + * @return a builder instance + */ + public Builder renameSecondaryIndex(String oldName, String newName) { + if (secondaryIndexNames.contains(oldName)) { + secondaryIndexNames.remove(oldName); + secondaryIndexNames.add(newName); + } + return this; + } + /** * Builds a TableMetadata instance. * 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 index 06e2f1af26..48db6d875b 100644 --- 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,21 +891,14 @@ public void renameColumn( try { TableMetadata currentTableMetadata = getTableMetadata(namespace, table); assert currentTableMetadata != null; - DataType columnType = currentTableMetadata.getColumnDataType(oldColumnName); TableMetadata.Builder tableMetadataBuilder = - TableMetadata.newBuilder(currentTableMetadata) - .removeColumn(oldColumnName) - .addColumn(newColumnName, columnType); + TableMetadata.newBuilder(currentTableMetadata).renameColumn(oldColumnName, newColumnName); if (currentTableMetadata.getPartitionKeyNames().contains(oldColumnName)) { - tableMetadataBuilder.removePartitionKey(oldColumnName); - tableMetadataBuilder.addPartitionKey(newColumnName); + tableMetadataBuilder.renamePartitionKey(oldColumnName, newColumnName); } else if (currentTableMetadata.getClusteringKeyNames().contains(oldColumnName)) { - Ordering.Order order = currentTableMetadata.getClusteringOrder(oldColumnName); - tableMetadataBuilder.removeClusteringKey(oldColumnName); - tableMetadataBuilder.addClusteringKey(newColumnName, order); + tableMetadataBuilder.renameClusteringKey(oldColumnName, newColumnName); } else if (currentTableMetadata.getSecondaryIndexNames().contains(oldColumnName)) { - tableMetadataBuilder.removeSecondaryIndex(oldColumnName); - tableMetadataBuilder.addSecondaryIndex(newColumnName); + tableMetadataBuilder.renameSecondaryIndex(oldColumnName, newColumnName); } TableMetadata updatedTableMetadata = tableMetadataBuilder.build(); String renameColumnStatement = diff --git a/core/src/test/java/com/scalar/db/api/TableMetadataTest.java b/core/src/test/java/com/scalar/db/api/TableMetadataTest.java index c931b309db..d03eaaade9 100644 --- a/core/src/test/java/com/scalar/db/api/TableMetadataTest.java +++ b/core/src/test/java/com/scalar/db/api/TableMetadataTest.java @@ -263,4 +263,80 @@ public void builder_basedOnAnotherTableMetadata_ShouldReturnProperTableMetadata( assertThat(tableMetadata.getEncryptedColumnNames()).containsOnly(COL_NAME10); } + + @Test + public void builder_RenameColumns_ShouldRenameColumnsInTableMetadataCorrectly() { + // Arrange + TableMetadata.Builder builder = + TableMetadata.newBuilder() + .addColumn(COL_NAME1, DataType.INT) + .addColumn(COL_NAME2, DataType.TEXT) + .addColumn(COL_NAME3, DataType.TEXT) + .addColumn(COL_NAME4, DataType.INT) + .addColumn(COL_NAME5, DataType.INT) + .addColumn(COL_NAME6, DataType.TEXT) + .addColumn(COL_NAME7, DataType.BIGINT) + .addColumn(COL_NAME8, DataType.FLOAT) + .addColumn(COL_NAME9, DataType.DOUBLE) + .addColumn(COL_NAME10, DataType.BOOLEAN, true) + .addColumn(COL_NAME11, DataType.BLOB, true) + .addPartitionKey(COL_NAME2) + .addPartitionKey(COL_NAME1) + .addClusteringKey(COL_NAME4, Order.ASC) + .addClusteringKey(COL_NAME3, Order.DESC); + + // Act + TableMetadata tableMetadata = + builder + .renameColumn(COL_NAME2, "new_" + COL_NAME2) + .renameColumn(COL_NAME4, "new_" + COL_NAME4) + .renameColumn(COL_NAME10, "new_" + COL_NAME10) + .renamePartitionKey(COL_NAME2, "new_" + COL_NAME2) + .renameClusteringKey(COL_NAME4, "new_" + COL_NAME4) + .build(); + + // Assert + assertThat(tableMetadata.getColumnNames().size()).isEqualTo(11); + assertThat(tableMetadata.getColumnNames().contains(COL_NAME2)).isFalse(); + assertThat(tableMetadata.getColumnNames().contains("new_" + COL_NAME2)).isTrue(); + assertThat(tableMetadata.getColumnNames().contains(COL_NAME4)).isFalse(); + assertThat(tableMetadata.getColumnNames().contains("new_" + COL_NAME4)).isTrue(); + assertThat(tableMetadata.getColumnNames().contains(COL_NAME10)).isFalse(); + assertThat(tableMetadata.getColumnNames().contains("new_" + COL_NAME10)).isTrue(); + assertThat(tableMetadata.getPartitionKeyNames().contains(COL_NAME2)).isFalse(); + assertThat(tableMetadata.getPartitionKeyNames().contains("new_" + COL_NAME2)).isTrue(); + assertThat(tableMetadata.getClusteringKeyNames().contains(COL_NAME4)).isFalse(); + assertThat(tableMetadata.getClusteringKeyNames().contains("new_" + COL_NAME4)).isTrue(); + assertThat(tableMetadata.getEncryptedColumnNames().contains(COL_NAME10)).isFalse(); + assertThat(tableMetadata.getEncryptedColumnNames().contains("new_" + COL_NAME10)).isTrue(); + } + + @Test + public void builder_RenameSecondaryIndex_ShouldRenameSecondaryIndexInTableMetadataCorrectly() { + // Arrange + TableMetadata.Builder builder = + TableMetadata.newBuilder() + .addColumn(COL_NAME1, DataType.INT) + .addColumn(COL_NAME2, DataType.TEXT) + .addColumn(COL_NAME3, DataType.TEXT) + .addColumn(COL_NAME4, DataType.INT) + .addColumn(COL_NAME5, DataType.INT) + .addColumn(COL_NAME6, DataType.TEXT) + .addPartitionKey(COL_NAME2) + .addPartitionKey(COL_NAME1) + .addClusteringKey(COL_NAME4, Order.ASC) + .addClusteringKey(COL_NAME3, Order.DESC) + .addSecondaryIndex(COL_NAME5) + .addSecondaryIndex(COL_NAME6); + + // Act + TableMetadata tableMetadata = + builder.renameSecondaryIndex(COL_NAME5, "new_" + COL_NAME5).build(); + + // Assert + assertThat(tableMetadata.getSecondaryIndexNames().size()).isEqualTo(2); + assertThat(tableMetadata.getSecondaryIndexNames().contains(COL_NAME5)).isFalse(); + assertThat(tableMetadata.getSecondaryIndexNames().contains("new_" + COL_NAME5)).isTrue(); + assertThat(tableMetadata.getSecondaryIndexNames().contains(COL_NAME6)).isTrue(); + } } 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 index f08bc0f9da..b707a2fbda 100644 --- a/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java @@ -1222,23 +1222,31 @@ public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly() .addColumn(getColumnName1(), DataType.INT) .addColumn(getColumnName2(), DataType.INT) .addColumn(getColumnName3(), DataType.TEXT) + .addColumn(getColumnName4(), DataType.INT) + .addColumn(getColumnName5(), DataType.INT) .addPartitionKey(getColumnName1()) - .addClusteringKey(getColumnName2()) + .addPartitionKey(getColumnName2()) + .addClusteringKey(getColumnName3()) + .addClusteringKey(getColumnName4()) .build(); admin.createTable(namespace1, getTable4(), currentTableMetadata, options); // Act - admin.renameColumn(namespace1, getTable4(), getColumnName1(), getColumnName4()); - admin.renameColumn(namespace1, getTable4(), getColumnName2(), getColumnName5()); + admin.renameColumn(namespace1, getTable4(), getColumnName1(), getColumnName6()); + admin.renameColumn(namespace1, getTable4(), getColumnName3(), getColumnName7()); // Assert TableMetadata expectedTableMetadata = TableMetadata.newBuilder() + .addColumn(getColumnName6(), DataType.INT) + .addColumn(getColumnName2(), DataType.INT) + .addColumn(getColumnName7(), DataType.TEXT) .addColumn(getColumnName4(), DataType.INT) .addColumn(getColumnName5(), DataType.INT) - .addColumn(getColumnName3(), DataType.TEXT) - .addPartitionKey(getColumnName4()) - .addClusteringKey(getColumnName5()) + .addPartitionKey(getColumnName6()) + .addPartitionKey(getColumnName2()) + .addClusteringKey(getColumnName7()) + .addClusteringKey(getColumnName4()) .build(); assertThat(admin.getTableMetadata(namespace1, getTable4())).isEqualTo(expectedTableMetadata); } finally { 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 index 46bcc54339..6c30118415 100644 --- a/integration-test/src/main/java/com/scalar/db/api/DistributedTransactionAdminIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/api/DistributedTransactionAdminIntegrationTestBase.java @@ -1090,23 +1090,31 @@ public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly() .addColumn("c1", DataType.INT) .addColumn("c2", DataType.INT) .addColumn("c3", DataType.TEXT) + .addColumn("c4", DataType.INT) + .addColumn("c5", DataType.INT) .addPartitionKey("c1") - .addClusteringKey("c2") + .addPartitionKey("c2") + .addClusteringKey("c3") + .addClusteringKey("c4") .build(); admin.createTable(namespace1, TABLE4, currentTableMetadata, options); // Act - admin.renameColumn(namespace1, TABLE4, "c1", "c4"); - admin.renameColumn(namespace1, TABLE4, "c2", "c5"); + admin.renameColumn(namespace1, TABLE4, "c1", "c6"); + admin.renameColumn(namespace1, TABLE4, "c3", "c7"); // Assert TableMetadata expectedTableMetadata = TableMetadata.newBuilder() + .addColumn("c6", DataType.INT) + .addColumn("c2", DataType.INT) + .addColumn("c7", DataType.TEXT) .addColumn("c4", DataType.INT) .addColumn("c5", DataType.INT) - .addColumn("c3", DataType.TEXT) - .addPartitionKey("c4") - .addClusteringKey("c5") + .addPartitionKey("c6") + .addPartitionKey("c2") + .addClusteringKey("c7") + .addClusteringKey("c4") .build(); assertThat(admin.getTableMetadata(namespace1, TABLE4)).isEqualTo(expectedTableMetadata); } finally { From 5ef1a032d28d67bdf1759e56f37f51fe70d63f70 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Tue, 9 Sep 2025 17:29:25 +0900 Subject: [PATCH 10/15] Fix to handle rename indexed column with Cassandra --- ...raAdminCaseSensitivityIntegrationTest.java | 42 ++++++++++++++++++- .../CassandraAdminIntegrationTest.java | 42 ++++++++++++++++++- ...mmitAdminIntegrationTestWithCassandra.java | 41 +++++++++++++++++- ...tionAdminIntegrationTestWithCassandra.java | 41 +++++++++++++++++- .../db/storage/cassandra/CassandraAdmin.java | 5 +++ ...ibutedStorageAdminIntegrationTestBase.java | 2 +- 6 files changed, 164 insertions(+), 9 deletions(-) diff --git a/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminCaseSensitivityIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminCaseSensitivityIntegrationTest.java index e0a8b111e2..b22494b7e5 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminCaseSensitivityIntegrationTest.java +++ b/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminCaseSensitivityIntegrationTest.java @@ -1,6 +1,11 @@ package com.scalar.db.storage.cassandra; +import static org.assertj.core.api.Assertions.assertThat; + import com.scalar.db.api.DistributedStorageAdminCaseSensitivityIntegrationTestBase; +import com.scalar.db.api.TableMetadata; +import com.scalar.db.exception.storage.ExecutionException; +import com.scalar.db.io.DataType; import com.scalar.db.util.AdminTestUtils; import java.util.Collections; import java.util.Map; @@ -34,6 +39,39 @@ protected boolean isTimestampTypeSupported() { public void renameColumn_ShouldRenameColumnCorrectly() {} @Override - @Disabled("Renaming non-primary key columns is not supported in Cassandra") - public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() {} + public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() + throws ExecutionException { + try { + // Arrange + Map options = getCreationOptions(); + TableMetadata currentTableMetadata = + TableMetadata.newBuilder() + .addColumn(getColumnName1(), DataType.INT) + .addColumn(getColumnName2(), DataType.INT) + .addColumn(getColumnName3(), DataType.TEXT) + .addPartitionKey(getColumnName1()) + .addSecondaryIndex(getColumnName1()) + .build(); + admin.createTable(getNamespace1(), getTable4(), currentTableMetadata, options); + + // Act + admin.renameColumn(getNamespace1(), getTable4(), getColumnName1(), getColumnName4()); + + // Assert + TableMetadata expectedTableMetadata = + TableMetadata.newBuilder() + .addColumn(getColumnName4(), DataType.INT) + .addColumn(getColumnName2(), DataType.INT) + .addColumn(getColumnName3(), DataType.TEXT) + .addPartitionKey(getColumnName4()) + .addSecondaryIndex(getColumnName4()) + .build(); + assertThat(admin.getTableMetadata(getNamespace1(), getTable4())) + .isEqualTo(expectedTableMetadata); + assertThat(admin.indexExists(getNamespace1(), getTable4(), getColumnName1())).isFalse(); + assertThat(admin.indexExists(getNamespace1(), getTable4(), getColumnName4())).isTrue(); + } finally { + admin.dropTable(getNamespace1(), getTable4(), true); + } + } } diff --git a/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminIntegrationTest.java index 46c8fe9be1..c8b0475b0d 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminIntegrationTest.java +++ b/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminIntegrationTest.java @@ -1,6 +1,11 @@ package com.scalar.db.storage.cassandra; +import static org.assertj.core.api.Assertions.assertThat; + import com.scalar.db.api.DistributedStorageAdminIntegrationTestBase; +import com.scalar.db.api.TableMetadata; +import com.scalar.db.exception.storage.ExecutionException; +import com.scalar.db.io.DataType; import com.scalar.db.util.AdminTestUtils; import java.util.Collections; import java.util.Map; @@ -33,6 +38,39 @@ protected boolean isTimestampTypeSupported() { public void renameColumn_ShouldRenameColumnCorrectly() {} @Override - @Disabled("Renaming non-primary key columns is not supported in Cassandra") - public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() {} + public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() + throws ExecutionException { + try { + // Arrange + Map options = getCreationOptions(); + TableMetadata currentTableMetadata = + TableMetadata.newBuilder() + .addColumn(getColumnName1(), DataType.INT) + .addColumn(getColumnName2(), DataType.INT) + .addColumn(getColumnName3(), DataType.TEXT) + .addPartitionKey(getColumnName1()) + .addSecondaryIndex(getColumnName1()) + .build(); + admin.createTable(getNamespace1(), getTable4(), currentTableMetadata, options); + + // Act + admin.renameColumn(getNamespace1(), getTable4(), getColumnName1(), getColumnName4()); + + // Assert + TableMetadata expectedTableMetadata = + TableMetadata.newBuilder() + .addColumn(getColumnName4(), DataType.INT) + .addColumn(getColumnName2(), DataType.INT) + .addColumn(getColumnName3(), DataType.TEXT) + .addPartitionKey(getColumnName4()) + .addSecondaryIndex(getColumnName4()) + .build(); + assertThat(admin.getTableMetadata(getNamespace1(), getTable4())) + .isEqualTo(expectedTableMetadata); + assertThat(admin.indexExists(getNamespace1(), getTable4(), getColumnName1())).isFalse(); + assertThat(admin.indexExists(getNamespace1(), getTable4(), getColumnName4())).isTrue(); + } finally { + admin.dropTable(getNamespace1(), getTable4(), true); + } + } } diff --git a/core/src/integration-test/java/com/scalar/db/storage/cassandra/ConsensusCommitAdminIntegrationTestWithCassandra.java b/core/src/integration-test/java/com/scalar/db/storage/cassandra/ConsensusCommitAdminIntegrationTestWithCassandra.java index b8b8b1d12e..85619a0dba 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/cassandra/ConsensusCommitAdminIntegrationTestWithCassandra.java +++ b/core/src/integration-test/java/com/scalar/db/storage/cassandra/ConsensusCommitAdminIntegrationTestWithCassandra.java @@ -1,5 +1,10 @@ package com.scalar.db.storage.cassandra; +import static org.assertj.core.api.Assertions.assertThat; + +import com.scalar.db.api.TableMetadata; +import com.scalar.db.exception.storage.ExecutionException; +import com.scalar.db.io.DataType; import com.scalar.db.transaction.consensuscommit.ConsensusCommitAdminIntegrationTestBase; import com.scalar.db.util.AdminTestUtils; import java.util.Collections; @@ -34,6 +39,38 @@ protected boolean isTimestampTypeSupported() { public void renameColumn_ShouldRenameColumnCorrectly() {} @Override - @Disabled("Renaming non-primary key columns is not supported in Cassandra") - public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() {} + public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() + throws ExecutionException { + try { + // Arrange + Map options = getCreationOptions(); + TableMetadata currentTableMetadata = + TableMetadata.newBuilder() + .addColumn("c1", DataType.INT) + .addColumn("c2", DataType.INT) + .addColumn("c3", DataType.TEXT) + .addPartitionKey("c1") + .addSecondaryIndex("c1") + .build(); + admin.createTable(namespace1, TABLE4, currentTableMetadata, options); + + // Act + admin.renameColumn(namespace1, TABLE4, "c1", "c4"); + + // Assert + TableMetadata expectedTableMetadata = + TableMetadata.newBuilder() + .addColumn("c4", DataType.INT) + .addColumn("c2", DataType.INT) + .addColumn("c3", DataType.TEXT) + .addPartitionKey("c4") + .addSecondaryIndex("c4") + .build(); + assertThat(admin.getTableMetadata(namespace1, TABLE4)).isEqualTo(expectedTableMetadata); + assertThat(admin.indexExists(namespace1, TABLE4, "c1")).isFalse(); + assertThat(admin.indexExists(namespace1, TABLE4, "c4")).isTrue(); + } finally { + admin.dropTable(namespace1, TABLE4, true); + } + } } diff --git a/core/src/integration-test/java/com/scalar/db/storage/cassandra/SingleCrudOperationTransactionAdminIntegrationTestWithCassandra.java b/core/src/integration-test/java/com/scalar/db/storage/cassandra/SingleCrudOperationTransactionAdminIntegrationTestWithCassandra.java index 05b48a0af8..b6b93b7071 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/cassandra/SingleCrudOperationTransactionAdminIntegrationTestWithCassandra.java +++ b/core/src/integration-test/java/com/scalar/db/storage/cassandra/SingleCrudOperationTransactionAdminIntegrationTestWithCassandra.java @@ -1,5 +1,10 @@ package com.scalar.db.storage.cassandra; +import static org.assertj.core.api.Assertions.assertThat; + +import com.scalar.db.api.TableMetadata; +import com.scalar.db.exception.storage.ExecutionException; +import com.scalar.db.io.DataType; import com.scalar.db.transaction.singlecrudoperation.SingleCrudOperationTransactionAdminIntegrationTestBase; import java.util.Collections; import java.util.Map; @@ -29,6 +34,38 @@ protected boolean isTimestampTypeSupported() { public void renameColumn_ShouldRenameColumnCorrectly() {} @Override - @Disabled("Renaming non-primary key columns is not supported in Cassandra") - public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() {} + public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() + throws ExecutionException { + try { + // Arrange + Map options = getCreationOptions(); + TableMetadata currentTableMetadata = + TableMetadata.newBuilder() + .addColumn("c1", DataType.INT) + .addColumn("c2", DataType.INT) + .addColumn("c3", DataType.TEXT) + .addPartitionKey("c1") + .addSecondaryIndex("c1") + .build(); + admin.createTable(namespace1, TABLE4, currentTableMetadata, options); + + // Act + admin.renameColumn(namespace1, TABLE4, "c1", "c4"); + + // Assert + TableMetadata expectedTableMetadata = + TableMetadata.newBuilder() + .addColumn("c4", DataType.INT) + .addColumn("c2", DataType.INT) + .addColumn("c3", DataType.TEXT) + .addPartitionKey("c4") + .addSecondaryIndex("c4") + .build(); + assertThat(admin.getTableMetadata(namespace1, TABLE4)).isEqualTo(expectedTableMetadata); + assertThat(admin.indexExists(namespace1, TABLE4, "c1")).isFalse(); + assertThat(admin.indexExists(namespace1, TABLE4, "c4")).isTrue(); + } finally { + admin.dropTable(namespace1, TABLE4, true); + } + } } diff --git a/core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java b/core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java index bbb74d9837..63f7383fbf 100644 --- a/core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java +++ b/core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java @@ -455,6 +455,11 @@ public void renameColumn( .getQueryString(); clusterManager.getSession().execute(alterTableQuery); + if (tableMetadata.getSecondaryIndexNames().contains(oldColumnName)) { + // Cassandra does not support renaming indexes + dropIndex(namespace, table, oldColumnName); + createIndex(namespace, table, newColumnName, Collections.emptyMap()); + } } catch (IllegalArgumentException e) { throw e; } catch (RuntimeException e) { 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 index b707a2fbda..3665b7c2b4 100644 --- a/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java @@ -59,7 +59,7 @@ public abstract class DistributedStorageAdminIntegrationTestBase { private static final String COL_NAME14 = "c14"; private static final String COL_NAME15 = "c15"; private StorageFactory storageFactory; - private DistributedStorageAdmin admin; + protected DistributedStorageAdmin admin; private String namespace1; private String namespace2; private String namespace3; From d1542baa598ca7b3d087e2d6eff0100dfeace9dd Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Tue, 9 Sep 2025 19:50:16 +0900 Subject: [PATCH 11/15] Fix to handle renaming a primary key column specified as an index key --- .../main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java | 6 ++++-- .../api/DistributedStorageAdminIntegrationTestBase.java | 9 +++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) 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 index 48db6d875b..dd31d56914 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java @@ -895,9 +895,11 @@ public void renameColumn( TableMetadata.newBuilder(currentTableMetadata).renameColumn(oldColumnName, newColumnName); if (currentTableMetadata.getPartitionKeyNames().contains(oldColumnName)) { tableMetadataBuilder.renamePartitionKey(oldColumnName, newColumnName); - } else if (currentTableMetadata.getClusteringKeyNames().contains(oldColumnName)) { + } + if (currentTableMetadata.getClusteringKeyNames().contains(oldColumnName)) { tableMetadataBuilder.renameClusteringKey(oldColumnName, newColumnName); - } else if (currentTableMetadata.getSecondaryIndexNames().contains(oldColumnName)) { + } + if (currentTableMetadata.getSecondaryIndexNames().contains(oldColumnName)) { tableMetadataBuilder.renameSecondaryIndex(oldColumnName, newColumnName); } TableMetadata updatedTableMetadata = tableMetadataBuilder.build(); 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 index 3665b7c2b4..b9691fe5a7 100644 --- a/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java @@ -1266,23 +1266,28 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() .addColumn(getColumnName2(), DataType.INT) .addColumn(getColumnName3(), DataType.TEXT) .addPartitionKey(getColumnName1()) + .addSecondaryIndex(getColumnName1()) .addSecondaryIndex(getColumnName3()) .build(); admin.createTable(namespace1, getTable4(), currentTableMetadata, options); // Act + admin.renameColumn(namespace1, getTable4(), getColumnName1(), getColumnName5()); admin.renameColumn(namespace1, getTable4(), getColumnName3(), getColumnName4()); // Assert TableMetadata expectedTableMetadata = TableMetadata.newBuilder() - .addColumn(getColumnName1(), DataType.INT) + .addColumn(getColumnName5(), DataType.INT) .addColumn(getColumnName2(), DataType.INT) .addColumn(getColumnName4(), DataType.TEXT) - .addPartitionKey(getColumnName1()) + .addPartitionKey(getColumnName5()) .addSecondaryIndex(getColumnName4()) + .addSecondaryIndex(getColumnName5()) .build(); assertThat(admin.getTableMetadata(namespace1, getTable4())).isEqualTo(expectedTableMetadata); + assertThat(admin.indexExists(namespace1, getTable4(), getColumnName1())).isFalse(); + assertThat(admin.indexExists(namespace1, getTable4(), getColumnName5())).isTrue(); assertThat(admin.indexExists(namespace1, getTable4(), getColumnName3())).isFalse(); assertThat(admin.indexExists(namespace1, getTable4(), getColumnName4())).isTrue(); } finally { From f87a79ed36ad569b5cd5644b7b81173f88482d0b Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Wed, 10 Sep 2025 09:04:38 +0900 Subject: [PATCH 12/15] Fix integration tests --- .../cassandra/CassandraAdminCaseSensitivityIntegrationTest.java | 2 ++ .../db/storage/cassandra/CassandraAdminIntegrationTest.java | 2 ++ .../ConsensusCommitAdminIntegrationTestWithCassandra.java | 2 ++ ...udOperationTransactionAdminIntegrationTestWithCassandra.java | 2 ++ .../db/api/DistributedStorageAdminIntegrationTestBase.java | 2 ++ .../db/api/DistributedTransactionAdminIntegrationTestBase.java | 2 ++ 6 files changed, 12 insertions(+) diff --git a/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminCaseSensitivityIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminCaseSensitivityIntegrationTest.java index b22494b7e5..c596f503bd 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminCaseSensitivityIntegrationTest.java +++ b/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminCaseSensitivityIntegrationTest.java @@ -50,6 +50,7 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() .addColumn(getColumnName2(), DataType.INT) .addColumn(getColumnName3(), DataType.TEXT) .addPartitionKey(getColumnName1()) + .addClusteringKey(getColumnName2()) .addSecondaryIndex(getColumnName1()) .build(); admin.createTable(getNamespace1(), getTable4(), currentTableMetadata, options); @@ -64,6 +65,7 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() .addColumn(getColumnName2(), DataType.INT) .addColumn(getColumnName3(), DataType.TEXT) .addPartitionKey(getColumnName4()) + .addClusteringKey(getColumnName2()) .addSecondaryIndex(getColumnName4()) .build(); assertThat(admin.getTableMetadata(getNamespace1(), getTable4())) diff --git a/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminIntegrationTest.java index c8b0475b0d..6c0ffc90a2 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminIntegrationTest.java +++ b/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminIntegrationTest.java @@ -49,6 +49,7 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() .addColumn(getColumnName2(), DataType.INT) .addColumn(getColumnName3(), DataType.TEXT) .addPartitionKey(getColumnName1()) + .addClusteringKey(getColumnName2()) .addSecondaryIndex(getColumnName1()) .build(); admin.createTable(getNamespace1(), getTable4(), currentTableMetadata, options); @@ -63,6 +64,7 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() .addColumn(getColumnName2(), DataType.INT) .addColumn(getColumnName3(), DataType.TEXT) .addPartitionKey(getColumnName4()) + .addClusteringKey(getColumnName2()) .addSecondaryIndex(getColumnName4()) .build(); assertThat(admin.getTableMetadata(getNamespace1(), getTable4())) diff --git a/core/src/integration-test/java/com/scalar/db/storage/cassandra/ConsensusCommitAdminIntegrationTestWithCassandra.java b/core/src/integration-test/java/com/scalar/db/storage/cassandra/ConsensusCommitAdminIntegrationTestWithCassandra.java index 85619a0dba..b4b0cdf60f 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/cassandra/ConsensusCommitAdminIntegrationTestWithCassandra.java +++ b/core/src/integration-test/java/com/scalar/db/storage/cassandra/ConsensusCommitAdminIntegrationTestWithCassandra.java @@ -50,6 +50,7 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() .addColumn("c2", DataType.INT) .addColumn("c3", DataType.TEXT) .addPartitionKey("c1") + .addClusteringKey("c2") .addSecondaryIndex("c1") .build(); admin.createTable(namespace1, TABLE4, currentTableMetadata, options); @@ -64,6 +65,7 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() .addColumn("c2", DataType.INT) .addColumn("c3", DataType.TEXT) .addPartitionKey("c4") + .addClusteringKey("c2") .addSecondaryIndex("c4") .build(); assertThat(admin.getTableMetadata(namespace1, TABLE4)).isEqualTo(expectedTableMetadata); diff --git a/core/src/integration-test/java/com/scalar/db/storage/cassandra/SingleCrudOperationTransactionAdminIntegrationTestWithCassandra.java b/core/src/integration-test/java/com/scalar/db/storage/cassandra/SingleCrudOperationTransactionAdminIntegrationTestWithCassandra.java index b6b93b7071..89c268425e 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/cassandra/SingleCrudOperationTransactionAdminIntegrationTestWithCassandra.java +++ b/core/src/integration-test/java/com/scalar/db/storage/cassandra/SingleCrudOperationTransactionAdminIntegrationTestWithCassandra.java @@ -45,6 +45,7 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() .addColumn("c2", DataType.INT) .addColumn("c3", DataType.TEXT) .addPartitionKey("c1") + .addClusteringKey("c2") .addSecondaryIndex("c1") .build(); admin.createTable(namespace1, TABLE4, currentTableMetadata, options); @@ -59,6 +60,7 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() .addColumn("c2", DataType.INT) .addColumn("c3", DataType.TEXT) .addPartitionKey("c4") + .addClusteringKey("c2") .addSecondaryIndex("c4") .build(); assertThat(admin.getTableMetadata(namespace1, TABLE4)).isEqualTo(expectedTableMetadata); 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 index b9691fe5a7..a4b3ac9284 100644 --- a/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java @@ -1266,6 +1266,7 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() .addColumn(getColumnName2(), DataType.INT) .addColumn(getColumnName3(), DataType.TEXT) .addPartitionKey(getColumnName1()) + .addClusteringKey(getColumnName2()) .addSecondaryIndex(getColumnName1()) .addSecondaryIndex(getColumnName3()) .build(); @@ -1282,6 +1283,7 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() .addColumn(getColumnName2(), DataType.INT) .addColumn(getColumnName4(), DataType.TEXT) .addPartitionKey(getColumnName5()) + .addClusteringKey(getColumnName2()) .addSecondaryIndex(getColumnName4()) .addSecondaryIndex(getColumnName5()) .build(); 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 index 6c30118415..a5877044ba 100644 --- a/integration-test/src/main/java/com/scalar/db/api/DistributedTransactionAdminIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/api/DistributedTransactionAdminIntegrationTestBase.java @@ -1134,6 +1134,7 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() .addColumn("c2", DataType.INT) .addColumn("c3", DataType.TEXT) .addPartitionKey("c1") + .addClusteringKey("c2") .addSecondaryIndex("c3") .build(); admin.createTable(namespace1, TABLE4, currentTableMetadata, options); @@ -1148,6 +1149,7 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() .addColumn("c2", DataType.INT) .addColumn("c4", DataType.TEXT) .addPartitionKey("c1") + .addClusteringKey("c2") .addSecondaryIndex("c4") .build(); assertThat(admin.getTableMetadata(namespace1, TABLE4)).isEqualTo(expectedTableMetadata); From c03a8e0bc814a74d6daa174b32af228ecfe5e4f0 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Thu, 11 Sep 2025 12:34:55 +0900 Subject: [PATCH 13/15] Apply suggestions --- ...tAdminIntegrationTestWithJdbcDatabase.java | 36 +++++++++++++++ ...bcAdminCaseSensitivityIntegrationTest.java | 45 +++++++++++++++++++ .../jdbc/JdbcAdminIntegrationTest.java | 45 +++++++++++++++++++ ...nAdminIntegrationTestWithJdbcDatabase.java | 36 +++++++++++++++ .../JdbcTransactionAdminIntegrationTest.java | 36 +++++++++++++++ .../java/com/scalar/db/common/CoreError.java | 18 ++++---- .../DecoratedDistributedTransactionAdmin.java | 14 +++--- .../com/scalar/db/storage/jdbc/JdbcAdmin.java | 12 +++++ 8 files changed, 226 insertions(+), 16 deletions(-) diff --git a/core/src/integration-test/java/com/scalar/db/storage/jdbc/ConsensusCommitAdminIntegrationTestWithJdbcDatabase.java b/core/src/integration-test/java/com/scalar/db/storage/jdbc/ConsensusCommitAdminIntegrationTestWithJdbcDatabase.java index 1667b67122..f9a1db19ba 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/jdbc/ConsensusCommitAdminIntegrationTestWithJdbcDatabase.java +++ b/core/src/integration-test/java/com/scalar/db/storage/jdbc/ConsensusCommitAdminIntegrationTestWithJdbcDatabase.java @@ -1,12 +1,18 @@ package com.scalar.db.storage.jdbc; +import static org.assertj.core.api.Assertions.assertThatCode; + +import com.scalar.db.api.TableMetadata; import com.scalar.db.config.DatabaseConfig; import com.scalar.db.exception.storage.ExecutionException; +import com.scalar.db.io.DataType; import com.scalar.db.transaction.consensuscommit.ConsensusCommitAdminIntegrationTestBase; import com.scalar.db.util.AdminTestUtils; +import java.util.Map; import java.util.Properties; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledIf; +import org.junit.jupiter.api.condition.EnabledIf; public class ConsensusCommitAdminIntegrationTestWithJdbcDatabase extends ConsensusCommitAdminIntegrationTestBase { @@ -53,4 +59,34 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() throws ExecutionException { super.renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly(); } + + @Test + @EnabledIf("isDb2") + public void renameColumn_Db2_ForPrimaryOrIndexKeyColumn_ShouldThrowUnsupportedOperationException() + throws ExecutionException { + try { + // Arrange + Map options = getCreationOptions(); + TableMetadata currentTableMetadata = + TableMetadata.newBuilder() + .addColumn(COL_NAME1, DataType.INT) + .addColumn(COL_NAME2, DataType.INT) + .addColumn(COL_NAME3, DataType.TEXT) + .addPartitionKey(COL_NAME1) + .addClusteringKey(COL_NAME2) + .addSecondaryIndex(COL_NAME3) + .build(); + admin.createTable(namespace1, TABLE4, currentTableMetadata, options); + + // Act Assert + assertThatCode(() -> admin.renameColumn(namespace1, TABLE4, COL_NAME1, COL_NAME4)) + .isInstanceOf(UnsupportedOperationException.class); + assertThatCode(() -> admin.renameColumn(namespace1, TABLE4, COL_NAME2, COL_NAME4)) + .isInstanceOf(UnsupportedOperationException.class); + assertThatCode(() -> admin.renameColumn(namespace1, TABLE4, COL_NAME3, COL_NAME4)) + .isInstanceOf(UnsupportedOperationException.class); + } finally { + admin.dropTable(namespace1, TABLE4, true); + } + } } diff --git a/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminCaseSensitivityIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminCaseSensitivityIntegrationTest.java index 376c83a78d..66708e09ea 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminCaseSensitivityIntegrationTest.java +++ b/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminCaseSensitivityIntegrationTest.java @@ -1,12 +1,18 @@ package com.scalar.db.storage.jdbc; +import static org.assertj.core.api.Assertions.assertThatCode; + import com.scalar.db.api.DistributedStorageAdminCaseSensitivityIntegrationTestBase; +import com.scalar.db.api.TableMetadata; import com.scalar.db.config.DatabaseConfig; import com.scalar.db.exception.storage.ExecutionException; +import com.scalar.db.io.DataType; import com.scalar.db.util.AdminTestUtils; +import java.util.Map; import java.util.Properties; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledIf; +import org.junit.jupiter.api.condition.EnabledIf; public class JdbcAdminCaseSensitivityIntegrationTest extends DistributedStorageAdminCaseSensitivityIntegrationTestBase { @@ -52,4 +58,43 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() throws ExecutionException { super.renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly(); } + + @Test + @EnabledIf("isDb2") + public void renameColumn_Db2_ForPrimaryOrIndexKeyColumn_ShouldThrowUnsupportedOperationException() + throws ExecutionException { + try { + // Arrange + Map options = getCreationOptions(); + TableMetadata currentTableMetadata = + TableMetadata.newBuilder() + .addColumn(getColumnName1(), DataType.INT) + .addColumn(getColumnName2(), DataType.INT) + .addColumn(getColumnName3(), DataType.TEXT) + .addPartitionKey(getColumnName1()) + .addClusteringKey(getColumnName2()) + .addSecondaryIndex(getColumnName3()) + .build(); + admin.createTable(getNamespace1(), getTable4(), currentTableMetadata, options); + + // Act Assert + assertThatCode( + () -> + admin.renameColumn( + getNamespace1(), getTable4(), getColumnName1(), getColumnName4())) + .isInstanceOf(UnsupportedOperationException.class); + assertThatCode( + () -> + admin.renameColumn( + getNamespace1(), getTable4(), getColumnName2(), getColumnName4())) + .isInstanceOf(UnsupportedOperationException.class); + assertThatCode( + () -> + admin.renameColumn( + getNamespace1(), getTable4(), getColumnName3(), getColumnName4())) + .isInstanceOf(UnsupportedOperationException.class); + } finally { + admin.dropTable(getNamespace1(), getTable4(), true); + } + } } diff --git a/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminIntegrationTest.java index 9c7fe1dc28..ed757ba490 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminIntegrationTest.java +++ b/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminIntegrationTest.java @@ -1,12 +1,18 @@ package com.scalar.db.storage.jdbc; +import static org.assertj.core.api.Assertions.assertThatCode; + import com.scalar.db.api.DistributedStorageAdminIntegrationTestBase; +import com.scalar.db.api.TableMetadata; import com.scalar.db.config.DatabaseConfig; import com.scalar.db.exception.storage.ExecutionException; +import com.scalar.db.io.DataType; import com.scalar.db.util.AdminTestUtils; +import java.util.Map; import java.util.Properties; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledIf; +import org.junit.jupiter.api.condition.EnabledIf; public class JdbcAdminIntegrationTest extends DistributedStorageAdminIntegrationTestBase { private RdbEngineStrategy rdbEngine; @@ -51,4 +57,43 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() throws ExecutionException { super.renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly(); } + + @Test + @EnabledIf("isDb2") + public void renameColumn_Db2_ForPrimaryOrIndexKeyColumn_ShouldThrowUnsupportedOperationException() + throws ExecutionException { + try { + // Arrange + Map options = getCreationOptions(); + TableMetadata currentTableMetadata = + TableMetadata.newBuilder() + .addColumn(getColumnName1(), DataType.INT) + .addColumn(getColumnName2(), DataType.INT) + .addColumn(getColumnName3(), DataType.TEXT) + .addPartitionKey(getColumnName1()) + .addClusteringKey(getColumnName2()) + .addSecondaryIndex(getColumnName3()) + .build(); + admin.createTable(getNamespace1(), getTable4(), currentTableMetadata, options); + + // Act Assert + assertThatCode( + () -> + admin.renameColumn( + getNamespace1(), getTable4(), getColumnName1(), getColumnName4())) + .isInstanceOf(UnsupportedOperationException.class); + assertThatCode( + () -> + admin.renameColumn( + getNamespace1(), getTable4(), getColumnName2(), getColumnName4())) + .isInstanceOf(UnsupportedOperationException.class); + assertThatCode( + () -> + admin.renameColumn( + getNamespace1(), getTable4(), getColumnName3(), getColumnName4())) + .isInstanceOf(UnsupportedOperationException.class); + } finally { + admin.dropTable(getNamespace1(), getTable4(), true); + } + } } diff --git a/core/src/integration-test/java/com/scalar/db/storage/jdbc/SingleCrudOperationTransactionAdminIntegrationTestWithJdbcDatabase.java b/core/src/integration-test/java/com/scalar/db/storage/jdbc/SingleCrudOperationTransactionAdminIntegrationTestWithJdbcDatabase.java index f14d0e618b..bbadccc670 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/jdbc/SingleCrudOperationTransactionAdminIntegrationTestWithJdbcDatabase.java +++ b/core/src/integration-test/java/com/scalar/db/storage/jdbc/SingleCrudOperationTransactionAdminIntegrationTestWithJdbcDatabase.java @@ -1,12 +1,18 @@ package com.scalar.db.storage.jdbc; +import static org.assertj.core.api.Assertions.assertThatCode; + +import com.scalar.db.api.TableMetadata; import com.scalar.db.config.DatabaseConfig; import com.scalar.db.exception.storage.ExecutionException; +import com.scalar.db.io.DataType; import com.scalar.db.transaction.singlecrudoperation.SingleCrudOperationTransactionAdminIntegrationTestBase; import com.scalar.db.util.AdminTestUtils; +import java.util.Map; import java.util.Properties; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledIf; +import org.junit.jupiter.api.condition.EnabledIf; public class SingleCrudOperationTransactionAdminIntegrationTestWithJdbcDatabase extends SingleCrudOperationTransactionAdminIntegrationTestBase { @@ -53,4 +59,34 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() throws ExecutionException { super.renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly(); } + + @Test + @EnabledIf("isDb2") + public void renameColumn_Db2_ForPrimaryOrIndexKeyColumn_ShouldThrowUnsupportedOperationException() + throws ExecutionException { + try { + // Arrange + Map options = getCreationOptions(); + TableMetadata currentTableMetadata = + TableMetadata.newBuilder() + .addColumn(COL_NAME1, DataType.INT) + .addColumn(COL_NAME2, DataType.INT) + .addColumn(COL_NAME3, DataType.TEXT) + .addPartitionKey(COL_NAME1) + .addClusteringKey(COL_NAME2) + .addSecondaryIndex(COL_NAME3) + .build(); + admin.createTable(namespace1, TABLE4, currentTableMetadata, options); + + // Act Assert + assertThatCode(() -> admin.renameColumn(namespace1, TABLE4, COL_NAME1, COL_NAME4)) + .isInstanceOf(UnsupportedOperationException.class); + assertThatCode(() -> admin.renameColumn(namespace1, TABLE4, COL_NAME2, COL_NAME4)) + .isInstanceOf(UnsupportedOperationException.class); + assertThatCode(() -> admin.renameColumn(namespace1, TABLE4, COL_NAME3, COL_NAME4)) + .isInstanceOf(UnsupportedOperationException.class); + } finally { + admin.dropTable(namespace1, TABLE4, true); + } + } } diff --git a/core/src/integration-test/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdminIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdminIntegrationTest.java index cc83e15dd0..2a85669b85 100644 --- a/core/src/integration-test/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdminIntegrationTest.java +++ b/core/src/integration-test/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdminIntegrationTest.java @@ -1,18 +1,24 @@ package com.scalar.db.transaction.jdbc; +import static org.assertj.core.api.Assertions.assertThatCode; + import com.scalar.db.api.DistributedTransactionAdminIntegrationTestBase; +import com.scalar.db.api.TableMetadata; import com.scalar.db.config.DatabaseConfig; import com.scalar.db.exception.storage.ExecutionException; +import com.scalar.db.io.DataType; import com.scalar.db.storage.jdbc.JdbcConfig; import com.scalar.db.storage.jdbc.JdbcEnv; import com.scalar.db.storage.jdbc.JdbcTestUtils; import com.scalar.db.storage.jdbc.RdbEngineFactory; import com.scalar.db.storage.jdbc.RdbEngineStrategy; import com.scalar.db.util.AdminTestUtils; +import java.util.Map; import java.util.Properties; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledIf; +import org.junit.jupiter.api.condition.EnabledIf; public class JdbcTransactionAdminIntegrationTest extends DistributedTransactionAdminIntegrationTestBase { @@ -124,4 +130,34 @@ public void renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly() throws ExecutionException { super.renameColumn_ForIndexKeyColumn_ShouldRenameColumnAndIndexCorrectly(); } + + @Test + @EnabledIf("isDb2") + public void renameColumn_Db2_ForPrimaryOrIndexKeyColumn_ShouldThrowUnsupportedOperationException() + throws ExecutionException { + try { + // Arrange + Map options = getCreationOptions(); + TableMetadata currentTableMetadata = + TableMetadata.newBuilder() + .addColumn(COL_NAME1, DataType.INT) + .addColumn(COL_NAME2, DataType.INT) + .addColumn(COL_NAME3, DataType.TEXT) + .addPartitionKey(COL_NAME1) + .addClusteringKey(COL_NAME2) + .addSecondaryIndex(COL_NAME3) + .build(); + admin.createTable(namespace1, TABLE4, currentTableMetadata, options); + + // Act Assert + assertThatCode(() -> admin.renameColumn(namespace1, TABLE4, COL_NAME1, COL_NAME4)) + .isInstanceOf(UnsupportedOperationException.class); + assertThatCode(() -> admin.renameColumn(namespace1, TABLE4, COL_NAME2, COL_NAME4)) + .isInstanceOf(UnsupportedOperationException.class); + assertThatCode(() -> admin.renameColumn(namespace1, TABLE4, COL_NAME3, COL_NAME4)) + .isInstanceOf(UnsupportedOperationException.class); + } finally { + admin.dropTable(namespace1, TABLE4, true); + } + } } diff --git a/core/src/main/java/com/scalar/db/common/CoreError.java b/core/src/main/java/com/scalar/db/common/CoreError.java index d4b067db05..c97c20ac4a 100644 --- a/core/src/main/java/com/scalar/db/common/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/CoreError.java @@ -692,26 +692,26 @@ public enum CoreError implements ScalarDbError { ""), DYNAMO_DROP_COLUMN_NOT_SUPPORTED( Category.USER_ERROR, "0218", "DynamoDB does not support the dropping column feature", "", ""), - RENAME_PRIMARY_KEY_COLUMN_NOT_SUPPORTED( - Category.USER_ERROR, - "0219", - "Primary key columns cannot be renamed. Table: %s; Column: %s", - "", - ""), COSMOS_RENAME_COLUMN_NOT_SUPPORTED( Category.USER_ERROR, - "0220", + "0219", "Cosmos DB does not support the renaming column feature", "", ""), DYNAMO_RENAME_COLUMN_NOT_SUPPORTED( - Category.USER_ERROR, "0221", "DynamoDB does not support the renaming column feature", "", ""), + Category.USER_ERROR, "0220", "DynamoDB does not support the renaming column feature", "", ""), CASSANDRA_RENAME_NON_PRIMARY_KEY_COLUMN_NOT_SUPPORTED( Category.USER_ERROR, - "0222", + "0221", "Cassandra does not support renaming non-primary key columns", "", ""), + DB2_RENAME_PRIMARY_OR_INDEX_KEY_COLUMN_NOT_SUPPORTED( + Category.USER_ERROR, + "0222", + "Db2 does not support renaming primary key or index key columns", + "", + ""), // // Errors for the concurrency error category diff --git a/core/src/main/java/com/scalar/db/common/DecoratedDistributedTransactionAdmin.java b/core/src/main/java/com/scalar/db/common/DecoratedDistributedTransactionAdmin.java index 0b3f360acf..e8954c002b 100644 --- a/core/src/main/java/com/scalar/db/common/DecoratedDistributedTransactionAdmin.java +++ b/core/src/main/java/com/scalar/db/common/DecoratedDistributedTransactionAdmin.java @@ -187,13 +187,6 @@ public void addNewColumnToTable( distributedTransactionAdmin.addNewColumnToTable(namespace, table, columnName, columnType); } - @Override - public void renameColumn( - String namespace, String table, String oldColumnName, String newColumnName) - throws ExecutionException { - distributedTransactionAdmin.renameColumn(namespace, table, oldColumnName, newColumnName); - } - @Override public void addNewColumnToTable( String namespace, String table, String columnName, DataType columnType, boolean encrypted) @@ -228,6 +221,13 @@ public void dropColumnFromTable( distributedTransactionAdmin.dropColumnFromTable(namespace, table, columnName, ifExists); } + @Override + public void renameColumn( + String namespace, String table, String oldColumnName, String newColumnName) + throws ExecutionException { + distributedTransactionAdmin.renameColumn(namespace, table, oldColumnName, newColumnName); + } + @Override public void importTable(String namespace, String table, Map options) throws ExecutionException { 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 index dd31d56914..a4638d7c18 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java @@ -894,12 +894,24 @@ public void renameColumn( TableMetadata.Builder tableMetadataBuilder = TableMetadata.newBuilder(currentTableMetadata).renameColumn(oldColumnName, newColumnName); if (currentTableMetadata.getPartitionKeyNames().contains(oldColumnName)) { + if (rdbEngine instanceof RdbEngineDb2) { + throw new UnsupportedOperationException( + CoreError.DB2_RENAME_PRIMARY_OR_INDEX_KEY_COLUMN_NOT_SUPPORTED.buildMessage()); + } tableMetadataBuilder.renamePartitionKey(oldColumnName, newColumnName); } if (currentTableMetadata.getClusteringKeyNames().contains(oldColumnName)) { + if (rdbEngine instanceof RdbEngineDb2) { + throw new UnsupportedOperationException( + CoreError.DB2_RENAME_PRIMARY_OR_INDEX_KEY_COLUMN_NOT_SUPPORTED.buildMessage()); + } tableMetadataBuilder.renameClusteringKey(oldColumnName, newColumnName); } if (currentTableMetadata.getSecondaryIndexNames().contains(oldColumnName)) { + if (rdbEngine instanceof RdbEngineDb2) { + throw new UnsupportedOperationException( + CoreError.DB2_RENAME_PRIMARY_OR_INDEX_KEY_COLUMN_NOT_SUPPORTED.buildMessage()); + } tableMetadataBuilder.renameSecondaryIndex(oldColumnName, newColumnName); } TableMetadata updatedTableMetadata = tableMetadataBuilder.build(); From 16a06bb8a4beb371869bc63cb47993a26d9e0faa Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Thu, 11 Sep 2025 14:07:54 +0900 Subject: [PATCH 14/15] Apply suggestions --- .../main/java/com/scalar/db/common/CoreError.java | 2 +- .../java/com/scalar/db/storage/jdbc/JdbcAdmin.java | 13 +------------ .../com/scalar/db/storage/jdbc/RdbEngineDb2.java | 10 ++++++++++ .../scalar/db/storage/jdbc/RdbEngineStrategy.java | 9 +++++++++ 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/com/scalar/db/common/CoreError.java b/core/src/main/java/com/scalar/db/common/CoreError.java index c97c20ac4a..e683f5ea6d 100644 --- a/core/src/main/java/com/scalar/db/common/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/CoreError.java @@ -706,7 +706,7 @@ public enum CoreError implements ScalarDbError { "Cassandra does not support renaming non-primary key columns", "", ""), - DB2_RENAME_PRIMARY_OR_INDEX_KEY_COLUMN_NOT_SUPPORTED( + JDBC_DB2_RENAME_PRIMARY_OR_INDEX_KEY_COLUMN_NOT_SUPPORTED( Category.USER_ERROR, "0222", "Db2 does not support renaming primary key or index key columns", 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 index a4638d7c18..3dd4b9bd4c 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java @@ -890,28 +890,17 @@ public void renameColumn( throws ExecutionException { try { TableMetadata currentTableMetadata = getTableMetadata(namespace, table); + rdbEngine.throwIfRenameColumnNotSupported(oldColumnName, currentTableMetadata); assert currentTableMetadata != null; TableMetadata.Builder tableMetadataBuilder = TableMetadata.newBuilder(currentTableMetadata).renameColumn(oldColumnName, newColumnName); if (currentTableMetadata.getPartitionKeyNames().contains(oldColumnName)) { - if (rdbEngine instanceof RdbEngineDb2) { - throw new UnsupportedOperationException( - CoreError.DB2_RENAME_PRIMARY_OR_INDEX_KEY_COLUMN_NOT_SUPPORTED.buildMessage()); - } tableMetadataBuilder.renamePartitionKey(oldColumnName, newColumnName); } if (currentTableMetadata.getClusteringKeyNames().contains(oldColumnName)) { - if (rdbEngine instanceof RdbEngineDb2) { - throw new UnsupportedOperationException( - CoreError.DB2_RENAME_PRIMARY_OR_INDEX_KEY_COLUMN_NOT_SUPPORTED.buildMessage()); - } tableMetadataBuilder.renameClusteringKey(oldColumnName, newColumnName); } if (currentTableMetadata.getSecondaryIndexNames().contains(oldColumnName)) { - if (rdbEngine instanceof RdbEngineDb2) { - throw new UnsupportedOperationException( - CoreError.DB2_RENAME_PRIMARY_OR_INDEX_KEY_COLUMN_NOT_SUPPORTED.buildMessage()); - } tableMetadataBuilder.renameSecondaryIndex(oldColumnName, newColumnName); } TableMetadata updatedTableMetadata = tableMetadataBuilder.build(); 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 index 63cf5aba26..882400cab3 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineDb2.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineDb2.java @@ -527,6 +527,16 @@ public String getProjectionsSqlForSelectQuery( .collect(Collectors.joining(",")); } + @Override + public void throwIfRenameColumnNotSupported(String columnName, TableMetadata tableMetadata) { + if (tableMetadata.getPartitionKeyNames().contains(columnName) + || tableMetadata.getClusteringKeyNames().contains(columnName) + || tableMetadata.getSecondaryIndexNames().contains(columnName)) { + throw new UnsupportedOperationException( + CoreError.JDBC_DB2_RENAME_PRIMARY_OR_INDEX_KEY_COLUMN_NOT_SUPPORTED.buildMessage()); + } + } + private String getProjection(String columnName, DataType dataType) { if (dataType == DataType.DATE) { // Selecting a DATE column requires special handling. We need to cast the DATE column values 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 index ea9bc1c812..235f6cb255 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java @@ -272,6 +272,15 @@ default void throwIfDuplicatedIndexWarning(SQLWarning warning) throws SQLExcepti // Do nothing } + /** + * Throws an exception if renaming columns is not supported in the underlying database. + * + * @param columnName the current name of the column to rename + * @param tableMetadata the current table metadata + * @throws UnsupportedOperationException if renaming the column is not supported + */ + default void throwIfRenameColumnNotSupported(String columnName, TableMetadata tableMetadata) {} + default void setConnectionToReadOnly(Connection connection, boolean readOnly) throws SQLException { connection.setReadOnly(readOnly); From ed4501bffa6042818e7eaef53708176e63f82942 Mon Sep 17 00:00:00 2001 From: Kodai Doki <52027276+KodaiD@users.noreply.github.com> Date: Thu, 11 Sep 2025 17:20:38 +0900 Subject: [PATCH 15/15] Apply suggestions from code review Co-authored-by: Toshihiro Suzuki --- core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java | 2 +- .../main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 index 3dd4b9bd4c..dda73607e7 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java @@ -890,8 +890,8 @@ public void renameColumn( throws ExecutionException { try { TableMetadata currentTableMetadata = getTableMetadata(namespace, table); - rdbEngine.throwIfRenameColumnNotSupported(oldColumnName, currentTableMetadata); assert currentTableMetadata != null; + rdbEngine.throwIfRenameColumnNotSupported(oldColumnName, currentTableMetadata); TableMetadata.Builder tableMetadataBuilder = TableMetadata.newBuilder(currentTableMetadata).renameColumn(oldColumnName, newColumnName); if (currentTableMetadata.getPartitionKeyNames().contains(oldColumnName)) { 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 index 235f6cb255..b0826ceeb0 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java @@ -273,7 +273,7 @@ default void throwIfDuplicatedIndexWarning(SQLWarning warning) throws SQLExcepti } /** - * Throws an exception if renaming columns is not supported in the underlying database. + * Throws an exception if renaming the column is not supported in the underlying database. * * @param columnName the current name of the column to rename * @param tableMetadata the current table metadata