From 18a48c86ad07e0bde4c39806dd1612aaea4903f5 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Thu, 7 Aug 2025 21:38:48 +0900 Subject: [PATCH 01/13] Add drop column --- .../main/java/com/scalar/db/api/Admin.java | 12 +++ .../CheckedDistributedStorageAdmin.java | 33 +++++++ .../java/com/scalar/db/common/CoreError.java | 12 +++ .../DecoratedDistributedTransactionAdmin.java | 6 ++ .../com/scalar/db/service/AdminService.java | 6 ++ .../db/storage/cassandra/CassandraAdmin.java | 21 ++++ .../scalar/db/storage/cosmos/CosmosAdmin.java | 17 ++++ .../scalar/db/storage/dynamo/DynamoAdmin.java | 19 ++++ .../com/scalar/db/storage/jdbc/JdbcAdmin.java | 25 +++++ .../multistorage/MultiStorageAdmin.java | 6 ++ .../consensuscommit/ConsensusCommitAdmin.java | 16 +++ .../jdbc/JdbcTransactionAdmin.java | 6 ++ .../SingleCrudOperationTransactionAdmin.java | 6 ++ ...ibutedStorageAdminIntegrationTestBase.java | 97 +++++++++++++++++++ ...edTransactionAdminIntegrationTestBase.java | 96 ++++++++++++++++++ 15 files changed, 378 insertions(+) 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..8785e1ac62 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,18 @@ default void addNewColumnToTable( } } + /** + * Drops a column from an existing table. The column cannot be a partition or clustering key. + * + * @param namespace the table namespace + * @param table the table name + * @param columnName the name of the column to drop + * @throws IllegalArgumentException if the table does not exist or the column does not exist + * @throws ExecutionException if the operation fails + */ + void dropColumnFromTable(String namespace, String table, String columnName) + 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..eaa8fffc0d 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,39 @@ public void addNewColumnToTable( } } + @Override + public void dropColumnFromTable(String namespace, String table, String columnName) + 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(columnName)) { + throw new IllegalArgumentException( + CoreError.COLUMN_NOT_FOUND2.buildMessage( + ScalarDbUtils.getFullTableName(namespace, table), columnName)); + } + + if (tableMetadata.getPartitionKeyNames().contains(columnName) + || tableMetadata.getClusteringKeyNames().contains(columnName) + || tableMetadata.getSecondaryIndexNames().contains(columnName)) { + throw new IllegalArgumentException( + CoreError.COLUMN_SPECIFIED_AS_PRIMARY_KEY_OR_INDEX_KEY.buildMessage( + ScalarDbUtils.getFullTableName(namespace, table), columnName)); + } + + try { + admin.dropColumnFromTable(namespace, table, columnName); + } catch (ExecutionException e) { + throw new ExecutionException( + CoreError.DROPPING_COLUMN_FROM_TABLE_FAILED.buildMessage( + ScalarDbUtils.getFullTableName(namespace, table), columnName), + 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..4a43088123 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,12 @@ 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", + "", + ""), // // Errors for the concurrency error category @@ -960,6 +966,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", "", ""), + DROPPING_COLUMN_FROM_TABLE_FAILED( + Category.INTERNAL_ERROR, + "0059", + "Dropping a column from the table failed. Table: %s; Column: %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..3c3496a8e1 100644 --- a/core/src/main/java/com/scalar/db/common/DecoratedDistributedTransactionAdmin.java +++ b/core/src/main/java/com/scalar/db/common/DecoratedDistributedTransactionAdmin.java @@ -195,6 +195,12 @@ public void addNewColumnToTable( namespace, table, columnName, columnType, encrypted); } + @Override + public void dropColumnFromTable(String namespace, String table, String columnName) + throws ExecutionException { + distributedTransactionAdmin.dropColumnFromTable(namespace, table, columnName); + } + @Override public void importTable(String namespace, String table, Map options) throws ExecutionException { 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..f9edfbfa94 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,12 @@ public void addNewColumnToTable( admin.addNewColumnToTable(namespace, table, columnName, columnType); } + @Override + public void dropColumnFromTable(String namespace, String table, String columnName) + throws ExecutionException { + admin.dropColumnFromTable(namespace, table, columnName); + } + @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..39b1cc3951 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,27 @@ columnName, getFullTableName(namespace, table)), } } + @Override + public void dropColumnFromTable(String namespace, String table, String columnName) + throws ExecutionException { + try { + String alterTableQuery = + SchemaBuilder.alterTable(quoteIfNecessary(namespace), quoteIfNecessary(table)) + .dropColumn(columnName) + .getQueryString(); + + clusterManager.getSession().execute(alterTableQuery); + } catch (IllegalArgumentException e) { + throw e; + } catch (RuntimeException e) { + throw new ExecutionException( + String.format( + "Dropping the %s column from the %s table failed", + columnName, 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..d9cfbcebc4 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,23 @@ columnName, getFullTableName(namespace, table)), } } + @Override + public void dropColumnFromTable(String namespace, String table, String columnName) + throws ExecutionException { + try { + TableMetadata currentTableMetadata = getTableMetadata(namespace, table); + TableMetadata updatedTableMetadata = + TableMetadata.newBuilder(currentTableMetadata).removeColumn(columnName).build(); + upsertTableMetadata(namespace, table, updatedTableMetadata); + } catch (ExecutionException e) { + throw new ExecutionException( + String.format( + "Adding the new %s column to the %s container failed", + columnName, getFullTableName(namespace, table)), + e); + } + } + @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..c69b71f15b 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,25 @@ columnName, getFullTableName(namespace, table)), } } + @Override + public void dropColumnFromTable(String nonPrefixedNamespace, String table, String columnName) + throws ExecutionException { + Namespace namespace = Namespace.of(namespacePrefix, nonPrefixedNamespace); + try { + TableMetadata currentTableMetadata = getTableMetadata(nonPrefixedNamespace, table); + TableMetadata updatedTableMetadata = + TableMetadata.newBuilder(currentTableMetadata).removeColumn(columnName).build(); + + upsertTableMetadata(namespace, table, updatedTableMetadata); + } catch (ExecutionException e) { + throw new ExecutionException( + String.format( + "Dropping the %s column from the %s table failed", + columnName, getFullTableName(namespace, table)), + e); + } + } + @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..f425209d1e 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,31 @@ columnName, getFullTableName(namespace, table)), } } + @Override + public void dropColumnFromTable(String namespace, String table, String columnName) + throws ExecutionException { + try { + TableMetadata currentTableMetadata = getTableMetadata(namespace, table); + TableMetadata updatedTableMetadata = + TableMetadata.newBuilder(currentTableMetadata).removeColumn(columnName).build(); + String addNewColumnStatement = + "ALTER TABLE " + + encloseFullTableName(namespace, table) + + " DROP COLUMN" + + enclose(columnName); + try (Connection connection = dataSource.getConnection()) { + execute(connection, addNewColumnStatement); + addTableMetadata(connection, namespace, table, updatedTableMetadata, false, true); + } + } catch (SQLException e) { + throw new ExecutionException( + String.format( + "Adding the new %s column to the %s table failed", + columnName, 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/multistorage/MultiStorageAdmin.java b/core/src/main/java/com/scalar/db/storage/multistorage/MultiStorageAdmin.java index 5c7c4f9a79..b4d4c5de65 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,12 @@ public void addNewColumnToTable( getAdmin(namespace, table).addNewColumnToTable(namespace, table, columnName, columnType); } + @Override + public void dropColumnFromTable(String namespace, String table, String columnName) + throws ExecutionException { + getAdmin(namespace, table).dropColumnFromTable(namespace, table, columnName); + } + @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..da479d6ce9 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,22 @@ public void addNewColumnToTable( admin.addNewColumnToTable(namespace, table, beforeColumnName, columnType); } + @Override + public void dropColumnFromTable(String namespace, String table, String columnName) + 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 beforeColumnName = getBeforeImageColumnName(columnName, tableMetadata); + + admin.dropColumnFromTable(namespace, table, columnName); + admin.dropColumnFromTable(namespace, table, beforeColumnName); + } + @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..2b99bc1094 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,12 @@ public void addNewColumnToTable( jdbcAdmin.addNewColumnToTable(namespace, table, columnName, columnType); } + @Override + public void dropColumnFromTable(String namespace, String table, String columnName) + throws ExecutionException { + jdbcAdmin.dropColumnFromTable(namespace, table, columnName); + } + @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..161870d154 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,12 @@ public void addNewColumnToTable( distributedStorageAdmin.addNewColumnToTable(namespace, table, columnName, columnType); } + @Override + public void dropColumnFromTable(String namespace, String table, String columnName) + throws ExecutionException { + distributedStorageAdmin.dropColumnFromTable(namespace, table, columnName); + } + @Override public Set getNamespaceNames() throws ExecutionException { return distributedStorageAdmin.getNamespaceNames(); 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 7c123778ba..5f3552ba7f 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 @@ -969,6 +969,71 @@ public void addNewColumnToTable_AddColumnForEachExistingDataType_ShouldAddNewCol } } + @Test + public void dropColumnFromTable_DropColumnForEachExistingDataType_ShouldDropColumnsCorrectly() + throws ExecutionException { + try { + // Arrange + Map options = getCreationOptions(); + TableMetadata.Builder currentTableMetadataBuilder = + TableMetadata.newBuilder() + .addColumn(getColumnName1(), DataType.INT) + .addColumn(getColumnName2(), DataType.INT) + .addColumn(getColumnName3(), DataType.INT) + .addColumn(getColumnName4(), DataType.BIGINT) + .addColumn(getColumnName5(), DataType.FLOAT) + .addColumn(getColumnName6(), DataType.DOUBLE) + .addColumn(getColumnName7(), DataType.TEXT) + .addColumn(getColumnName8(), DataType.BLOB) + .addColumn(getColumnName9(), DataType.DATE) + .addColumn(getColumnName10(), DataType.TIME) + .addPartitionKey(getColumnName1()) + .addClusteringKey(getColumnName2(), Scan.Ordering.Order.ASC); + if (isTimestampTypeSupported()) { + currentTableMetadataBuilder + .addColumn(getColumnName11(), DataType.TIMESTAMP) + .addColumn(getColumnName12(), DataType.TIMESTAMPTZ); + } + TableMetadata currentTableMetadata = currentTableMetadataBuilder.build(); + admin.createTable(namespace1, getTable4(), currentTableMetadata, options); + + // Act + admin.dropColumnFromTable(namespace1, getTable4(), getColumnName3()); + admin.dropColumnFromTable(namespace1, getTable4(), getColumnName4()); + admin.dropColumnFromTable(namespace1, getTable4(), getColumnName5()); + admin.dropColumnFromTable(namespace1, getTable4(), getColumnName6()); + admin.dropColumnFromTable(namespace1, getTable4(), getColumnName7()); + admin.dropColumnFromTable(namespace1, getTable4(), getColumnName8()); + admin.dropColumnFromTable(namespace1, getTable4(), getColumnName9()); + admin.dropColumnFromTable(namespace1, getTable4(), getColumnName10()); + if (isTimestampTypeSupported()) { + admin.dropColumnFromTable(namespace1, getTable4(), getColumnName11()); + admin.dropColumnFromTable(namespace1, getTable4(), getColumnName12()); + } + + // Assert + TableMetadata.Builder expectedTableMetadataBuilder = + TableMetadata.newBuilder(currentTableMetadata) + .removeColumn(getColumnName3()) + .removeColumn(getColumnName4()) + .removeColumn(getColumnName5()) + .removeColumn(getColumnName6()) + .removeColumn(getColumnName7()) + .removeColumn(getColumnName8()) + .removeColumn(getColumnName9()) + .removeColumn(getColumnName10()); + if (isTimestampTypeSupported()) { + expectedTableMetadataBuilder + .removeColumn(getColumnName11()) + .removeColumn(getColumnName12()); + } + TableMetadata expectedTableMetadata = expectedTableMetadataBuilder.build(); + assertThat(admin.getTableMetadata(namespace1, getTable4())).isEqualTo(expectedTableMetadata); + } finally { + admin.dropTable(namespace1, getTable4(), true); + } + } + @Test public void getNamespaceNames_ShouldReturnCreatedNamespaces() throws ExecutionException { // Arrange @@ -1002,6 +1067,38 @@ public void addNewColumnToTable_ForAlreadyExistingColumn_ShouldThrowIllegalArgum .isInstanceOf(IllegalArgumentException.class); } + @Test + public void dropColumnFromTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() { + // Arrange + + // Act Assert + assertThatThrownBy(() -> admin.dropColumnFromTable(namespace1, getTable4(), getColumnName2())) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentException() { + // Arrange + + // Act Assert + assertThatThrownBy( + () -> admin.dropColumnFromTable(namespace1, getTable1(), "nonExistingColumn")) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() { + // Arrange + + // Act Assert + assertThatThrownBy(() -> admin.dropColumnFromTable(namespace1, getTable1(), getColumnName1())) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> admin.dropColumnFromTable(namespace1, getTable1(), getColumnName3())) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> admin.dropColumnFromTable(namespace1, getTable1(), getColumnName5())) + .isInstanceOf(IllegalArgumentException.class); + } + @Test public void upgrade_WhenMetadataTableExistsButNotNamespacesTable_ShouldCreateNamespacesTableAndImportExistingNamespaces() 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 73cc1b363f..f1dfb3fb49 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 @@ -858,6 +858,71 @@ public void addNewColumnToTable_AddColumnForEachExistingDataType_ShouldAddNewCol } } + @Test + public void dropColumnFromTable_DropColumnForEachExistingDataType_ShouldDropColumnsCorrectly() + throws ExecutionException { + try { + // Arrange + Map options = getCreationOptions(); + TableMetadata.Builder currentTableMetadataBuilder = + TableMetadata.newBuilder() + .addColumn("c1", DataType.INT) + .addColumn("c2", DataType.INT) + .addColumn("c3", DataType.INT) + .addColumn("c4", DataType.BIGINT) + .addColumn("c5", DataType.FLOAT) + .addColumn("c6", DataType.DOUBLE) + .addColumn("c7", DataType.TEXT) + .addColumn("c8", DataType.BLOB) + .addColumn("c9", DataType.DATE) + .addColumn("c10", DataType.TIME) + .addPartitionKey("c1") + .addClusteringKey("c2", Scan.Ordering.Order.ASC); + if (isTimestampTypeSupported()) { + currentTableMetadataBuilder + .addColumn("c11", DataType.TIMESTAMP) + .addColumn("c12", DataType.TIMESTAMPTZ); + } + TableMetadata currentTableMetadata = currentTableMetadataBuilder.build(); + admin.createTable(namespace1, TABLE4, currentTableMetadata, options); + + // Act + admin.dropColumnFromTable(namespace1, TABLE4, "c3"); + admin.dropColumnFromTable(namespace1, TABLE4, "c4"); + admin.dropColumnFromTable(namespace1, TABLE4, "c5"); + admin.dropColumnFromTable(namespace1, TABLE4, "c6"); + admin.dropColumnFromTable(namespace1, TABLE4, "c7"); + admin.dropColumnFromTable(namespace1, TABLE4, "c8"); + admin.dropColumnFromTable(namespace1, TABLE4, "c9"); + admin.dropColumnFromTable(namespace1, TABLE4, "c10"); + if (isTimestampTypeSupported()) { + admin.dropColumnFromTable(namespace1, TABLE4, "c11"); + admin.dropColumnFromTable(namespace1, TABLE4, "c12"); + } + + // Assert + TableMetadata.Builder expectedTableMetadataBuilder = + TableMetadata.newBuilder(currentTableMetadata) + .removeColumn("c3") + .removeColumn("c4") + .removeColumn("c5") + .removeColumn("c6") + .removeColumn("c7") + .removeColumn("c8") + .removeColumn("c9") + .removeColumn("c10") + .removeColumn("c11") + .removeColumn("c12"); + if (isTimestampTypeSupported()) { + expectedTableMetadataBuilder.removeColumn("c11").removeColumn("c12"); + } + TableMetadata expectedTableMetadata = expectedTableMetadataBuilder.build(); + assertThat(admin.getTableMetadata(namespace1, TABLE4)).isEqualTo(expectedTableMetadata); + } finally { + admin.dropTable(namespace1, TABLE4, true); + } + } + @Test public void addNewColumnToTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() { // Arrange @@ -878,6 +943,37 @@ public void addNewColumnToTable_ForAlreadyExistingColumn_ShouldThrowIllegalArgum .isInstanceOf(IllegalArgumentException.class); } + @Test + public void dropColumnFromTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() { + // Arrange + + // Act Assert + assertThatThrownBy(() -> admin.dropColumnFromTable(namespace1, TABLE4, COL_NAME2)) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentException() { + // Arrange + + // Act Assert + assertThatThrownBy(() -> admin.dropColumnFromTable(namespace1, TABLE1, "nonExistingColumn")) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() { + // Arrange + + // Act Assert + assertThatThrownBy(() -> admin.dropColumnFromTable(namespace1, TABLE1, "c1")) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> admin.dropColumnFromTable(namespace1, TABLE1, "c3")) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> admin.dropColumnFromTable(namespace1, TABLE1, "c5")) + .isInstanceOf(IllegalArgumentException.class); + } + @Test public void createCoordinatorTables_ShouldCreateCoordinatorTablesCorrectly() throws ExecutionException { From 31c3d15a77d4ab6381e857421dcacef52535adcb Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Thu, 7 Aug 2025 22:52:23 +0900 Subject: [PATCH 02/13] Add unit tests --- .../com/scalar/db/storage/jdbc/JdbcAdmin.java | 2 +- .../storage/cassandra/CassandraAdminTest.java | 27 ++++ .../db/storage/cosmos/CosmosAdminTest.java | 43 ++++++ .../storage/dynamo/DynamoAdminTestBase.java | 67 +++++++++ .../scalar/db/storage/jdbc/JdbcAdminTest.java | 139 ++++++++++++++++++ .../multistorage/MultiStorageAdminTest.java | 62 ++++++++ .../ConsensusCommitAdminTestBase.java | 23 +++ .../jdbc/JdbcTransactionAdminTest.java | 14 ++ ...ngleCrudOperationTransactionAdminTest.java | 15 ++ 9 files changed, 391 insertions(+), 1 deletion(-) 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 f425209d1e..37d63cddcd 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 @@ -871,7 +871,7 @@ public void dropColumnFromTable(String namespace, String table, String columnNam String addNewColumnStatement = "ALTER TABLE " + encloseFullTableName(namespace, table) - + " DROP COLUMN" + + " DROP COLUMN " + enclose(columnName); try (Connection connection = dataSource.getConnection()) { execute(connection, addNewColumnStatement); 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..b7446f7976 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,33 @@ public void addNewColumnToTable_ShouldWorkProperly() throws ExecutionException { verify(cassandraSession).execute(alterTableQuery); } + @Test + public void dropColumnFromTable_ShouldWorkProperly() throws ExecutionException { + // Arrange + String namespace = "sample_ns"; + String table = "tbl"; + String column = "c2"; + com.datastax.driver.core.TableMetadata tableMetadata = + mock(com.datastax.driver.core.TableMetadata.class); + ColumnMetadata c1 = mock(ColumnMetadata.class); + when(c1.getName()).thenReturn("c1"); + 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); + when(clusterManager.getSession()).thenReturn(cassandraSession); + + // Act + cassandraAdmin.dropColumnFromTable(namespace, table, column); + + // Assert + String alterTableQuery = + SchemaBuilder.alterTable(namespace, table).dropColumn(column).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..77e952d274 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 @@ -1131,6 +1131,49 @@ public void addNewColumnToTable_ShouldWorkProperly() throws ExecutionException { verify(container).upsertItem(expectedCosmosTableMetadata); } + @Test + public void dropColumnFromTable_ShouldWorkProperly() throws ExecutionException { + // Arrange + String namespace = "ns"; + String table = "table"; + String column1 = "c1"; + String column2 = "c2"; + String fullTableName = getFullTableName(namespace, table); + @SuppressWarnings("unchecked") + CosmosItemResponse response = mock(CosmosItemResponse.class); + + when(client.getDatabase(METADATA_DATABASE)).thenReturn(database); + when(database.getContainer(CosmosAdmin.TABLE_METADATA_CONTAINER)).thenReturn(container); + when(container.readItem( + anyString(), + any(PartitionKey.class), + ArgumentMatchers.>any())) + .thenReturn(response); + + CosmosTableMetadata cosmosTableMetadata = + CosmosTableMetadata.newBuilder() + .partitionKeyNames(Sets.newLinkedHashSet(column1)) + .columns(ImmutableMap.of(column1, "text", column2, "int")) + .build(); + + when(response.getItem()).thenReturn(cosmosTableMetadata); + + // Act + admin.dropColumnFromTable(namespace, table, column2); + + // Assert + verify(container) + .readItem(fullTableName, new PartitionKey(fullTableName), CosmosTableMetadata.class); + + CosmosTableMetadata expectedCosmosTableMetadata = + CosmosTableMetadata.newBuilder() + .id(fullTableName) + .partitionKeyNames(Sets.newLinkedHashSet(column1)) + .columns(ImmutableMap.of(column1, "text")) + .build(); + verify(container).upsertItem(expectedCosmosTableMetadata); + } + @Test public void unsupportedOperations_ShouldThrowUnsupportedException() { // Arrange 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..a25367df4e 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 @@ -1441,6 +1441,73 @@ public void addNewColumnToTable_ShouldWorkProperly() throws ExecutionException { .build()); } + @Test + public void dropColumnFromTable_ShouldWorkProperly() throws ExecutionException { + // Arrange + String column1 = "c1"; + String column2 = "c2"; + + GetItemResponse response = mock(GetItemResponse.class); + when(client.getItem(any(GetItemRequest.class))).thenReturn(response); + when(response.item()) + .thenReturn( + ImmutableMap.builder() + .put( + DynamoAdmin.METADATA_ATTR_TABLE, + AttributeValue.builder().s(getFullTableName()).build()) + .put( + DynamoAdmin.METADATA_ATTR_COLUMNS, + AttributeValue.builder() + .m( + ImmutableMap.builder() + .put(column1, AttributeValue.builder().s("text").build()) + .put(column2, AttributeValue.builder().s("int").build()) + .build()) + .build()) + .put( + DynamoAdmin.METADATA_ATTR_PARTITION_KEY, + AttributeValue.builder().l(AttributeValue.builder().s(column1).build()).build()) + .build()); + + // Act + admin.dropColumnFromTable(NAMESPACE, TABLE, column2); + + // Assert + // Get metadata + Map key = new HashMap<>(); + key.put( + DynamoAdmin.METADATA_ATTR_TABLE, AttributeValue.builder().s(getFullTableName()).build()); + verify(client) + .getItem( + GetItemRequest.builder() + .tableName(getFullMetadataTableName()) + .key(key) + .consistentRead(true) + .build()); + + // Put metadata + Map itemValues = new HashMap<>(); + itemValues.put( + DynamoAdmin.METADATA_ATTR_TABLE, AttributeValue.builder().s(getFullTableName()).build()); + Map columns = new HashMap<>(); + + columns.put( + column1, AttributeValue.builder().s(DataType.TEXT.toString().toLowerCase()).build()); + + itemValues.put(DynamoAdmin.METADATA_ATTR_COLUMNS, AttributeValue.builder().m(columns).build()); + itemValues.put( + DynamoAdmin.METADATA_ATTR_PARTITION_KEY, + AttributeValue.builder() + .l(Collections.singletonList(AttributeValue.builder().s(column1).build())) + .build()); + verify(client) + .putItem( + PutItemRequest.builder() + .tableName(getFullMetadataTableName()) + .item(itemValues) + .build()); + } + @Test public void createNamespace_WithNonExistingNamespacesTable_ShouldCreateNamespacesTableAndAddNamespace() 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..85dc10d425 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,145 @@ private void addNewColumnToTable_ForX_ShouldWorkProperly( } } + @Test + public void dropColumnFromTable_ForMysql_ShouldWorkProperly() + throws SQLException, ExecutionException { + dropColumnFromTable_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` DROP COLUMN `c2`", + "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)"); + } + + @Test + public void dropColumnFromTable_ForOracle_ShouldWorkProperly() + throws SQLException, ExecutionException { + dropColumnFromTable_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\" DROP COLUMN \"c2\"", + "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)"); + } + + @Test + public void dropColumnFromTable_ForPostgresql_ShouldWorkProperly() + throws SQLException, ExecutionException { + dropColumnFromTable_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\" DROP COLUMN \"c2\"", + "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)"); + } + + @Test + public void dropColumnFromTable_ForSqlServer_ShouldWorkProperly() + throws SQLException, ExecutionException { + dropColumnFromTable_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", + "ALTER TABLE [ns].[table] DROP COLUMN [c2]", + "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)"); + } + + @Test + public void dropColumnFromTable_ForSqlite_ShouldWorkProperly() + throws SQLException, ExecutionException { + dropColumnFromTable_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\" DROP COLUMN \"c2\"", + "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)"); + } + + @Test + public void dropColumnFromTable_ForDb2_ShouldWorkProperly() + throws SQLException, ExecutionException { + dropColumnFromTable_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\" DROP COLUMN \"c2\"", + "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)"); + } + + private void dropColumnFromTable_ForX_ShouldWorkProperly( + RdbEngine rdbEngine, String expectedGetMetadataStatement, String... expectedSqlStatements) + throws SQLException, ExecutionException { + // Arrange + String namespace = "ns"; + String table = "table"; + String column1 = "c1"; + String column2 = "c2"; + + PreparedStatement selectStatement = mock(PreparedStatement.class); + ResultSet resultSet = + mockResultSet( + new SelectAllFromMetadataTableResultSetMocker.Row( + column1, DataType.TEXT.toString(), "PARTITION", null, false), + new SelectAllFromMetadataTableResultSetMocker.Row( + column2, 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.dropColumnFromTable(namespace, table, column2); + + // 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..3e0cae38a5 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,68 @@ public void addNewColumnToTable_ForTable1InNamespace2_ShouldCallAddNewColumnOfAd verify(admin2).addNewColumnToTable(namespace, table, column, dataType); } + @Test + public void dropColumnFromTable_ForTable1InNamespace1_ShouldCallDropColumnFromTableOfAdmin1() + throws ExecutionException { + // Arrange + String namespace = NAMESPACE1; + String table = TABLE1; + String column = "c1"; + + // Act + multiStorageAdmin.dropColumnFromTable(namespace, table, column); + + // Assert + verify(admin1).dropColumnFromTable(namespace, table, column); + } + + @Test + public void + dropColumnFromTable_ForTable2InNamespace1_ShouldShouldCallDropColumnFromTableOfAdmin2() + throws ExecutionException { + // Arrange + String namespace = NAMESPACE1; + String table = TABLE2; + String column = "c1"; + + // Act + multiStorageAdmin.dropColumnFromTable(namespace, table, column); + + // Assert + verify(admin2).dropColumnFromTable(namespace, table, column); + } + + @Test + public void + dropColumnFromTable_ForTable3InNamespace1_ShouldCallDropColumnFromTableOfDefaultAdmin() + throws ExecutionException { + // Arrange + String namespace = NAMESPACE1; + String table = TABLE3; + String column = "c1"; + + // Act + multiStorageAdmin.dropColumnFromTable(namespace, table, column); + + // Assert + verify(admin3).dropColumnFromTable(namespace, table, column); + } + + @Test + public void dropColumnFromTable_ForTable1InNamespace2_ShouldCallDropColumnFromTableOfAdmin2() + throws ExecutionException { + // Arrange + String namespace = NAMESPACE2; + String table = TABLE1; + String column = "c1"; + + // Act + multiStorageAdmin.dropColumnFromTable(namespace, table, column); + + // Assert + verify(admin2).dropColumnFromTable(namespace, table, column); + } + @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..789124919f 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,29 @@ public void addNewColumnToTable_WithEncrypted_ShouldThrowUnsupportedOperationExc .isInstanceOf(UnsupportedOperationException.class); } + @Test + public void dropColumnFromTable_ShouldCallJdbcAdminProperly() throws ExecutionException { + // Arrange + String targetColumn = "col2"; + TableMetadata tableMetadata = + TableMetadata.newBuilder() + .addColumn("col1", DataType.INT) + .addColumn(targetColumn, DataType.INT) + .addPartitionKey("col1") + .build(); + when(distributedStorageAdmin.getTableMetadata(any(), any())) + .thenReturn(ConsensusCommitUtils.buildTransactionTableMetadata(tableMetadata)); + + // Act + admin.dropColumnFromTable(NAMESPACE, TABLE, targetColumn); + + // Assert + verify(distributedStorageAdmin).getTableMetadata(NAMESPACE, TABLE); + verify(distributedStorageAdmin).dropColumnFromTable(NAMESPACE, TABLE, targetColumn); + verify(distributedStorageAdmin) + .dropColumnFromTable(NAMESPACE, TABLE, Attribute.BEFORE_PREFIX + targetColumn); + } + @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..b88fdebe67 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,20 @@ public void addNewColumnToTable_ShouldCallJdbcAdminProperly() throws ExecutionEx verify(jdbcAdmin).addNewColumnToTable(namespace, table, column, dataType); } + @Test + public void dropColumnFromTable_ShouldCallJdbcAdminProperly() throws ExecutionException { + // Arrange + String namespace = "ns"; + String table = "tbl"; + String column = "c1"; + + // Act + admin.dropColumnFromTable(namespace, table, column); + + // Assert + verify(jdbcAdmin).dropColumnFromTable(namespace, table, column); + } + @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..f463ad7063 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 dropColumnFromTable_ShouldCallDistributedStorageAdminProperly() + throws ExecutionException { + // Arrange + String namespace = "ns"; + String table = "tbl"; + String column = "c1"; + + // Act + admin.dropColumnFromTable(namespace, table, column); + + // Assert + verify(distributedStorageAdmin).dropColumnFromTable(namespace, table, column); + } + @Test public void importTable_ShouldCallDistributedStorageAdminProperly() throws ExecutionException { // Arrange From 6a6b4b9a9bcc60c216c70ebccbfcd461eeccb1bc Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Fri, 8 Aug 2025 10:49:47 +0900 Subject: [PATCH 03/13] Add permission tests --- ...utedStorageAdminPermissionIntegrationTestBase.java | 11 +++++++++++ 1 file changed, 11 insertions(+) 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..035e31266d 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,17 @@ public void addNewColumnToTable_WithSufficientPermission_ShouldSucceed() .doesNotThrowAnyException(); } + @Test + public void dropColumnFromTable_WithSufficientPermission_ShouldSucceed() + throws ExecutionException { + // Arrange + createNamespaceByRoot(); + createTableByRoot(); + // Act Assert + assertThatCode(() -> adminForNormalUser.dropColumnFromTable(NAMESPACE, TABLE, COL_NAME3)) + .doesNotThrowAnyException(); + } + @Test public void importTable_WithSufficientPermission_ShouldSucceed() throws Exception { // Arrange From d2b4a97ee12d3b8b2d1bd4ff4cf85ae6868d9edc Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Tue, 19 Aug 2025 18:28:12 +0900 Subject: [PATCH 04/13] Add drop column if exists --- .../main/java/com/scalar/db/api/Admin.java | 26 ++++++++++ .../DecoratedDistributedTransactionAdmin.java | 7 +++ ...ibutedStorageAdminIntegrationTestBase.java | 10 ++++ ...edTransactionAdminIntegrationTestBase.java | 49 +++++++++++-------- 4 files changed, 72 insertions(+), 20 deletions(-) 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 8785e1ac62..83d66ac8a6 100644 --- a/core/src/main/java/com/scalar/db/api/Admin.java +++ b/core/src/main/java/com/scalar/db/api/Admin.java @@ -3,6 +3,7 @@ import com.scalar.db.common.CoreError; import com.scalar.db.exception.storage.ExecutionException; import com.scalar.db.io.DataType; +import com.scalar.db.util.ScalarDbUtils; import java.util.Collections; import java.util.Map; import java.util.Set; @@ -453,6 +454,31 @@ default void addNewColumnToTable( void dropColumnFromTable(String namespace, String table, String columnName) throws ExecutionException; + /** + * Drops a column from an existing table. The column cannot be a partition or clustering key. + * + * @param namespace the table namespace + * @param table the table name + * @param columnName the name of the column to drop + * @param IfExists if set to true, the column will be dropped only if it exists. If set to false, + * it will throw an exception if it does not exist + * @throws IllegalArgumentException if the table does not exist + * @throws ExecutionException if the operation fails + */ + default void dropColumnFromTable( + String namespace, String table, String columnName, boolean IfExists) + throws ExecutionException { + TableMetadata tableMetadata = getTableMetadata(namespace, table); + if (tableMetadata == null) { + throw new IllegalArgumentException( + CoreError.TABLE_NOT_FOUND.buildMessage(ScalarDbUtils.getFullTableName(namespace, table))); + } + if (IfExists && !tableMetadata.getColumnNames().contains(columnName)) { + return; + } + dropColumnFromTable(namespace, table, columnName); + } + /** * Imports an existing table that is not managed by ScalarDB. * 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 3c3496a8e1..b4842085ac 100644 --- a/core/src/main/java/com/scalar/db/common/DecoratedDistributedTransactionAdmin.java +++ b/core/src/main/java/com/scalar/db/common/DecoratedDistributedTransactionAdmin.java @@ -201,6 +201,13 @@ public void dropColumnFromTable(String namespace, String table, String columnNam distributedTransactionAdmin.dropColumnFromTable(namespace, table, columnName); } + @Override + public void dropColumnFromTable( + String namespace, String table, String columnName, boolean ifExists) + throws ExecutionException { + distributedTransactionAdmin.dropColumnFromTable(namespace, table, columnName, ifExists); + } + @Override public void importTable(String namespace, String table, Map options) throws ExecutionException { 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 5f3552ba7f..72301f3e3b 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 @@ -1099,6 +1099,16 @@ public void dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArg .isInstanceOf(IllegalArgumentException.class); } + @Test + public void dropColumnFromTable_IfExists_ForNonExistingColumn_ShouldNotThrowAnyException() { + // Arrange + + // Act Assert + assertThatCode( + () -> admin.dropColumnFromTable(namespace1, getTable1(), "nonExistingColumn", true)) + .doesNotThrowAnyException(); + } + @Test public void upgrade_WhenMetadataTableExistsButNotNamespacesTable_ShouldCreateNamespacesTableAndImportExistingNamespaces() 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 f1dfb3fb49..9d1e8ad07a 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 @@ -858,6 +858,26 @@ public void addNewColumnToTable_AddColumnForEachExistingDataType_ShouldAddNewCol } } + @Test + public void addNewColumnToTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() { + // Arrange + + // Act Assert + assertThatThrownBy( + () -> admin.addNewColumnToTable(namespace1, TABLE4, COL_NAME2, DataType.TEXT)) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void addNewColumnToTable_ForAlreadyExistingColumn_ShouldThrowIllegalArgumentException() { + // Arrange + + // Act Assert + assertThatThrownBy( + () -> admin.addNewColumnToTable(namespace1, TABLE1, COL_NAME2, DataType.TEXT)) + .isInstanceOf(IllegalArgumentException.class); + } + @Test public void dropColumnFromTable_DropColumnForEachExistingDataType_ShouldDropColumnsCorrectly() throws ExecutionException { @@ -923,26 +943,6 @@ public void dropColumnFromTable_DropColumnForEachExistingDataType_ShouldDropColu } } - @Test - public void addNewColumnToTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() { - // Arrange - - // Act Assert - assertThatThrownBy( - () -> admin.addNewColumnToTable(namespace1, TABLE4, COL_NAME2, DataType.TEXT)) - .isInstanceOf(IllegalArgumentException.class); - } - - @Test - public void addNewColumnToTable_ForAlreadyExistingColumn_ShouldThrowIllegalArgumentException() { - // Arrange - - // Act Assert - assertThatThrownBy( - () -> admin.addNewColumnToTable(namespace1, TABLE1, COL_NAME2, DataType.TEXT)) - .isInstanceOf(IllegalArgumentException.class); - } - @Test public void dropColumnFromTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() { // Arrange @@ -974,6 +974,15 @@ public void dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArg .isInstanceOf(IllegalArgumentException.class); } + @Test + public void dropColumnFromTable_IfNotExists_ForNonExistingColumn_ShouldNotThrowAnyException() { + // Arrange + + // Act Assert + assertThatCode(() -> admin.dropColumnFromTable(namespace1, TABLE1, "nonExistingColumn", true)) + .doesNotThrowAnyException(); + } + @Test public void createCoordinatorTables_ShouldCreateCoordinatorTablesCorrectly() throws ExecutionException { From a848e6339e20cf9e79ccfd4d3f4a218481709af3 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Mon, 25 Aug 2025 17:41:21 +0900 Subject: [PATCH 05/13] Make drop column unsupported in CosmosAdmin and DynamoAdmin --- .../java/com/scalar/db/common/CoreError.java | 6 ++ .../scalar/db/storage/cosmos/CosmosAdmin.java | 14 +--- .../scalar/db/storage/dynamo/DynamoAdmin.java | 16 +---- .../db/storage/cosmos/CosmosAdminTest.java | 45 +----------- .../storage/dynamo/DynamoAdminTestBase.java | 69 +------------------ 5 files changed, 14 insertions(+), 136 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 4a43088123..5d0265c502 100644 --- a/core/src/main/java/com/scalar/db/common/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/CoreError.java @@ -684,6 +684,12 @@ public enum CoreError implements ScalarDbError { "The column %s is specified as a primary key or an index key", "", ""), + COSMOS_DROP_COLUMN_NOT_SUPPORTED( + Category.USER_ERROR, + "0217", + "Drop column functionality is not supported in Cosmos DB", + "", + ""), // // Errors for the concurrency error category 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 d9cfbcebc4..c7be4cd074 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 @@ -647,18 +647,8 @@ columnName, getFullTableName(namespace, table)), @Override public void dropColumnFromTable(String namespace, String table, String columnName) throws ExecutionException { - try { - TableMetadata currentTableMetadata = getTableMetadata(namespace, table); - TableMetadata updatedTableMetadata = - TableMetadata.newBuilder(currentTableMetadata).removeColumn(columnName).build(); - upsertTableMetadata(namespace, table, updatedTableMetadata); - } catch (ExecutionException e) { - throw new ExecutionException( - String.format( - "Adding the new %s column to the %s container failed", - columnName, getFullTableName(namespace, table)), - e); - } + throw new UnsupportedOperationException( + CoreError.COSMOS_DROP_COLUMN_NOT_SUPPORTED.buildMessage()); } @Override 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 c69b71f15b..234b8f9dcc 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 @@ -1443,20 +1443,8 @@ columnName, getFullTableName(namespace, table)), @Override public void dropColumnFromTable(String nonPrefixedNamespace, String table, String columnName) throws ExecutionException { - Namespace namespace = Namespace.of(namespacePrefix, nonPrefixedNamespace); - try { - TableMetadata currentTableMetadata = getTableMetadata(nonPrefixedNamespace, table); - TableMetadata updatedTableMetadata = - TableMetadata.newBuilder(currentTableMetadata).removeColumn(columnName).build(); - - upsertTableMetadata(namespace, table, updatedTableMetadata); - } catch (ExecutionException e) { - throw new ExecutionException( - String.format( - "Dropping the %s column from the %s table failed", - columnName, getFullTableName(namespace, table)), - e); - } + throw new UnsupportedOperationException( + "Drop column functionality is not supported in DynamoDB"); } @Override 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 77e952d274..8e8e39b52a 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 @@ -1131,49 +1131,6 @@ public void addNewColumnToTable_ShouldWorkProperly() throws ExecutionException { verify(container).upsertItem(expectedCosmosTableMetadata); } - @Test - public void dropColumnFromTable_ShouldWorkProperly() throws ExecutionException { - // Arrange - String namespace = "ns"; - String table = "table"; - String column1 = "c1"; - String column2 = "c2"; - String fullTableName = getFullTableName(namespace, table); - @SuppressWarnings("unchecked") - CosmosItemResponse response = mock(CosmosItemResponse.class); - - when(client.getDatabase(METADATA_DATABASE)).thenReturn(database); - when(database.getContainer(CosmosAdmin.TABLE_METADATA_CONTAINER)).thenReturn(container); - when(container.readItem( - anyString(), - any(PartitionKey.class), - ArgumentMatchers.>any())) - .thenReturn(response); - - CosmosTableMetadata cosmosTableMetadata = - CosmosTableMetadata.newBuilder() - .partitionKeyNames(Sets.newLinkedHashSet(column1)) - .columns(ImmutableMap.of(column1, "text", column2, "int")) - .build(); - - when(response.getItem()).thenReturn(cosmosTableMetadata); - - // Act - admin.dropColumnFromTable(namespace, table, column2); - - // Assert - verify(container) - .readItem(fullTableName, new PartitionKey(fullTableName), CosmosTableMetadata.class); - - CosmosTableMetadata expectedCosmosTableMetadata = - CosmosTableMetadata.newBuilder() - .id(fullTableName) - .partitionKeyNames(Sets.newLinkedHashSet(column1)) - .columns(ImmutableMap.of(column1, "text")) - .build(); - verify(container).upsertItem(expectedCosmosTableMetadata); - } - @Test public void unsupportedOperations_ShouldThrowUnsupportedException() { // Arrange @@ -1192,11 +1149,13 @@ public void unsupportedOperations_ShouldThrowUnsupportedException() { () -> admin.importTable( namespace, table, Collections.emptyMap(), Collections.emptyMap())); + Throwable thrown4 = catchThrowable(() -> admin.dropColumnFromTable(namespace, table, column)); // 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 a25367df4e..1f971cd82c 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 @@ -1441,73 +1441,6 @@ public void addNewColumnToTable_ShouldWorkProperly() throws ExecutionException { .build()); } - @Test - public void dropColumnFromTable_ShouldWorkProperly() throws ExecutionException { - // Arrange - String column1 = "c1"; - String column2 = "c2"; - - GetItemResponse response = mock(GetItemResponse.class); - when(client.getItem(any(GetItemRequest.class))).thenReturn(response); - when(response.item()) - .thenReturn( - ImmutableMap.builder() - .put( - DynamoAdmin.METADATA_ATTR_TABLE, - AttributeValue.builder().s(getFullTableName()).build()) - .put( - DynamoAdmin.METADATA_ATTR_COLUMNS, - AttributeValue.builder() - .m( - ImmutableMap.builder() - .put(column1, AttributeValue.builder().s("text").build()) - .put(column2, AttributeValue.builder().s("int").build()) - .build()) - .build()) - .put( - DynamoAdmin.METADATA_ATTR_PARTITION_KEY, - AttributeValue.builder().l(AttributeValue.builder().s(column1).build()).build()) - .build()); - - // Act - admin.dropColumnFromTable(NAMESPACE, TABLE, column2); - - // Assert - // Get metadata - Map key = new HashMap<>(); - key.put( - DynamoAdmin.METADATA_ATTR_TABLE, AttributeValue.builder().s(getFullTableName()).build()); - verify(client) - .getItem( - GetItemRequest.builder() - .tableName(getFullMetadataTableName()) - .key(key) - .consistentRead(true) - .build()); - - // Put metadata - Map itemValues = new HashMap<>(); - itemValues.put( - DynamoAdmin.METADATA_ATTR_TABLE, AttributeValue.builder().s(getFullTableName()).build()); - Map columns = new HashMap<>(); - - columns.put( - column1, AttributeValue.builder().s(DataType.TEXT.toString().toLowerCase()).build()); - - itemValues.put(DynamoAdmin.METADATA_ATTR_COLUMNS, AttributeValue.builder().m(columns).build()); - itemValues.put( - DynamoAdmin.METADATA_ATTR_PARTITION_KEY, - AttributeValue.builder() - .l(Collections.singletonList(AttributeValue.builder().s(column1).build())) - .build()); - verify(client) - .putItem( - PutItemRequest.builder() - .tableName(getFullMetadataTableName()) - .item(itemValues) - .build()); - } - @Test public void createNamespace_WithNonExistingNamespacesTable_ShouldCreateNamespacesTableAndAddNamespace() @@ -1784,11 +1717,13 @@ public void unsupportedOperations_ShouldThrowUnsupportedException() { () -> admin.importTable( NAMESPACE, TABLE, Collections.emptyMap(), Collections.emptyMap())); + Throwable thrown4 = catchThrowable(() -> admin.dropColumnFromTable(NAMESPACE, TABLE, "c1")); // Assert assertThat(thrown1).isInstanceOf(UnsupportedOperationException.class); assertThat(thrown2).isInstanceOf(UnsupportedOperationException.class); assertThat(thrown3).isInstanceOf(UnsupportedOperationException.class); + assertThat(thrown4).isInstanceOf(UnsupportedOperationException.class); } @Test From 55ec7105cb096b8b3a2c447284b2061abb66a12d Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Thu, 28 Aug 2025 11:00:07 +0900 Subject: [PATCH 06/13] Disable drop column tests for Cosmos DB and DynamoDB --- ...sCommitAdminIntegrationTestWithCosmos.java | 22 +++++++++++++++++++ ...osAdminCaseSensitivityIntegrationTest.java | 22 +++++++++++++++++++ .../cosmos/CosmosAdminIntegrationTest.java | 22 +++++++++++++++++++ ...sactionAdminIntegrationTestWithCosmos.java | 22 +++++++++++++++++++ ...sCommitAdminIntegrationTestWithDynamo.java | 22 +++++++++++++++++++ ...moAdminCaseSensitivityIntegrationTest.java | 22 +++++++++++++++++++ .../dynamo/DynamoAdminIntegrationTest.java | 22 +++++++++++++++++++ .../DynamoAdminPermissionIntegrationTest.java | 5 +++++ ...sactionAdminIntegrationTestWithDynamo.java | 22 +++++++++++++++++++ .../main/java/com/scalar/db/api/Admin.java | 9 +++++--- 10 files changed, 187 insertions(+), 3 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 b372b419fb..f42de893c9 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,25 @@ protected Map getCreationOptions() { protected AdminTestUtils getAdminTestUtils(String testName) { return new CosmosAdminTestUtils(getProperties(testName)); } + + @Override + @Disabled("Cosmos DB does not support dropping columns") + public void dropColumnFromTable_DropColumnForEachExistingDataType_ShouldDropColumnsCorrectly() {} + + @Override + @Disabled("Cosmos DB does not support dropping columns") + public void dropColumnFromTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("Cosmos DB does not support dropping columns") + public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("Cosmos DB does not support dropping columns") + public void + dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("Cosmos DB does not support dropping columns") + public void dropColumnFromTable_IfNotExists_ForNonExistingColumn_ShouldNotThrowAnyException() {} } 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..38ead6ed0b 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,25 @@ protected Map getCreationOptions() { protected AdminTestUtils getAdminTestUtils(String testName) { return new CosmosAdminTestUtils(getProperties(testName)); } + + @Disabled("Cosmos DB does not support dropping columns") + @Override + public void dropColumnFromTable_DropColumnForEachExistingDataType_ShouldDropColumnsCorrectly() {} + + @Disabled("Cosmos DB does not support dropping columns") + @Override + public void dropColumnFromTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} + + @Disabled("Cosmos DB does not support dropping columns") + @Override + public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} + + @Disabled("Cosmos DB does not support dropping columns") + @Override + public void + dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} + + @Disabled("Cosmos DB does not support dropping columns") + @Override + public void dropColumnFromTable_IfExists_ForNonExistingColumn_ShouldNotThrowAnyException() {} } 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..8d30e3b91b 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,25 @@ protected Map getCreationOptions() { protected AdminTestUtils getAdminTestUtils(String testName) { return new CosmosAdminTestUtils(getProperties(testName)); } + + @Disabled("Cosmos DB does not support dropping columns") + @Override + public void dropColumnFromTable_DropColumnForEachExistingDataType_ShouldDropColumnsCorrectly() {} + + @Disabled("Cosmos DB does not support dropping columns") + @Override + public void dropColumnFromTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} + + @Disabled("Cosmos DB does not support dropping columns") + @Override + public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} + + @Disabled("Cosmos DB does not support dropping columns") + @Override + public void + dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} + + @Disabled("Cosmos DB does not support dropping columns") + @Override + public void dropColumnFromTable_IfExists_ForNonExistingColumn_ShouldNotThrowAnyException() {} } 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..d4ecdd43d1 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,25 @@ protected Properties getProps(String testName) { protected Map getCreationOptions() { return CosmosEnv.getCreationOptions(); } + + @Override + @Disabled("Cosmos DB does not support dropping columns") + public void dropColumnFromTable_DropColumnForEachExistingDataType_ShouldDropColumnsCorrectly() {} + + @Override + @Disabled("Cosmos DB does not support dropping columns") + public void dropColumnFromTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("Cosmos DB does not support dropping columns") + public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("Cosmos DB does not support dropping columns") + public void + dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("Cosmos DB does not support dropping columns") + public void dropColumnFromTable_IfNotExists_ForNonExistingColumn_ShouldNotThrowAnyException() {} } 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..b4c6089e34 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,25 @@ protected boolean isIndexOnBooleanColumnSupported() { protected AdminTestUtils getAdminTestUtils(String testName) { return new DynamoAdminTestUtils(getProperties(testName)); } + + @Override + @Disabled("DynamoDB does not support dropping columns") + public void dropColumnFromTable_DropColumnForEachExistingDataType_ShouldDropColumnsCorrectly() {} + + @Override + @Disabled("DynamoDB does not support dropping columns") + public void dropColumnFromTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("DynamoDB does not support dropping columns") + public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("DynamoDB does not support dropping columns") + public void + dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("DynamoDB does not support dropping columns") + public void dropColumnFromTable_IfNotExists_ForNonExistingColumn_ShouldNotThrowAnyException() {} } 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..42ec596645 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,25 @@ protected boolean isIndexOnBooleanColumnSupported() { protected AdminTestUtils getAdminTestUtils(String testName) { return new DynamoAdminTestUtils(getProperties(testName)); } + + @Disabled("DynamoDB does not support dropping columns") + @Override + public void dropColumnFromTable_DropColumnForEachExistingDataType_ShouldDropColumnsCorrectly() {} + + @Disabled("DynamoDB does not support dropping columns") + @Override + public void dropColumnFromTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} + + @Disabled("DynamoDB does not support dropping columns") + @Override + public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} + + @Disabled("DynamoDB does not support dropping columns") + @Override + public void + dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} + + @Disabled("DynamoDB does not support dropping columns") + @Override + public void dropColumnFromTable_IfExists_ForNonExistingColumn_ShouldNotThrowAnyException() {} } 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..535d246e80 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,25 @@ protected boolean isIndexOnBooleanColumnSupported() { protected AdminTestUtils getAdminTestUtils(String testName) { return new DynamoAdminTestUtils(getProperties(testName)); } + + @Disabled("DynamoDB does not support dropping columns") + @Override + public void dropColumnFromTable_DropColumnForEachExistingDataType_ShouldDropColumnsCorrectly() {} + + @Disabled("DynamoDB does not support dropping columns") + @Override + public void dropColumnFromTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} + + @Disabled("DynamoDB does not support dropping columns") + @Override + public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} + + @Disabled("DynamoDB does not support dropping columns") + @Override + public void + dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} + + @Disabled("DynamoDB does not support dropping columns") + @Override + public void dropColumnFromTable_IfExists_ForNonExistingColumn_ShouldNotThrowAnyException() {} } 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..2879b2e5ee 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 dropping columns") + public void dropColumnFromTable_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..c7f078fdff 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,25 @@ protected Map getCreationOptions() { protected boolean isIndexOnBooleanColumnSupported() { return false; } + + @Override + @Disabled("DynamoDB does not support dropping columns") + public void dropColumnFromTable_DropColumnForEachExistingDataType_ShouldDropColumnsCorrectly() {} + + @Override + @Disabled("DynamoDB does not support dropping columns") + public void dropColumnFromTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("DynamoDB does not support dropping columns") + public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("DynamoDB does not support dropping columns") + public void + dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} + + @Override + @Disabled("DynamoDB does not support dropping columns") + public void dropColumnFromTable_IfNotExists_ForNonExistingColumn_ShouldNotThrowAnyException() {} } 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 83d66ac8a6..abfde3e119 100644 --- a/core/src/main/java/com/scalar/db/api/Admin.java +++ b/core/src/main/java/com/scalar/db/api/Admin.java @@ -443,12 +443,14 @@ default void addNewColumnToTable( } /** - * Drops a column from an existing table. The column cannot be a partition or clustering key. + * Drops a column from an existing table. The column cannot be a partition key, clustering key, or + * indexed. * * @param namespace the table namespace * @param table the table name * @param columnName the name of the column to drop - * @throws IllegalArgumentException if the table does not exist or the column does not exist + * @throws IllegalArgumentException if the table or column does not exist, or the column is a + * partition key column, clustering key column, or is indexed * @throws ExecutionException if the operation fails */ void dropColumnFromTable(String namespace, String table, String columnName) @@ -462,7 +464,8 @@ void dropColumnFromTable(String namespace, String table, String columnName) * @param columnName the name of the column to drop * @param IfExists if set to true, the column will be dropped only if it exists. If set to false, * it will throw an exception if it does not exist - * @throws IllegalArgumentException if the table does not exist + * @throws IllegalArgumentException if the table does not exist, or the column is a partition key + * column, clustering key column, or is indexed * @throws ExecutionException if the operation fails */ default void dropColumnFromTable( From 0232c330225dab614547417e480604c54022fc6b Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Thu, 28 Aug 2025 13:59:16 +0900 Subject: [PATCH 07/13] Apply suggestions --- core/src/main/java/com/scalar/db/common/CoreError.java | 2 +- .../src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java | 6 +++--- .../api/DistributedTransactionAdminIntegrationTestBase.java | 3 --- 3 files changed, 4 insertions(+), 7 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 5d0265c502..f970305bf7 100644 --- a/core/src/main/java/com/scalar/db/common/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/CoreError.java @@ -681,7 +681,7 @@ public enum CoreError implements ScalarDbError { 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", + "The column is specified as a primary key or an index key. Table: %s; Column: %s", "", ""), COSMOS_DROP_COLUMN_NOT_SUPPORTED( 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 37d63cddcd..b8366c1919 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 @@ -868,19 +868,19 @@ public void dropColumnFromTable(String namespace, String table, String columnNam TableMetadata currentTableMetadata = getTableMetadata(namespace, table); TableMetadata updatedTableMetadata = TableMetadata.newBuilder(currentTableMetadata).removeColumn(columnName).build(); - String addNewColumnStatement = + String dropColumnStatement = "ALTER TABLE " + encloseFullTableName(namespace, table) + " DROP COLUMN " + enclose(columnName); try (Connection connection = dataSource.getConnection()) { - execute(connection, addNewColumnStatement); + execute(connection, dropColumnStatement); addTableMetadata(connection, namespace, table, updatedTableMetadata, false, true); } } catch (SQLException e) { throw new ExecutionException( String.format( - "Adding the new %s column to the %s table failed", + "Dropping the %s column from the %s table failed", columnName, getFullTableName(namespace, table)), e); } 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 9d1e8ad07a..80dc9238f9 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 @@ -933,9 +933,6 @@ public void dropColumnFromTable_DropColumnForEachExistingDataType_ShouldDropColu .removeColumn("c10") .removeColumn("c11") .removeColumn("c12"); - if (isTimestampTypeSupported()) { - expectedTableMetadataBuilder.removeColumn("c11").removeColumn("c12"); - } TableMetadata expectedTableMetadata = expectedTableMetadataBuilder.build(); assertThat(admin.getTableMetadata(namespace1, TABLE4)).isEqualTo(expectedTableMetadata); } finally { From b705bc32af288310eab55fc6d0d7c9a95aa27f27 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Thu, 28 Aug 2025 15:28:47 +0900 Subject: [PATCH 08/13] Fix to reorg when using Db2 --- .../java/com/scalar/db/storage/jdbc/JdbcAdmin.java | 10 ++++------ .../java/com/scalar/db/storage/jdbc/RdbEngineDb2.java | 11 +++++++++++ .../com/scalar/db/storage/jdbc/RdbEngineStrategy.java | 9 +++++++++ 3 files changed, 24 insertions(+), 6 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 b8366c1919..0af4e6cbbd 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 @@ -868,13 +868,11 @@ public void dropColumnFromTable(String namespace, String table, String columnNam TableMetadata currentTableMetadata = getTableMetadata(namespace, table); TableMetadata updatedTableMetadata = TableMetadata.newBuilder(currentTableMetadata).removeColumn(columnName).build(); - String dropColumnStatement = - "ALTER TABLE " - + encloseFullTableName(namespace, table) - + " DROP COLUMN " - + enclose(columnName); + String[] dropColumnStatements = rdbEngine.dropColumnSql(namespace, table, columnName); try (Connection connection = dataSource.getConnection()) { - execute(connection, dropColumnStatement); + for (String dropColumnStatement : dropColumnStatements) { + execute(connection, dropColumnStatement); + } addTableMetadata(connection, namespace, table, updatedTableMetadata, false, true); } } catch (SQLException e) { 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 cc52d349b2..b68a3fad61 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 @@ -246,6 +246,17 @@ public void dropNamespaceTranslateSQLException(SQLException e, String namespace) throw new ExecutionException("Dropping the schema failed: " + namespace, e); } + @Override + public String[] dropColumnSql(String namespace, String table, String columnName) { + return new String[] { + "ALTER TABLE " + + encloseFullTableName(namespace, table) + + " DROP COLUMN " + + enclose(columnName), + "CALL SYSPROC.ADMIN_CMD('REORG TABLE " + encloseFullTableName(namespace, table) + "')" + }; + } + @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..d5992cfea4 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,15 @@ default String truncateTableSql(String namespace, String table) { void dropNamespaceTranslateSQLException(SQLException e, String namespace) throws ExecutionException; + default String[] dropColumnSql(String namespace, String table, String columnName) { + return new String[] { + "ALTER TABLE " + + encloseFullTableName(namespace, table) + + " DROP COLUMN " + + enclose(columnName) + }; + } + String alterColumnTypeSql(String namespace, String table, String columnName, String columnType); String tableExistsInternalTableCheckSql(String fullTableName); From c38a647c78348b1a4cb3a5618dfef20358afa518 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Thu, 28 Aug 2025 16:14:57 +0900 Subject: [PATCH 09/13] Update unit test for Db2 --- core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java | 1 + 1 file changed, 1 insertion(+) 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 85dc10d425..7cd83297fc 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 @@ -3027,6 +3027,7 @@ public void dropColumnFromTable_ForDb2_ShouldWorkProperly() + METADATA_SCHEMA + "\".\"metadata\" WHERE \"full_table_name\"=? ORDER BY \"ordinal_position\" ASC", "ALTER TABLE \"ns\".\"table\" DROP COLUMN \"c2\"", + "CALL SYSPROC.ADMIN_CMD('REORG TABLE \"ns\".\"table\"')", "DELETE FROM \"" + METADATA_SCHEMA + "\".\"metadata\" WHERE \"full_table_name\" = 'ns.table'", From 7c2a74b2acf1da8bdc397bdf0930e1bb8973bf4f Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Thu, 28 Aug 2025 18:59:15 +0900 Subject: [PATCH 10/13] Fix to quote column name in CassandraAdmin --- .../java/com/scalar/db/storage/cassandra/CassandraAdmin.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 39b1cc3951..1e9d90e86d 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 @@ -421,7 +421,7 @@ public void dropColumnFromTable(String namespace, String table, String columnNam try { String alterTableQuery = SchemaBuilder.alterTable(quoteIfNecessary(namespace), quoteIfNecessary(table)) - .dropColumn(columnName) + .dropColumn(quoteIfNecessary(columnName)) .getQueryString(); clusterManager.getSession().execute(alterTableQuery); From 31cdfd30629cfb85a4c49cf1dd8cbcdd4a49d326 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Mon, 1 Sep 2025 13:30:48 +0900 Subject: [PATCH 11/13] Apply suggestions --- core/src/main/java/com/scalar/db/common/CoreError.java | 6 ++++++ .../main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java | 3 ++- ...istributedStorageAdminPermissionIntegrationTestBase.java | 1 + 3 files changed, 9 insertions(+), 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 f970305bf7..c938493579 100644 --- a/core/src/main/java/com/scalar/db/common/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/CoreError.java @@ -690,6 +690,12 @@ public enum CoreError implements ScalarDbError { "Drop column functionality is not supported in Cosmos DB", "", ""), + DYNAMO_DROP_COLUMN_NOT_SUPPORTED( + Category.USER_ERROR, + "0218", + "Drop column functionality is not supported in DynamoDB", + "", + ""), // // Errors for the concurrency error category 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 234b8f9dcc..74ab81da02 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 @@ -9,6 +9,7 @@ import com.scalar.db.api.Scan.Ordering.Order; import com.scalar.db.api.StorageInfo; import com.scalar.db.api.TableMetadata; +import com.scalar.db.common.CoreError; import com.scalar.db.common.StorageInfoImpl; import com.scalar.db.config.DatabaseConfig; import com.scalar.db.exception.storage.ExecutionException; @@ -1444,7 +1445,7 @@ columnName, getFullTableName(namespace, table)), public void dropColumnFromTable(String nonPrefixedNamespace, String table, String columnName) throws ExecutionException { throw new UnsupportedOperationException( - "Drop column functionality is not supported in DynamoDB"); + CoreError.DYNAMO_DROP_COLUMN_NOT_SUPPORTED.buildMessage()); } @Override 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 035e31266d..4402e5222d 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 @@ -314,6 +314,7 @@ public void dropColumnFromTable_WithSufficientPermission_ShouldSucceed() // Arrange createNamespaceByRoot(); createTableByRoot(); + // Act Assert assertThatCode(() -> adminForNormalUser.dropColumnFromTable(NAMESPACE, TABLE, COL_NAME3)) .doesNotThrowAnyException(); From d55d59586960e189ec6e24f44f7cb21d9f3b1e1e Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Tue, 2 Sep 2025 16:43:53 +0900 Subject: [PATCH 12/13] Fix to allow dropping indexed columns --- ...sCommitAdminIntegrationTestWithCosmos.java | 7 +++- ...osAdminCaseSensitivityIntegrationTest.java | 7 +++- .../cosmos/CosmosAdminIntegrationTest.java | 7 +++- ...sactionAdminIntegrationTestWithCosmos.java | 7 +++- ...sCommitAdminIntegrationTestWithDynamo.java | 7 +++- ...moAdminCaseSensitivityIntegrationTest.java | 7 +++- .../dynamo/DynamoAdminIntegrationTest.java | 7 +++- ...sactionAdminIntegrationTestWithDynamo.java | 7 +++- .../main/java/com/scalar/db/api/Admin.java | 7 ++-- .../CheckedDistributedStorageAdmin.java | 9 +++-- .../java/com/scalar/db/common/CoreError.java | 4 +- ...ibutedStorageAdminIntegrationTestBase.java | 37 +++++++++++++++++-- ...edTransactionAdminIntegrationTestBase.java | 37 +++++++++++++++++-- 13 files changed, 120 insertions(+), 30 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 f42de893c9..20d061d508 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 @@ -38,8 +38,11 @@ public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentE @Override @Disabled("Cosmos DB does not support dropping columns") - public void - dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} + public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() {} + + @Disabled("Cosmos DB does not support dropping columns") + @Override + public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() {} @Override @Disabled("Cosmos DB does not support dropping columns") 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 38ead6ed0b..e26cd4295e 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 @@ -38,8 +38,11 @@ public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentE @Disabled("Cosmos DB does not support dropping columns") @Override - public void - dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} + public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() {} + + @Disabled("Cosmos DB does not support dropping columns") + @Override + public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() {} @Disabled("Cosmos DB does not support dropping columns") @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 8d30e3b91b..6c3aaf948c 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 @@ -37,8 +37,11 @@ public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentE @Disabled("Cosmos DB does not support dropping columns") @Override - public void - dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} + public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() {} + + @Disabled("Cosmos DB does not support dropping columns") + @Override + public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() {} @Disabled("Cosmos DB does not support dropping columns") @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 d4ecdd43d1..96deded7bf 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 @@ -32,8 +32,11 @@ public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentE @Override @Disabled("Cosmos DB does not support dropping columns") - public void - dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} + public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() {} + + @Disabled("Cosmos DB does not support dropping columns") + @Override + public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() {} @Override @Disabled("Cosmos DB does not support dropping columns") 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 b4c6089e34..b6163a00e1 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 @@ -43,8 +43,11 @@ public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentE @Override @Disabled("DynamoDB does not support dropping columns") - public void - dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} + public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() {} + + @Disabled("DynamoDB does not support dropping columns") + @Override + public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() {} @Override @Disabled("DynamoDB does not support dropping columns") 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 42ec596645..12d673135d 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 @@ -43,8 +43,11 @@ public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentE @Disabled("DynamoDB does not support dropping columns") @Override - public void - dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} + public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() {} + + @Disabled("DynamoDB does not support dropping columns") + @Override + public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() {} @Disabled("DynamoDB does not support dropping columns") @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 535d246e80..c86fd33073 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 @@ -42,8 +42,11 @@ public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentE @Disabled("DynamoDB does not support dropping columns") @Override - public void - dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} + public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() {} + + @Disabled("DynamoDB does not support dropping columns") + @Override + public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() {} @Disabled("DynamoDB does not support dropping columns") @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 c7f078fdff..83dc86eef4 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 @@ -37,8 +37,11 @@ public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentE @Override @Disabled("DynamoDB does not support dropping columns") - public void - dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() {} + public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() {} + + @Disabled("DynamoDB does not support dropping columns") + @Override + public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() {} @Override @Disabled("DynamoDB does not support dropping columns") 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 daf5847aed..b4c150b904 100644 --- a/core/src/main/java/com/scalar/db/api/Admin.java +++ b/core/src/main/java/com/scalar/db/api/Admin.java @@ -480,8 +480,8 @@ default void addNewColumnToTable( } /** - * Drops a column from an existing table. The column cannot be a partition key, clustering key, or - * indexed. + * Drops a column from an existing table. The column cannot be a partition key or a clustering + * key. * * @param namespace the table namespace * @param table the table name @@ -494,7 +494,8 @@ void dropColumnFromTable(String namespace, String table, String columnName) throws ExecutionException; /** - * Drops a column from an existing table. The column cannot be a partition or clustering key. + * Drops a column from an existing table. The column cannot be a partition key or a clustering + * key. * * @param namespace the table namespace * @param table the table name 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 eaa8fffc0d..9769dbcef9 100644 --- a/core/src/main/java/com/scalar/db/common/CheckedDistributedStorageAdmin.java +++ b/core/src/main/java/com/scalar/db/common/CheckedDistributedStorageAdmin.java @@ -311,13 +311,16 @@ public void dropColumnFromTable(String namespace, String table, String columnNam } if (tableMetadata.getPartitionKeyNames().contains(columnName) - || tableMetadata.getClusteringKeyNames().contains(columnName) - || tableMetadata.getSecondaryIndexNames().contains(columnName)) { + || tableMetadata.getClusteringKeyNames().contains(columnName)) { throw new IllegalArgumentException( - CoreError.COLUMN_SPECIFIED_AS_PRIMARY_KEY_OR_INDEX_KEY.buildMessage( + CoreError.COLUMN_SPECIFIED_AS_PRIMARY_KEY.buildMessage( ScalarDbUtils.getFullTableName(namespace, table), columnName)); } + if (tableMetadata.getSecondaryIndexNames().contains(columnName)) { + dropIndex(namespace, table, columnName); + } + try { admin.dropColumnFromTable(namespace, table, columnName); } catch (ExecutionException e) { 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 c938493579..b3b9aa862e 100644 --- a/core/src/main/java/com/scalar/db/common/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/CoreError.java @@ -678,10 +678,10 @@ public enum CoreError implements ScalarDbError { "Mutations across multiple storages are not allowed. Mutations: %s", "", ""), - COLUMN_SPECIFIED_AS_PRIMARY_KEY_OR_INDEX_KEY( + COLUMN_SPECIFIED_AS_PRIMARY_KEY( Category.USER_ERROR, "0216", - "The column is specified as a primary key or an index key. Table: %s; Column: %s", + "The column is specified as a primary key. Table: %s; Column: %s", "", ""), COSMOS_DROP_COLUMN_NOT_SUPPORTED( 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 c49fce6baa..952e70b9b3 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 @@ -1095,7 +1095,7 @@ public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentE } @Test - public void dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() { + public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() { // Arrange // Act Assert @@ -1103,8 +1103,39 @@ public void dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArg .isInstanceOf(IllegalArgumentException.class); assertThatThrownBy(() -> admin.dropColumnFromTable(namespace1, getTable1(), getColumnName3())) .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> admin.dropColumnFromTable(namespace1, getTable1(), getColumnName5())) - .isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() + 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(getColumnName2()) + .build(); + admin.createTable(namespace1, getTable4(), currentTableMetadata, options); + + // Act + admin.dropColumnFromTable(namespace1, getTable4(), getColumnName2()); + + // Assert + TableMetadata expectedTableMetadata = + TableMetadata.newBuilder() + .addColumn(getColumnName1(), DataType.INT) + .addColumn(getColumnName3(), DataType.TEXT) + .addPartitionKey(getColumnName1()) + .build(); + assertThat(admin.getTableMetadata(namespace1, getTable4())).isEqualTo(expectedTableMetadata); + assertThat(admin.indexExists(namespace1, getTable4(), getColumnName2())).isFalse(); + } 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 80bec0c42e..3c39d3dfaf 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 @@ -980,7 +980,7 @@ public void dropColumnFromTable_ForNonExistingColumn_ShouldThrowIllegalArgumentE } @Test - public void dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArgumentException() { + public void dropColumnFromTable_ForPrimaryKeyColumn_ShouldThrowIllegalArgumentException() { // Arrange // Act Assert @@ -988,8 +988,39 @@ public void dropColumnFromTable_ForPrimaryOrIndexKeyColumn_ShouldThrowIllegalArg .isInstanceOf(IllegalArgumentException.class); assertThatThrownBy(() -> admin.dropColumnFromTable(namespace1, TABLE1, "c3")) .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> admin.dropColumnFromTable(namespace1, TABLE1, "c5")) - .isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void dropColumnFromTable_ForIndexedColumn_ShouldDropColumnAndIndexCorrectly() + 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("c2") + .build(); + admin.createTable(namespace1, TABLE4, currentTableMetadata, options); + + // Act + admin.dropColumnFromTable(namespace1, TABLE4, "c2"); + + // Assert + TableMetadata expectedTableMetadata = + TableMetadata.newBuilder() + .addColumn("c1", DataType.INT) + .addColumn("c3", DataType.TEXT) + .addPartitionKey("c1") + .build(); + assertThat(admin.getTableMetadata(namespace1, TABLE4)).isEqualTo(expectedTableMetadata); + assertThat(admin.indexExists(namespace1, TABLE4, "c2")).isFalse(); + } finally { + admin.dropTable(namespace1, TABLE4, true); + } } @Test From 9ba22064fd9c7de18a1ff23d2427cbdb5bb62a29 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Wed, 3 Sep 2025 22:13:39 +0900 Subject: [PATCH 13/13] Apply suggestions --- core/src/main/java/com/scalar/db/api/Admin.java | 4 ++-- .../db/common/CheckedDistributedStorageAdmin.java | 2 +- .../main/java/com/scalar/db/common/CoreError.java | 12 ++++-------- 3 files changed, 7 insertions(+), 11 deletions(-) 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 b4c150b904..b309c2ec38 100644 --- a/core/src/main/java/com/scalar/db/api/Admin.java +++ b/core/src/main/java/com/scalar/db/api/Admin.java @@ -487,7 +487,7 @@ default void addNewColumnToTable( * @param table the table name * @param columnName the name of the column to drop * @throws IllegalArgumentException if the table or column does not exist, or the column is a - * partition key column, clustering key column, or is indexed + * partition key column or clustering key column * @throws ExecutionException if the operation fails */ void dropColumnFromTable(String namespace, String table, String columnName) @@ -503,7 +503,7 @@ void dropColumnFromTable(String namespace, String table, String columnName) * @param IfExists if set to true, the column will be dropped only if it exists. If set to false, * it will throw an exception if it does not exist * @throws IllegalArgumentException if the table does not exist, or the column is a partition key - * column, clustering key column, or is indexed + * column or clustering key column * @throws ExecutionException if the operation fails */ default void dropColumnFromTable( 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 9769dbcef9..afc4592b64 100644 --- a/core/src/main/java/com/scalar/db/common/CheckedDistributedStorageAdmin.java +++ b/core/src/main/java/com/scalar/db/common/CheckedDistributedStorageAdmin.java @@ -313,7 +313,7 @@ public void dropColumnFromTable(String namespace, String table, String columnNam if (tableMetadata.getPartitionKeyNames().contains(columnName) || tableMetadata.getClusteringKeyNames().contains(columnName)) { throw new IllegalArgumentException( - CoreError.COLUMN_SPECIFIED_AS_PRIMARY_KEY.buildMessage( + CoreError.DROP_PRIMARY_KEY_COLUMN_NOT_SUPPORTED.buildMessage( ScalarDbUtils.getFullTableName(namespace, table), columnName)); } 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 b3b9aa862e..c187941fd7 100644 --- a/core/src/main/java/com/scalar/db/common/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/CoreError.java @@ -678,24 +678,20 @@ public enum CoreError implements ScalarDbError { "Mutations across multiple storages are not allowed. Mutations: %s", "", ""), - COLUMN_SPECIFIED_AS_PRIMARY_KEY( + DROP_PRIMARY_KEY_COLUMN_NOT_SUPPORTED( Category.USER_ERROR, "0216", - "The column is specified as a primary key. Table: %s; Column: %s", + "Primary key columns cannot be dropped. Table: %s; Column: %s", "", ""), COSMOS_DROP_COLUMN_NOT_SUPPORTED( Category.USER_ERROR, "0217", - "Drop column functionality is not supported in Cosmos DB", + "Cosmos DB does not support the dropping column feature", "", ""), DYNAMO_DROP_COLUMN_NOT_SUPPORTED( - Category.USER_ERROR, - "0218", - "Drop column functionality is not supported in DynamoDB", - "", - ""), + Category.USER_ERROR, "0218", "DynamoDB does not support the dropping column feature", "", ""), // // Errors for the concurrency error category