Skip to content

Commit

Permalink
Add options to Admin.importTable() (#1337)
Browse files Browse the repository at this point in the history
  • Loading branch information
brfrn169 authored Nov 30, 2023
1 parent fa06d9c commit 51274f3
Show file tree
Hide file tree
Showing 33 changed files with 306 additions and 134 deletions.
4 changes: 3 additions & 1 deletion core/src/main/java/com/scalar/db/api/Admin.java
Original file line number Diff line number Diff line change
Expand Up @@ -408,9 +408,11 @@ void addNewColumnToTable(String namespace, String table, String columnName, Data
*
* @param namespace an existing namespace
* @param table an existing table
* @param options options to import
* @throws IllegalArgumentException if the table is already managed by ScalarDB, if the target
* table does not exist, or if the table does not meet the requirement of ScalarDB table
* @throws ExecutionException if the operation fails
*/
void importTable(String namespace, String table) throws ExecutionException;
void importTable(String namespace, String table, Map<String, String> options)
throws ExecutionException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -290,15 +290,16 @@ public TableMetadata getImportTableMetadata(String namespace, String table)
}

@Override
public void importTable(String namespace, String table) throws ExecutionException {
public void importTable(String namespace, String table, Map<String, String> options)
throws ExecutionException {
TableMetadata tableMetadata = getTableMetadata(namespace, table);
if (tableMetadata != null) {
throw new IllegalArgumentException(
"Table already exists: " + ScalarDbUtils.getFullTableName(namespace, table));
}

try {
admin.importTable(namespace, table);
admin.importTable(namespace, table, options);
} catch (ExecutionException e) {
throw new ExecutionException(
"Importing the table failed: " + ScalarDbUtils.getFullTableName(namespace, table), e);
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/com/scalar/db/service/AdminService.java
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,9 @@ public void addRawColumnToTable(
}

@Override
public void importTable(String namespace, String table) throws ExecutionException {
admin.importTable(namespace, table);
public void importTable(String namespace, String table, Map<String, String> options)
throws ExecutionException {
admin.importTable(namespace, table, options);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ public void addRawColumnToTable(
}

@Override
public void importTable(String namespace, String table) {
public void importTable(String namespace, String table, Map<String, String> options) {
throw new UnsupportedOperationException(
"Import-related functionality is not supported in Cassandra");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ public void addRawColumnToTable(
}

@Override
public void importTable(String namespace, String table) {
public void importTable(String namespace, String table, Map<String, String> options) {
throw new UnsupportedOperationException(
"Import-related functionality is not supported in Cosmos DB");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1214,7 +1214,7 @@ public void addRawColumnToTable(
}

@Override
public void importTable(String namespace, String table) {
public void importTable(String namespace, String table, Map<String, String> options) {
throw new UnsupportedOperationException(
"Import-related functionality is not supported in DynamoDB");
}
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import com.google.inject.Inject;
Expand Down Expand Up @@ -490,11 +489,12 @@ public TableMetadata getImportTableMetadata(String namespace, String table)
}

@Override
public void importTable(String namespace, String table) throws ExecutionException {
public void importTable(String namespace, String table, Map<String, String> options)
throws ExecutionException {
TableMetadata tableMetadata = getImportTableMetadata(namespace, table);

// add ScalarDB metadata
repairTable(namespace, table, tableMetadata, ImmutableMap.of());
repairTable(namespace, table, tableMetadata, options);
}

private String getSelectColumnsStatement() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,9 @@ public void addRawColumnToTable(
}

@Override
public void importTable(String namespace, String table) throws ExecutionException {
getAdmin(namespace, table).importTable(namespace, table);
public void importTable(String namespace, String table, Map<String, String> options)
throws ExecutionException {
getAdmin(namespace, table).importTable(namespace, table, options);
}

private DistributedStorageAdmin getAdmin(String namespace) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ public void addRawColumnToTable(
}

@Override
public void importTable(String namespace, String table) {
public void importTable(String namespace, String table, Map<String, String> options) {
throw new UnsupportedOperationException(
"Import-related functionality is not supported in ScalarDB Server");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import static com.scalar.db.transaction.consensuscommit.ConsensusCommitUtils.removeTransactionMetaColumns;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableMap;
import com.google.inject.Inject;
import com.scalar.db.api.DistributedStorageAdmin;
import com.scalar.db.api.DistributedTransactionAdmin;
Expand Down Expand Up @@ -193,7 +192,8 @@ public void addNewColumnToTable(
}

@Override
public void importTable(String namespace, String table) throws ExecutionException {
public void importTable(String namespace, String table, Map<String, String> options)
throws ExecutionException {
TableMetadata tableMetadata = admin.getTableMetadata(namespace, table);
if (tableMetadata != null) {
throw new IllegalArgumentException(
Expand All @@ -216,8 +216,7 @@ public void importTable(String namespace, String table) throws ExecutionExceptio
}

// add ScalarDB metadata
admin.repairTable(
namespace, table, buildTransactionTableMetadata(tableMetadata), ImmutableMap.of());
admin.repairTable(namespace, table, buildTransactionTableMetadata(tableMetadata), options);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,9 @@ public boolean namespaceExists(String namespace) throws ExecutionException {
}

@Override
public void importTable(String namespace, String table) throws ExecutionException {
jdbcAdmin.importTable(namespace, table);
public void importTable(String namespace, String table, Map<String, String> options)
throws ExecutionException {
jdbcAdmin.importTable(namespace, table, options);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ public void addNewColumnToTable(
}

@Override
public void importTable(String namespace, String table) {
public void importTable(String namespace, String table, Map<String, String> options) {
throw new UnsupportedOperationException(
"Import-related functionality is not supported in ScalarDB Server");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,8 @@ public void unsupportedOperations_ShouldThrowUnsupportedException() {
Throwable thrown2 =
catchThrowable(
() -> cassandraAdmin.addRawColumnToTable(namespace, table, column, DataType.INT));
Throwable thrown3 = catchThrowable(() -> cassandraAdmin.importTable(namespace, table));
Throwable thrown3 =
catchThrowable(() -> cassandraAdmin.importTable(namespace, table, Collections.emptyMap()));

// Assert
assertThat(thrown1).isInstanceOf(UnsupportedOperationException.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,8 @@ public void unsupportedOperations_ShouldThrowUnsupportedException() {
Throwable thrown1 = catchThrowable(() -> admin.getImportTableMetadata(namespace, table));
Throwable thrown2 =
catchThrowable(() -> admin.addRawColumnToTable(namespace, table, column, DataType.INT));
Throwable thrown3 = catchThrowable(() -> admin.importTable(namespace, table));
Throwable thrown3 =
catchThrowable(() -> admin.importTable(namespace, table, Collections.emptyMap()));

// Assert
assertThat(thrown1).isInstanceOf(UnsupportedOperationException.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1183,7 +1183,8 @@ public void unsupportedOperations_ShouldThrowUnsupportedException() {
Throwable thrown1 = catchThrowable(() -> admin.getImportTableMetadata(NAMESPACE, TABLE));
Throwable thrown2 =
catchThrowable(() -> admin.addRawColumnToTable(NAMESPACE, TABLE, "c1", DataType.INT));
Throwable thrown3 = catchThrowable(() -> admin.importTable(NAMESPACE, TABLE));
Throwable thrown3 =
catchThrowable(() -> admin.importTable(NAMESPACE, TABLE, Collections.emptyMap()));

// Assert
assertThat(thrown1).isInstanceOf(UnsupportedOperationException.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2519,7 +2519,7 @@ private void importTable_ForX_ShouldWorkProperly(
JdbcAdmin admin = createJdbcAdminFor(rdbEngine);

// Act
admin.importTable(NAMESPACE, TABLE);
admin.importTable(NAMESPACE, TABLE, Collections.emptyMap());

// Assert
for (int i = 0; i < expectedSqlStatements.size(); i++) {
Expand All @@ -2536,7 +2536,8 @@ public void importTable_ForSQLite_ShouldThrowUnsupportedOperationException() {
JdbcAdmin admin = createJdbcAdminFor(RdbEngine.SQLITE);

// Act
Throwable thrown = catchThrowable(() -> admin.importTable(NAMESPACE, TABLE));
Throwable thrown =
catchThrowable(() -> admin.importTable(NAMESPACE, TABLE, Collections.emptyMap()));

// Assert
assertThat(thrown).isInstanceOf(UnsupportedOperationException.class);
Expand Down
19 changes: 8 additions & 11 deletions core/src/test/java/com/scalar/db/storage/rpc/GrpcAdminTest.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.scalar.db.storage.rpc;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.catchThrowable;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -340,15 +340,12 @@ public void unsupportedOperations_ShouldThrowUnsupportedException() {
String table = "tbl";
String column = "col";

// Act
Throwable thrown1 = catchThrowable(() -> admin.getImportTableMetadata(namespace, table));
Throwable thrown2 =
catchThrowable(() -> admin.addRawColumnToTable(namespace, table, column, DataType.INT));
Throwable thrown3 = catchThrowable(() -> admin.importTable(namespace, table));

// Assert
assertThat(thrown1).isInstanceOf(UnsupportedOperationException.class);
assertThat(thrown2).isInstanceOf(UnsupportedOperationException.class);
assertThat(thrown3).isInstanceOf(UnsupportedOperationException.class);
// Act Assert
assertThatThrownBy(() -> admin.getImportTableMetadata(namespace, table))
.isInstanceOf(UnsupportedOperationException.class);
assertThatThrownBy(() -> admin.addRawColumnToTable(namespace, table, column, DataType.INT))
.isInstanceOf(UnsupportedOperationException.class);
assertThatThrownBy(() -> admin.importTable(namespace, table, Collections.emptyMap()))
.isInstanceOf(UnsupportedOperationException.class);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
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;
Expand Down Expand Up @@ -594,6 +595,7 @@ public void addNewColumnToTable_ShouldCallJdbcAdminProperly() throws ExecutionEx
@Test
public void importTable_ShouldCallStorageAdminProperly() throws ExecutionException {
// Arrange
Map<String, String> options = ImmutableMap.of("foo", "bar");
String primaryKeyColumn = "pk";
String column = "col";
TableMetadata metadata =
Expand All @@ -609,7 +611,7 @@ public void importTable_ShouldCallStorageAdminProperly() throws ExecutionExcepti
.addRawColumnToTable(anyString(), anyString(), anyString(), any(DataType.class));

// Act
admin.importTable(NAMESPACE, TABLE);
admin.importTable(NAMESPACE, TABLE, options);

// Assert
verify(distributedStorageAdmin).getTableMetadata(NAMESPACE, TABLE);
Expand All @@ -624,6 +626,8 @@ public void importTable_ShouldCallStorageAdminProperly() throws ExecutionExcepti
NAMESPACE, TABLE, getBeforeImageColumnName(column, metadata), DataType.INT);
verify(distributedStorageAdmin, never())
.addRawColumnToTable(NAMESPACE, TABLE, primaryKeyColumn, DataType.INT);
verify(distributedStorageAdmin)
.repairTable(NAMESPACE, TABLE, buildTransactionTableMetadata(metadata), options);
}

@Test
Expand All @@ -641,7 +645,8 @@ public void importTable_WithStorageTableAlreadyExists_ShouldThrowIllegalArgument
when(distributedStorageAdmin.getTableMetadata(NAMESPACE, TABLE)).thenReturn(metadata);

// Act
Throwable thrown = catchThrowable(() -> admin.importTable(NAMESPACE, TABLE));
Throwable thrown =
catchThrowable(() -> admin.importTable(NAMESPACE, TABLE, Collections.emptyMap()));

// Assert
assertThat(thrown).isInstanceOf(IllegalArgumentException.class);
Expand Down Expand Up @@ -673,7 +678,8 @@ public void importTable_WithTransactionTableAlreadyExists_ShouldThrowIllegalArgu
when(distributedStorageAdmin.getTableMetadata(NAMESPACE, TABLE)).thenReturn(metadata);

// Act
Throwable thrown = catchThrowable(() -> admin.importTable(NAMESPACE, TABLE));
Throwable thrown =
catchThrowable(() -> admin.importTable(NAMESPACE, TABLE, Collections.emptyMap()));

// Assert
assertThat(thrown).isInstanceOf(IllegalArgumentException.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,9 @@ public void importTable_ShouldCallJdbcAdminProperly() throws ExecutionException
String table = "tbl";

// Act
admin.importTable(namespace, table);
admin.importTable(namespace, table, Collections.emptyMap());

// Assert
verify(jdbcAdmin).importTable(namespace, table);
verify(jdbcAdmin).importTable(namespace, table, Collections.emptyMap());
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.scalar.db.transaction.rpc;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -411,4 +412,15 @@ public void addNewColumnToTable_CalledWithProperArguments_StubShouldBeCalledProp
.setColumnType(com.scalar.db.rpc.DataType.DATA_TYPE_TEXT)
.build());
}

@Test
public void unsupportedOperations_ShouldThrowUnsupportedException() {
// Arrange
String namespace = "sample_ns";
String table = "tbl";

// Act Assert
assertThatThrownBy(() -> admin.importTable(namespace, table, Collections.emptyMap()))
.isInstanceOf(UnsupportedOperationException.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,15 @@ public void importTable_ForUnsupportedDatabase_ShouldThrowUnsupportedOperationEx
admin.createNamespace(getNamespace(), getCreationOptions());

// Act Assert
assertThatThrownBy(() -> admin.importTable(getNamespace(), "unsupported_db"))
assertThatThrownBy(
() -> admin.importTable(getNamespace(), "unsupported_db", Collections.emptyMap()))
.isInstanceOf(UnsupportedOperationException.class);
}

private void importTable_ForImportableTable_ShouldImportProperly(
String table, TableMetadata metadata) throws ExecutionException {
// Act
admin.importTable(getNamespace(), table);
admin.importTable(getNamespace(), table, Collections.emptyMap());

// Assert
assertThat(admin.tableExists(getNamespace(), table)).isTrue();
Expand All @@ -119,14 +120,15 @@ private void importTable_ForImportableTable_ShouldImportProperly(
private void importTable_ForNonImportableTable_ShouldThrowIllegalArgumentException(String table) {
// Act Assert
assertThatThrownBy(
() -> admin.importTable(getNamespace(), table),
() -> admin.importTable(getNamespace(), table, Collections.emptyMap()),
"non-importable data type test failed: " + table)
.isInstanceOf(IllegalArgumentException.class);
}

private void importTable_ForNonExistingTable_ShouldThrowIllegalArgumentException() {
// Act Assert
assertThatThrownBy(() -> admin.importTable(getNamespace(), "non-existing-table"))
assertThatThrownBy(
() -> admin.importTable(getNamespace(), "non-existing-table", Collections.emptyMap()))
.isInstanceOf(IllegalArgumentException.class);
}
}
Loading

0 comments on commit 51274f3

Please sign in to comment.