Revisit importTable() in Consensus Commit

brfrn169 · brfrn169 · commit 600c743825a7 · 2025-10-31T00:00:17.000+09:00
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
@@ -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
@@ -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
@@ -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
@@ -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<String, DataType> 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
@@ -451,20 +451,6 @@ public Set<String> getNamespaceNames() throws ExecutionException {
     }
   }
 
-  @Override
-  public TableMetadata getImportTableMetadata(
-      String namespace, String table, Map<String, DataType> 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<String, String> options) throws ExecutionException {
     try {
diff --git 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<String, DataType> 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
@@ -285,20 +285,6 @@ private TableMetadata createTableMetadata(com.datastax.driver.core.TableMetadata
     return builder.build();
   }
 
-  @Override
-  public TableMetadata getImportTableMetadata(
-      String namespace, String table, Map<String, DataType> 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
@@ -674,18 +674,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<String, DataType> 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
@@ -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<String, DataType> 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
@@ -565,8 +565,8 @@ public TableMetadata getTableMetadata(String namespace, String table) throws Exe
     return builder.build();
   }
 
-  @Override
-  public TableMetadata getImportTableMetadata(
+  @VisibleForTesting
+  TableMetadata getImportTableMetadata(
       String namespace, String table, Map<String, DataType> overrideColumnsType)
       throws ExecutionException {
     TableMetadata.Builder builder = TableMetadata.newBuilder();
@@ -1036,34 +1036,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/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<String, DataType> 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
@@ -323,31 +323,25 @@ 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<String, DataType> entry :
         ConsensusCommitUtils.getTransactionMetaColumns().entrySet()) {
-      admin.addRawColumnToTable(namespace, table, entry.getKey(), entry.getValue());
+      admin.addNewColumnToTable(namespace, table, entry.getKey(), entry.getValue());
     }
 
     // add before image columns
     Set<String> nonPrimaryKeyColumns = getNonPrimaryKeyColumns(tableMetadata);
     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
@@ -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<? extends Mutation> mutations)
   @Override
   public List<BatchResult> batch(List<? extends Operation> 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
@@ -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<? extends Mutation> mutations)
   @Override
   public List<BatchResult> batch(List<? extends Operation> 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
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
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
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
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
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
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
