Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add options to Admin.importTable() #1337

Merged
merged 1 commit into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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