diff --git a/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminPermissionIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminPermissionIntegrationTest.java index 46ab9e6108..04df540728 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminPermissionIntegrationTest.java +++ b/core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraAdminPermissionIntegrationTest.java @@ -126,16 +126,6 @@ protected void waitForTableDeletion() { } } - @Test - @Override - @Disabled("Import-related functionality is not supported in Cassandra") - public void getImportTableMetadata_WithSufficientPermission_ShouldSucceed() {} - - @Test - @Override - @Disabled("Import-related functionality is not supported in Cassandra") - public void addRawColumnToTable_WithSufficientPermission_ShouldSucceed() {} - @Test @Override @Disabled("Import-related functionality is not supported in Cassandra") 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 c79c8afdb5..6c003d9861 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 @@ -45,16 +45,6 @@ protected void sleepBetweenTests() { Uninterruptibles.sleepUninterruptibly(SLEEP_BETWEEN_TESTS_SECONDS, TimeUnit.SECONDS); } - @Test - @Override - @Disabled("Import-related functionality is not supported in DynamoDB") - public void getImportTableMetadata_WithSufficientPermission_ShouldSucceed() {} - - @Test - @Override - @Disabled("Import-related functionality is not supported in DynamoDB") - public void addRawColumnToTable_WithSufficientPermission_ShouldSucceed() {} - @Test @Override @Disabled("Import-related functionality is not supported in DynamoDB") diff --git a/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcSchemaLoaderImportIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcSchemaLoaderImportIntegrationTest.java index d5d58bcc38..625a5c2697 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcSchemaLoaderImportIntegrationTest.java +++ b/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcSchemaLoaderImportIntegrationTest.java @@ -10,6 +10,7 @@ import java.util.Map; import java.util.Properties; import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledIf; import org.slf4j.Logger; @@ -191,6 +192,7 @@ public void importTables_ImportableTablesAndNonRelatedSameNameTableGiven_ShouldI super.importTables_ImportableTablesAndNonRelatedSameNameTableGiven_ShouldImportProperly(); } + @AfterAll @Override public void afterAll() { try { diff --git a/core/src/main/java/com/scalar/db/api/DistributedStorageAdmin.java b/core/src/main/java/com/scalar/db/api/DistributedStorageAdmin.java index 32c239a590..ce76905fa7 100644 --- a/core/src/main/java/com/scalar/db/api/DistributedStorageAdmin.java +++ b/core/src/main/java/com/scalar/db/api/DistributedStorageAdmin.java @@ -1,9 +1,6 @@ package com.scalar.db.api; import com.scalar.db.exception.storage.ExecutionException; -import com.scalar.db.io.DataType; -import java.util.Collections; -import java.util.Map; /** * An administrative interface for distributed storage implementations. The user can execute @@ -44,50 +41,6 @@ */ public interface DistributedStorageAdmin extends Admin, AutoCloseable { - /** - * Get import table metadata in the ScalarDB format. - * - * @param namespace namespace name of import table - * @param table import table name - * @throws IllegalArgumentException if the table does not exist - * @throws IllegalStateException if the table does not meet the requirement of ScalarDB table - * @throws ExecutionException if the operation fails - * @return import table metadata in the ScalarDB format - */ - default TableMetadata getImportTableMetadata(String namespace, String table) - throws ExecutionException { - return getImportTableMetadata(namespace, table, Collections.emptyMap()); - } - - /** - * Get import table metadata in the ScalarDB format. - * - * @param namespace namespace name of import table - * @param table import table name - * @param overrideColumnsType a map of column data type by column name. Only set the column for - * which you want to override the default data type mapping. - * @throws IllegalArgumentException if the table does not exist - * @throws IllegalStateException if the table does not meet the requirement of ScalarDB table - * @throws ExecutionException if the operation fails - * @return import table metadata in the ScalarDB format - */ - TableMetadata getImportTableMetadata( - String namespace, String table, Map overrideColumnsType) - throws ExecutionException; - - /** - * Add a column in the table without updating the metadata table in ScalarDB. - * - * @param namespace namespace name of import table - * @param table import table name - * @param columnName name of the column to be added - * @param columnType type of the column to be added - * @throws IllegalArgumentException if the table does not exist - * @throws ExecutionException if the operation fails - */ - void addRawColumnToTable(String namespace, String table, String columnName, DataType columnType) - throws ExecutionException; - /** * Returns the storage information. * diff --git a/core/src/main/java/com/scalar/db/common/CommonDistributedStorageAdmin.java b/core/src/main/java/com/scalar/db/common/CommonDistributedStorageAdmin.java index bfaeed741c..6fc5b5a5ac 100644 --- a/core/src/main/java/com/scalar/db/common/CommonDistributedStorageAdmin.java +++ b/core/src/main/java/com/scalar/db/common/CommonDistributedStorageAdmin.java @@ -451,20 +451,6 @@ public Set getNamespaceNames() throws ExecutionException { } } - @Override - public TableMetadata getImportTableMetadata( - String namespace, String table, Map overrideColumnsType) - throws ExecutionException { - try { - return admin.getImportTableMetadata(namespace, table, overrideColumnsType); - } catch (ExecutionException e) { - throw new ExecutionException( - CoreError.GETTING_IMPORT_TABLE_METADATA_FAILED.buildMessage( - ScalarDbUtils.getFullTableName(namespace, table)), - e); - } - } - @Override public void importTable( String namespace, @@ -489,20 +475,6 @@ public void importTable( } } - @Override - public void addRawColumnToTable( - String namespace, String table, String columnName, DataType columnType) - throws ExecutionException { - try { - admin.addRawColumnToTable(namespace, table, columnName, columnType); - } catch (ExecutionException e) { - throw new ExecutionException( - CoreError.ADDING_RAW_COLUMN_TO_TABLE_FAILED.buildMessage( - ScalarDbUtils.getFullTableName(namespace, table), columnName, columnType), - e); - } - } - @Override public void upgrade(Map options) 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 c4723f5fdf..25857f0759 100644 --- a/core/src/main/java/com/scalar/db/common/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/CoreError.java @@ -377,16 +377,16 @@ public enum CoreError implements ScalarDbError { ""), MULTI_STORAGE_STORAGE_NOT_FOUND( Category.USER_ERROR, "0084", "Storage not found. Storage: %s", "", ""), - JDBC_NAMESPACE_NAME_NOT_ACCEPTABLE( - Category.USER_ERROR, "0085", "The namespace name is not acceptable. Namespace: %s", "", ""), - JDBC_TABLE_NAME_NOT_ACCEPTABLE( - Category.USER_ERROR, "0086", "The table name is not acceptable. Table: %s", "", ""), - JDBC_IMPORT_NOT_SUPPORTED( + JDBC_SQLITE_NAMESPACE_NAME_NOT_ACCEPTABLE( Category.USER_ERROR, - "0087", - "Importing tables is not allowed in the RDB engine. RDB engine: %s", + "0085", + "The namespace name is not acceptable in SQLite. Namespace: %s", "", ""), + JDBC_SQLITE_TABLE_NAME_NOT_ACCEPTABLE( + Category.USER_ERROR, "0086", "The table name is not acceptable in SQLite. Table: %s", "", ""), + JDBC_SQLITE_IMPORT_NOT_SUPPORTED( + Category.USER_ERROR, "0087", "Importing tables is not allowed in SQLite", "", ""), JDBC_IMPORT_TABLE_WITHOUT_PRIMARY_KEY( Category.USER_ERROR, "0088", "The %s table must have a primary key", "", ""), JDBC_RDB_ENGINE_NOT_SUPPORTED( 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 459ebec308..5a49a84ddb 100644 --- a/core/src/main/java/com/scalar/db/service/AdminService.java +++ b/core/src/main/java/com/scalar/db/service/AdminService.java @@ -120,20 +120,6 @@ public void renameTable(String namespace, String oldTableName, String newTableNa admin.renameTable(namespace, oldTableName, newTableName); } - @Override - public TableMetadata getImportTableMetadata( - String namespace, String table, Map overrideColumnsType) - throws ExecutionException { - return admin.getImportTableMetadata(namespace, table, overrideColumnsType); - } - - @Override - public void addRawColumnToTable( - String namespace, String table, String columnName, DataType columnType) - throws ExecutionException { - admin.addRawColumnToTable(namespace, table, columnName, columnType); - } - @Override public void importTable( String namespace, 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 2484f7c6d6..87812c293d 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 @@ -285,20 +285,6 @@ private TableMetadata createTableMetadata(com.datastax.driver.core.TableMetadata return builder.build(); } - @Override - public TableMetadata getImportTableMetadata( - String namespace, String table, Map overrideColumnsType) { - throw new UnsupportedOperationException( - CoreError.CASSANDRA_IMPORT_NOT_SUPPORTED.buildMessage()); - } - - @Override - public void addRawColumnToTable( - String namespace, String table, String columnName, DataType columnType) { - throw new UnsupportedOperationException( - CoreError.CASSANDRA_IMPORT_NOT_SUPPORTED.buildMessage()); - } - @Override public void importTable( String namespace, 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 c2c3f3eb0c..d98213aa2e 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 @@ -682,18 +682,6 @@ public void renameTable(String namespace, String oldTableName, String newTableNa CoreError.COSMOS_RENAME_TABLE_NOT_SUPPORTED.buildMessage()); } - @Override - public TableMetadata getImportTableMetadata( - String namespace, String table, Map overrideColumnsType) { - throw new UnsupportedOperationException(CoreError.COSMOS_IMPORT_NOT_SUPPORTED.buildMessage()); - } - - @Override - public void addRawColumnToTable( - String namespace, String table, String columnName, DataType columnType) { - throw new UnsupportedOperationException(CoreError.COSMOS_IMPORT_NOT_SUPPORTED.buildMessage()); - } - @Override public void importTable( String namespace, 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 17959faf5e..c7656f137f 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 @@ -1466,18 +1466,6 @@ public void renameTable(String namespace, String oldTableName, String newTableNa CoreError.DYNAMO_RENAME_TABLE_NOT_SUPPORTED.buildMessage()); } - @Override - public TableMetadata getImportTableMetadata( - String namespace, String table, Map overrideColumnsType) { - throw new UnsupportedOperationException(CoreError.DYNAMO_IMPORT_NOT_SUPPORTED.buildMessage()); - } - - @Override - public void addRawColumnToTable( - String namespace, String table, String columnName, DataType columnType) { - throw new UnsupportedOperationException(CoreError.DYNAMO_IMPORT_NOT_SUPPORTED.buildMessage()); - } - @Override public void importTable( String namespace, 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 9956fbcad1..8b78637d44 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 @@ -147,10 +147,8 @@ static void execute( @Override public void createNamespace(String namespace, Map options) throws ExecutionException { - if (!rdbEngine.isValidNamespaceOrTableName(namespace)) { - throw new IllegalArgumentException( - CoreError.JDBC_NAMESPACE_NAME_NOT_ACCEPTABLE.buildMessage(namespace)); - } + rdbEngine.throwIfInvalidNamespaceName(namespace); + try (Connection connection = dataSource.getConnection()) { execute(connection, rdbEngine.createSchemaSqls(namespace)); createNamespacesTableIfNotExists(connection); @@ -182,10 +180,8 @@ void createTableInternal( TableMetadata metadata, boolean ifNotExists) throws SQLException { - if (!rdbEngine.isValidNamespaceOrTableName(table)) { - throw new IllegalArgumentException( - CoreError.JDBC_TABLE_NAME_NOT_ACCEPTABLE.buildMessage(table)); - } + rdbEngine.throwIfInvalidTableName(table); + String createTableStatement = "CREATE TABLE " + encloseFullTableName(schema, table) + "("; // Order the columns for their creation by (partition keys >> clustering keys >> other columns) LinkedHashSet sortedColumnNames = @@ -571,18 +567,13 @@ public TableMetadata getTableMetadata(String namespace, String table) throws Exe return builder.build(); } - @Override - public TableMetadata getImportTableMetadata( + @VisibleForTesting + TableMetadata getImportTableMetadata( String namespace, String table, Map overrideColumnsType) throws ExecutionException { TableMetadata.Builder builder = TableMetadata.newBuilder(); boolean primaryKeyExists = false; - if (!rdbEngine.isImportable()) { - throw new UnsupportedOperationException( - CoreError.JDBC_IMPORT_NOT_SUPPORTED.buildMessage(rdbEngine.getClass().getName())); - } - try (Connection connection = dataSource.getConnection()) { rdbEngine.setConnectionToReadOnly(connection, true); @@ -640,6 +631,8 @@ public void importTable( Map options, Map overrideColumnsType) throws ExecutionException { + rdbEngine.throwIfImportNotSupported(); + try (Connection connection = dataSource.getConnection()) { TableMetadata tableMetadata = getImportTableMetadata(namespace, table, overrideColumnsType); createNamespacesTableIfNotExists(connection); @@ -876,10 +869,8 @@ private boolean tableExistsInternal(Connection connection, String namespace, Str @Override public void repairNamespace(String namespace, Map options) throws ExecutionException { - if (!rdbEngine.isValidNamespaceOrTableName(namespace)) { - throw new IllegalArgumentException( - CoreError.JDBC_NAMESPACE_NAME_NOT_ACCEPTABLE.buildMessage(namespace)); - } + rdbEngine.throwIfInvalidNamespaceName(namespace); + try (Connection connection = dataSource.getConnection()) { createSchemaIfNotExists(connection, namespace); createNamespacesTableIfNotExists(connection); @@ -893,6 +884,8 @@ public void repairNamespace(String namespace, Map options) public void repairTable( String namespace, String table, TableMetadata metadata, Map options) throws ExecutionException { + rdbEngine.throwIfInvalidNamespaceName(table); + try (Connection connection = dataSource.getConnection()) { createTableInternal(connection, namespace, table, metadata, true); addTableMetadata(connection, namespace, table, metadata, true, true); @@ -1060,34 +1053,6 @@ public void renameTable(String namespace, String oldTableName, String newTableNa } } - @Override - public void addRawColumnToTable( - String namespace, String table, String columnName, DataType columnType) - throws ExecutionException { - try (Connection connection = dataSource.getConnection()) { - if (!tableExistsInternal(connection, namespace, table)) { - throw new IllegalArgumentException( - CoreError.TABLE_NOT_FOUND.buildMessage(getFullTableName(namespace, table))); - } - - String addNewColumnStatement = - "ALTER TABLE " - + encloseFullTableName(namespace, table) - + " ADD " - + enclose(columnName) - + " " - + rdbEngine.getDataTypeForEngine(columnType); - - execute(connection, addNewColumnStatement); - } catch (SQLException e) { - throw new ExecutionException( - String.format( - "Adding the new %s column to the %s table failed", - columnName, getFullTableName(namespace, table)), - e); - } - } - @VisibleForTesting void createIndex( Connection connection, String schema, String table, String indexedColumn, boolean ifNotExists) diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlite.java b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlite.java index 895420d63f..0c906fa7a0 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlite.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlite.java @@ -167,8 +167,19 @@ public DataType getDataTypeForScalarDbInternal( } @Override - public boolean isValidNamespaceOrTableName(String namespaceOrTableName) { - return !namespaceOrTableName.contains(NAMESPACE_SEPARATOR); + public void throwIfInvalidNamespaceName(String namespaceName) { + if (namespaceName.contains(NAMESPACE_SEPARATOR)) { + throw new IllegalArgumentException( + CoreError.JDBC_SQLITE_NAMESPACE_NAME_NOT_ACCEPTABLE.buildMessage(namespaceName)); + } + } + + @Override + public void throwIfInvalidTableName(String tableName) { + if (tableName.contains(NAMESPACE_SEPARATOR)) { + throw new IllegalArgumentException( + CoreError.JDBC_SQLITE_TABLE_NAME_NOT_ACCEPTABLE.buildMessage(tableName)); + } } @Override @@ -310,8 +321,9 @@ public Driver getDriver() { } @Override - public boolean isImportable() { - return false; + public void throwIfImportNotSupported() { + throw new UnsupportedOperationException( + CoreError.JDBC_SQLITE_IMPORT_NOT_SUPPORTED.buildMessage()); } @Override diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java index ec930a25f9..38bb3469e0 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 @@ -73,9 +73,9 @@ DataType getDataTypeForScalarDb( String[] createSchemaIfNotExistsSqls(String fullSchema); - default boolean isValidNamespaceOrTableName(String tableName) { - return true; - } + default void throwIfInvalidNamespaceName(String namespaceName) {} + + default void throwIfInvalidTableName(String tableName) {} String createTableInternalPrimaryKeyClause( boolean hasDescClusteringOrder, TableMetadata metadata); @@ -165,9 +165,7 @@ default String encloseFullTableName(String schema, String table) { Driver getDriver(); - default boolean isImportable() { - return true; - } + default void throwIfImportNotSupported() {} /** * Return properly-preprocessed like pattern for each underlying database. 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 6c27396e48..af82ac170e 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 @@ -231,20 +231,6 @@ public void renameTable(String namespace, String oldTableName, String newTableNa getAdmin(namespace, oldTableName).renameTable(namespace, oldTableName, newTableName); } - @Override - public TableMetadata getImportTableMetadata( - String namespace, String table, Map overrideColumnsType) - throws ExecutionException { - return getAdmin(namespace, table).getImportTableMetadata(namespace, table, overrideColumnsType); - } - - @Override - public void addRawColumnToTable( - String namespace, String table, String columnName, DataType columnType) - throws ExecutionException { - getAdmin(namespace, table).addRawColumnToTable(namespace, table, columnName, columnType); - } - @Override public void importTable( String namespace, 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 7af3b5a109..5e036df4e4 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 @@ -323,18 +323,16 @@ public void importTable( throws ExecutionException { checkNamespace(namespace); + // import the original table + admin.importTable(namespace, table, options, overrideColumnsType); + TableMetadata tableMetadata = admin.getTableMetadata(namespace, table); - if (tableMetadata != null) { - throw new IllegalArgumentException( - CoreError.TABLE_ALREADY_EXISTS.buildMessage( - ScalarDbUtils.getFullTableName(namespace, table))); - } - tableMetadata = admin.getImportTableMetadata(namespace, table, overrideColumnsType); + assert tableMetadata != null; // add transaction metadata columns for (Map.Entry entry : ConsensusCommitUtils.getTransactionMetaColumns().entrySet()) { - admin.addRawColumnToTable(namespace, table, entry.getKey(), entry.getValue()); + admin.addNewColumnToTable(namespace, table, entry.getKey(), entry.getValue()); } // add before image columns @@ -342,12 +340,8 @@ public void importTable( for (String columnName : nonPrimaryKeyColumns) { String beforeColumnName = getBeforeImageColumnName(columnName, tableMetadata); DataType columnType = tableMetadata.getColumnDataType(columnName); - admin.addRawColumnToTable(namespace, table, beforeColumnName, columnType); + admin.addNewColumnToTable(namespace, table, beforeColumnName, columnType); } - - // add ScalarDB metadata - admin.repairNamespace(namespace, options); - admin.repairTable(namespace, table, buildTransactionTableMetadata(tableMetadata), options); } @Override diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitManager.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitManager.java index e3969083a0..4a9b7c034a 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitManager.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitManager.java @@ -14,6 +14,7 @@ import com.scalar.db.api.Put; import com.scalar.db.api.Result; import com.scalar.db.api.Scan; +import com.scalar.db.api.Selection; import com.scalar.db.api.TransactionCrudOperable; import com.scalar.db.api.TransactionState; import com.scalar.db.api.TwoPhaseCommitTransaction; @@ -401,7 +402,7 @@ public void mutate(List mutations) @Override public List batch(List operations) throws CrudException, UnknownTransactionStatusException { - boolean readOnly = operations.stream().allMatch(o -> o instanceof Get || o instanceof Scan); + boolean readOnly = operations.stream().allMatch(o -> o instanceof Selection); return executeTransaction(t -> t.batch(operations), readOnly); } diff --git a/core/src/main/java/com/scalar/db/transaction/jdbc/JdbcTransactionManager.java b/core/src/main/java/com/scalar/db/transaction/jdbc/JdbcTransactionManager.java index d59be41248..67efbf85a7 100644 --- a/core/src/main/java/com/scalar/db/transaction/jdbc/JdbcTransactionManager.java +++ b/core/src/main/java/com/scalar/db/transaction/jdbc/JdbcTransactionManager.java @@ -12,6 +12,7 @@ import com.scalar.db.api.Put; import com.scalar.db.api.Result; import com.scalar.db.api.Scan; +import com.scalar.db.api.Selection; import com.scalar.db.api.SerializableStrategy; import com.scalar.db.api.TransactionCrudOperable; import com.scalar.db.api.TransactionState; @@ -381,7 +382,7 @@ public void mutate(List mutations) @Override public List batch(List operations) throws CrudException, UnknownTransactionStatusException { - boolean readOnly = operations.stream().allMatch(o -> o instanceof Get || o instanceof Scan); + boolean readOnly = operations.stream().allMatch(o -> o instanceof Selection); return executeTransaction(t -> t.batch(operations), readOnly); } 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 f7cc7546b1..3c8bf42258 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 @@ -974,19 +974,13 @@ public void unsupportedOperations_ShouldThrowUnsupportedException() { // Act Throwable thrown1 = - catchThrowable( - () -> cassandraAdmin.getImportTableMetadata(namespace, table, Collections.emptyMap())); - Throwable thrown2 = - catchThrowable( - () -> cassandraAdmin.addRawColumnToTable(namespace, table, column, DataType.INT)); - Throwable thrown3 = catchThrowable( () -> cassandraAdmin.importTable( namespace, table, Collections.emptyMap(), Collections.emptyMap())); - Throwable thrown4 = + Throwable thrown2 = catchThrowable(() -> cassandraAdmin.renameTable(namespace, table, "new_table")); - Throwable thrown5 = + Throwable thrown3 = catchThrowable( () -> cassandraAdmin.alterColumnType(namespace, table, column, DataType.INT)); @@ -994,8 +988,6 @@ public void unsupportedOperations_ShouldThrowUnsupportedException() { assertThat(thrown1).isInstanceOf(UnsupportedOperationException.class); assertThat(thrown2).isInstanceOf(UnsupportedOperationException.class); assertThat(thrown3).isInstanceOf(UnsupportedOperationException.class); - assertThat(thrown4).isInstanceOf(UnsupportedOperationException.class); - assertThat(thrown5).isInstanceOf(UnsupportedOperationException.class); } @Test 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 2aa946a67a..9ff6494507 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 @@ -1182,20 +1182,15 @@ public void unsupportedOperations_ShouldThrowUnsupportedException() { // Act Throwable thrown1 = - catchThrowable( - () -> admin.getImportTableMetadata(namespace, table, Collections.emptyMap())); - Throwable thrown2 = - catchThrowable(() -> admin.addRawColumnToTable(namespace, table, column, DataType.INT)); - Throwable thrown3 = catchThrowable( () -> admin.importTable( namespace, table, Collections.emptyMap(), Collections.emptyMap())); - Throwable thrown4 = catchThrowable(() -> admin.dropColumnFromTable(namespace, table, column)); - Throwable thrown5 = + Throwable thrown2 = catchThrowable(() -> admin.dropColumnFromTable(namespace, table, column)); + Throwable thrown3 = catchThrowable(() -> admin.renameColumn(namespace, table, column, "newCol")); - Throwable thrown6 = catchThrowable(() -> admin.renameTable(namespace, table, "newTable")); - Throwable thrown7 = + Throwable thrown4 = catchThrowable(() -> admin.renameTable(namespace, table, "newTable")); + Throwable thrown5 = catchThrowable(() -> admin.alterColumnType(namespace, table, column, DataType.INT)); // Assert @@ -1204,8 +1199,6 @@ public void unsupportedOperations_ShouldThrowUnsupportedException() { assertThat(thrown3).isInstanceOf(UnsupportedOperationException.class); assertThat(thrown4).isInstanceOf(UnsupportedOperationException.class); assertThat(thrown5).isInstanceOf(UnsupportedOperationException.class); - assertThat(thrown6).isInstanceOf(UnsupportedOperationException.class); - assertThat(thrown7).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 d86a4d9262..e6ac9dcfbd 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 @@ -1708,19 +1708,14 @@ public void getNamespacesNames_WithNonExistingNamespacesTable_ShouldReturnEmptyS public void unsupportedOperations_ShouldThrowUnsupportedException() { // Arrange Act Throwable thrown1 = - catchThrowable( - () -> admin.getImportTableMetadata(NAMESPACE, TABLE, Collections.emptyMap())); - Throwable thrown2 = - catchThrowable(() -> admin.addRawColumnToTable(NAMESPACE, TABLE, "c1", DataType.INT)); - Throwable thrown3 = catchThrowable( () -> admin.importTable( NAMESPACE, TABLE, Collections.emptyMap(), Collections.emptyMap())); - Throwable thrown4 = catchThrowable(() -> admin.dropColumnFromTable(NAMESPACE, TABLE, "c1")); - Throwable thrown5 = catchThrowable(() -> admin.renameColumn(NAMESPACE, TABLE, "c1", "c2")); - Throwable thrown6 = catchThrowable(() -> admin.renameTable(NAMESPACE, TABLE, "new_table")); - Throwable thrown7 = + Throwable thrown2 = catchThrowable(() -> admin.dropColumnFromTable(NAMESPACE, TABLE, "c1")); + Throwable thrown3 = catchThrowable(() -> admin.renameColumn(NAMESPACE, TABLE, "c1", "c2")); + Throwable thrown4 = catchThrowable(() -> admin.renameTable(NAMESPACE, TABLE, "new_table")); + Throwable thrown5 = catchThrowable(() -> admin.alterColumnType(NAMESPACE, TABLE, "c1", DataType.INT)); // Assert @@ -1729,8 +1724,6 @@ public void unsupportedOperations_ShouldThrowUnsupportedException() { assertThat(thrown3).isInstanceOf(UnsupportedOperationException.class); assertThat(thrown4).isInstanceOf(UnsupportedOperationException.class); assertThat(thrown5).isInstanceOf(UnsupportedOperationException.class); - assertThat(thrown6).isInstanceOf(UnsupportedOperationException.class); - assertThat(thrown7).isInstanceOf(UnsupportedOperationException.class); } @Test diff --git a/core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java b/core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java index 284947e7f9..c17d2b02cb 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 @@ -3727,20 +3727,16 @@ private void getNamespaceNames_forX_ShouldReturnNamespaceNames( } @ParameterizedTest - @EnumSource(RdbEngine.class) - public void getImportTableMetadata_ForX_ShouldWorkProperly(RdbEngine rdbEngine) + @EnumSource( + value = RdbEngine.class, + mode = Mode.EXCLUDE, + names = { + "SQLITE", + }) + public void getImportTableMetadata_ForXBesidesSqlite_ShouldWorkProperly(RdbEngine rdbEngine) throws SQLException, ExecutionException { - if (rdbEngine.equals(RdbEngine.SQLITE)) { - getImportTableMetadata_ForSQLite_ShouldThrowUnsupportedOperationException(rdbEngine); - } else { - getImportTableMetadata_ForOtherThanSQLite_ShouldWorkProperly( - rdbEngine, prepareSqlForTableCheck(rdbEngine, NAMESPACE, TABLE)); - } - } + String expectedCheckTableExistStatement = prepareSqlForTableCheck(rdbEngine, NAMESPACE, TABLE); - private void getImportTableMetadata_ForOtherThanSQLite_ShouldWorkProperly( - RdbEngine rdbEngine, String expectedCheckTableExistStatement) - throws SQLException, ExecutionException { // Arrange Statement checkTableExistStatement = mock(Statement.class); DatabaseMetaData metadata = mock(DatabaseMetaData.class); @@ -3832,16 +3828,6 @@ public Boolean answer(InvocationOnMock invocation) { eq(DataType.FLOAT)); } - private void getImportTableMetadata_ForSQLite_ShouldThrowUnsupportedOperationException( - RdbEngine rdbEngine) { - // Arrange - JdbcAdmin admin = createJdbcAdminFor(rdbEngine); - - // Act Assert - assertThatThrownBy(() -> admin.getImportTableMetadata(NAMESPACE, TABLE, Collections.emptyMap())) - .isInstanceOf(UnsupportedOperationException.class); - } - @Test public void getImportTableMetadata_PrimaryKeyNotExistsForX_ShouldThrowExecutionException() throws SQLException { @@ -3969,76 +3955,6 @@ private void getImportTableMetadata_UnsupportedDataTypeGivenForX_ShouldThrowExec assertThat(thrown).as(description).isInstanceOf(IllegalArgumentException.class); } - @Test - public void addRawColumnToTable_ForX_ShouldWorkProperly() - throws SQLException, ExecutionException { - for (RdbEngine rdbEngine : RDB_ENGINES.keySet()) { - addRawColumnToTable_ForX_ShouldWorkProperly( - rdbEngine, - prepareSqlForTableCheck(rdbEngine, NAMESPACE, TABLE), - prepareSqlForAlterTableAddColumn(rdbEngine, COLUMN_1)); - } - } - - private void addRawColumnToTable_ForX_ShouldWorkProperly( - RdbEngine rdbEngine, String... expectedSqlStatements) - throws SQLException, ExecutionException { - // Arrange - 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.addRawColumnToTable(NAMESPACE, TABLE, COLUMN_1, DataType.INT); - - // Assert - for (int i = 0; i < expectedSqlStatements.length; i++) { - verify( - expectedStatements.get(i), - description("database engine specific test failed: " + rdbEngine)) - .execute(expectedSqlStatements[i]); - } - } - - @Test - public void addRawColumnToTable_WithNonExistingTableForX_ShouldThrowIllegalArgumentException() - throws SQLException { - for (RdbEngine rdbEngine : RDB_ENGINES.keySet()) { - addRawColumnToTable_WithNonExistingTableForX_ShouldThrowIllegalArgumentException( - rdbEngine, prepareSqlForTableCheck(rdbEngine, NAMESPACE, TABLE)); - } - } - - private void addRawColumnToTable_WithNonExistingTableForX_ShouldThrowIllegalArgumentException( - RdbEngine rdbEngine, String expectedCheckTableExistStatement) throws SQLException { - // Arrange - Statement checkTableExistStatement = mock(Statement.class); - when(connection.createStatement()).thenReturn(checkTableExistStatement); - when(dataSource.getConnection()).thenReturn(connection); - - JdbcAdmin admin = createJdbcAdminFor(rdbEngine); - SQLException sqlException = mock(SQLException.class); - mockUndefinedTableError(rdbEngine, sqlException); - when(checkTableExistStatement.execute(any())).thenThrow(sqlException); - - // Act Assert - assertThatThrownBy(() -> admin.addRawColumnToTable(NAMESPACE, TABLE, COLUMN_1, DataType.INT)) - .isInstanceOf(IllegalArgumentException.class); - verify( - checkTableExistStatement, - description("database engine specific test failed: " + rdbEngine)) - .execute(expectedCheckTableExistStatement); - } - @ParameterizedTest @EnumSource( value = RdbEngine.class, diff --git a/core/src/test/java/com/scalar/db/storage/jdbc/RdbEngineSqliteTest.java b/core/src/test/java/com/scalar/db/storage/jdbc/RdbEngineSqliteTest.java index 516eb6d89d..6b99bb1f98 100644 --- a/core/src/test/java/com/scalar/db/storage/jdbc/RdbEngineSqliteTest.java +++ b/core/src/test/java/com/scalar/db/storage/jdbc/RdbEngineSqliteTest.java @@ -1,5 +1,7 @@ package com.scalar.db.storage.jdbc; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.AssertionsForClassTypes.catchThrowable; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -121,12 +123,26 @@ void isConflict_False() { } @Test - void isValidNamespaceOrTableName_True() { - assertTrue(rdbEngine.isValidNamespaceOrTableName("a_b")); + void throwIfInvalidNamespaceName_ShouldNotThrowAnyException() { + assertThatCode(() -> rdbEngine.throwIfInvalidNamespaceName("a-b")).doesNotThrowAnyException(); } @Test - void isValidNamespaceOrTableName_False_WhenContainsNamespaceSeparator() { - assertFalse(rdbEngine.isValidNamespaceOrTableName("a$b")); + void + throwIfInvalidNamespaceName_WhenContainsNamespaceSeparator_ShouldThrowIllegalArgumentException() { + assertThatThrownBy(() -> rdbEngine.throwIfInvalidNamespaceName("a$b")) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void throwIfInvalidTableName_ShouldNotThrowAnyException() { + assertThatCode(() -> rdbEngine.throwIfInvalidTableName("a-b")).doesNotThrowAnyException(); + } + + @Test + void + throwIfInvalidTableName_WhenContainsNamespaceSeparator_ShouldThrowIllegalArgumentException() { + assertThatThrownBy(() -> rdbEngine.throwIfInvalidTableName("a$b")) + .isInstanceOf(IllegalArgumentException.class); } } 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 681d91f730..8b4796f6f4 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 @@ -896,21 +896,6 @@ public void importTable_ForTable1InNamespace1_ShouldCallAdmin1() throws Executio verify(admin1).importTable(namespace, table, options, overrideColumnsType); } - @Test - public void getImportTableMetadata_ForTable1InNamespace1_ShouldCallAdmin1() - throws ExecutionException { - // Arrange - String namespace = NAMESPACE1; - String table = TABLE1; - Map overrideColumnsType = ImmutableMap.of("c", DataType.TIMESTAMPTZ); - - // Act - multiStorageAdmin.getImportTableMetadata(namespace, table, overrideColumnsType); - - // Assert - verify(admin1).getImportTableMetadata(namespace, table, overrideColumnsType); - } - @Test public void getStorageInfo_ShouldReturnProperStorageInfo() throws ExecutionException { // Arrange 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 6f0fbe9d2c..9c09dca672 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 @@ -1,11 +1,10 @@ package com.scalar.db.transaction.consensuscommit; -import static com.scalar.db.transaction.consensuscommit.ConsensusCommitUtils.buildTransactionTableMetadata; import static com.scalar.db.transaction.consensuscommit.ConsensusCommitUtils.getBeforeImageColumnName; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.assertj.core.api.Assertions.catchThrowable; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.never; @@ -704,87 +703,30 @@ public void importTable_ShouldCallStorageAdminProperly() throws ExecutionExcepti .addColumn(column, DataType.INT) .addPartitionKey(primaryKeyColumn) .build(); - when(distributedStorageAdmin.getTableMetadata(NAMESPACE, TABLE)).thenReturn(null); - when(distributedStorageAdmin.getImportTableMetadata(NAMESPACE, TABLE, overrideColumnsType)) - .thenReturn(metadata); doNothing() .when(distributedStorageAdmin) - .addRawColumnToTable(anyString(), anyString(), anyString(), any(DataType.class)); + .importTable(anyString(), anyString(), anyMap(), anyMap()); + when(distributedStorageAdmin.getTableMetadata(NAMESPACE, TABLE)).thenReturn(metadata); + doNothing() + .when(distributedStorageAdmin) + .addNewColumnToTable(anyString(), anyString(), anyString(), any(DataType.class)); // Act admin.importTable(NAMESPACE, TABLE, options, overrideColumnsType); // Assert - verify(distributedStorageAdmin).getTableMetadata(NAMESPACE, TABLE); - verify(distributedStorageAdmin).getImportTableMetadata(NAMESPACE, TABLE, overrideColumnsType); + verify(distributedStorageAdmin).importTable(NAMESPACE, TABLE, options, overrideColumnsType); for (Entry entry : ConsensusCommitUtils.getTransactionMetaColumns().entrySet()) { verify(distributedStorageAdmin) - .addRawColumnToTable(NAMESPACE, TABLE, entry.getKey(), entry.getValue()); + .addNewColumnToTable(NAMESPACE, TABLE, entry.getKey(), entry.getValue()); } + verify(distributedStorageAdmin).getTableMetadata(NAMESPACE, TABLE); verify(distributedStorageAdmin) - .addRawColumnToTable( + .addNewColumnToTable( NAMESPACE, TABLE, getBeforeImageColumnName(column, metadata), DataType.INT); verify(distributedStorageAdmin, never()) - .addRawColumnToTable(NAMESPACE, TABLE, primaryKeyColumn, DataType.INT); - verify(distributedStorageAdmin).repairNamespace(NAMESPACE, options); - verify(distributedStorageAdmin) - .repairTable(NAMESPACE, TABLE, buildTransactionTableMetadata(metadata), options); - } - - @Test - public void importTable_WithStorageTableAlreadyExists_ShouldThrowIllegalArgumentException() - throws ExecutionException { - // Arrange - String primaryKeyColumn = "pk"; - String column = "col"; - TableMetadata metadata = - TableMetadata.newBuilder() - .addColumn(primaryKeyColumn, DataType.INT) - .addColumn(column, DataType.INT) - .addPartitionKey(primaryKeyColumn) - .build(); - when(distributedStorageAdmin.getTableMetadata(NAMESPACE, TABLE)).thenReturn(metadata); - - // Act - Throwable thrown = - catchThrowable(() -> admin.importTable(NAMESPACE, TABLE, Collections.emptyMap())); - - // Assert - assertThat(thrown).isInstanceOf(IllegalArgumentException.class); - } - - @Test - public void importTable_WithTransactionTableAlreadyExists_ShouldThrowIllegalArgumentException() - throws ExecutionException { - // Arrange - String primaryKeyColumn = "pk"; - String column = "col"; - TableMetadata metadata = - TableMetadata.newBuilder() - .addColumn(primaryKeyColumn, DataType.INT) - .addColumn(column, DataType.INT) - .addColumn(Attribute.ID, DataType.TEXT) - .addColumn(Attribute.STATE, DataType.INT) - .addColumn(Attribute.VERSION, DataType.INT) - .addColumn(Attribute.PREPARED_AT, DataType.BIGINT) - .addColumn(Attribute.COMMITTED_AT, DataType.BIGINT) - .addColumn(Attribute.BEFORE_PREFIX + column, DataType.INT) - .addColumn(Attribute.BEFORE_ID, DataType.TEXT) - .addColumn(Attribute.BEFORE_STATE, DataType.INT) - .addColumn(Attribute.BEFORE_VERSION, DataType.INT) - .addColumn(Attribute.BEFORE_PREPARED_AT, DataType.BIGINT) - .addColumn(Attribute.BEFORE_COMMITTED_AT, DataType.BIGINT) - .addPartitionKey(primaryKeyColumn) - .build(); - when(distributedStorageAdmin.getTableMetadata(NAMESPACE, TABLE)).thenReturn(metadata); - - // Act - Throwable thrown = - catchThrowable(() -> admin.importTable(NAMESPACE, TABLE, Collections.emptyMap())); - - // Assert - assertThat(thrown).isInstanceOf(IllegalArgumentException.class); + .addNewColumnToTable(NAMESPACE, TABLE, primaryKeyColumn, DataType.INT); } @Test 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 c2b145f4c2..c12bd2d40c 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 @@ -114,33 +114,6 @@ public void afterEach() { sleepBetweenTests(); } - @Test - public void getImportTableMetadata_WithSufficientPermission_ShouldSucceed() - throws ExecutionException { - // Arrange - createNamespaceByRoot(); - createTableByRoot(); - - // Act Assert - assertThatCode(() -> adminForNormalUser.getImportTableMetadata(NAMESPACE, TABLE)) - .doesNotThrowAnyException(); - } - - @Test - public void addRawColumnToTable_WithSufficientPermission_ShouldSucceed() - throws ExecutionException { - // Arrange - createNamespaceByRoot(); - createTableByRoot(); - - // Act Assert - assertThatCode( - () -> - adminForNormalUser.addRawColumnToTable( - NAMESPACE, TABLE, RAW_COL_NAME, DataType.INT)) - .doesNotThrowAnyException(); - } - @Test public void createNamespace_WithSufficientPermission_ShouldSucceed() { // Arrange