Skip to content

Commit

Permalink
[#1635] Improvement: Replace ColumnDTO with ColumnImpl in test cases (#…
Browse files Browse the repository at this point in the history 
…2487)

### What changes were proposed in this pull request?
- Use `ColumnImpl` instead of `ColumnDTO` when calling gravitino API in
test cases.
- Rename variable columnDTOs to columns to avoid misunderstanding.

### Why are the changes needed?

Fix: #1635

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Existing integration tests.
  • Loading branch information
@zivali
zivali authored Mar 11, 2024
1 parent 0730534 commit c74841c
Show file tree
Hide file tree
Showing 9 changed files with 139 additions and 323 deletions.
41 changes: 27 additions & 14 deletions clients/client-java/src/test/java/com/datastrato/gravitino/client/TestRelationalCatalog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package com.datastrato.gravitino.client;

import static com.datastrato.gravitino.dto.rel.partitioning.Partitioning.EMPTY_PARTITIONING;
import static com.datastrato.gravitino.dto.util.DTOConverters.fromDTOs;
import static com.datastrato.gravitino.rel.expressions.sorts.SortDirection.DESCENDING;
import static org.apache.hc.core5.http.HttpStatus.SC_BAD_REQUEST;
import static org.apache.hc.core5.http.HttpStatus.SC_CONFLICT;
Expand Down Expand Up @@ -50,6 +51,7 @@
import com.datastrato.gravitino.exceptions.RESTException;
import com.datastrato.gravitino.exceptions.SchemaAlreadyExistsException;
import com.datastrato.gravitino.exceptions.TableAlreadyExistsException;
import com.datastrato.gravitino.rel.Column;
import com.datastrato.gravitino.rel.Schema;
import com.datastrato.gravitino.rel.SupportsSchemas;
import com.datastrato.gravitino.rel.Table;
Expand Down Expand Up @@ -372,7 +374,8 @@ public void testCreateTable() throws JsonProcessingException {
Table table =
catalog
.asTableCatalog()
.createTable(tableId, columns, "comment", Collections.emptyMap(), sortOrderDTOs);
.createTable(
tableId, fromDTOs(columns), "comment", Collections.emptyMap(), sortOrderDTOs);
Assertions.assertEquals(expectedTable.name(), table.name());
Assertions.assertEquals(expectedTable.comment(), table.comment());
Assertions.assertEquals(expectedTable.properties(), table.properties());
Expand All @@ -388,14 +391,15 @@ public void testCreateTable() throws JsonProcessingException {
assertTableEquals(expectedTable, table);

// test validate column default value
ColumnDTO[] errorColumns =
new ColumnDTO[] {
createMockColumn("col1", Types.ByteType.get(), "comment1"),
createMockColumn(
Column[] errorColumns =
new Column[] {
Column.of("col1", Types.ByteType.get(), "comment1"),
Column.of(
"col2",
Types.StringType.get(),
"comment2",
false,
false,
new LiteralDTO.Builder().withValue(null).withDataType(Types.NullType.get()).build())
};

Expand All @@ -420,7 +424,9 @@ public void testCreateTable() throws JsonProcessingException {
Throwable ex =
Assertions.assertThrows(
NoSuchSchemaException.class,
() -> tableCatalog.createTable(tableId, columns, "comment", emptyMap, sortOrder));
() ->
tableCatalog.createTable(
tableId, fromDTOs(columns), "comment", emptyMap, sortOrder));
Assertions.assertTrue(ex.getMessage().contains("schema not found"));

// Test throw TableAlreadyExistsException
Expand All @@ -432,7 +438,9 @@ public void testCreateTable() throws JsonProcessingException {
Throwable ex1 =
Assertions.assertThrows(
TableAlreadyExistsException.class,
() -> tableCatalog.createTable(tableId, columns, "comment", emptyMap, sortOrder));
() ->
tableCatalog.createTable(
tableId, fromDTOs(columns), "comment", emptyMap, sortOrder));
Assertions.assertTrue(ex1.getMessage().contains("table already exists"));
}

Expand Down Expand Up @@ -474,7 +482,8 @@ public void testCreatePartitionedTable() throws JsonProcessingException {
Table table =
catalog
.asTableCatalog()
.createTable(tableId, columns, "comment", Collections.emptyMap(), EMPTY_PARTITIONING);
.createTable(
tableId, fromDTOs(columns), "comment", Collections.emptyMap(), EMPTY_PARTITIONING);
assertTableEquals(expectedTable, table);

// Test partitioning
Expand Down Expand Up @@ -507,7 +516,8 @@ public void testCreatePartitionedTable() throws JsonProcessingException {
table =
catalog
.asTableCatalog()
.createTable(tableId, columns, "comment", Collections.emptyMap(), partitioning);
.createTable(
tableId, fromDTOs(columns), "comment", Collections.emptyMap(), partitioning);
assertTableEquals(expectedTable, table);

// Test throw TableAlreadyExistsException
Expand All @@ -521,7 +531,9 @@ public void testCreatePartitionedTable() throws JsonProcessingException {
Throwable ex1 =
Assertions.assertThrows(
TableAlreadyExistsException.class,
() -> tableCatalog.createTable(tableId, columns, "comment", emptyMap, partitioning));
() ->
tableCatalog.createTable(
tableId, fromDTOs(columns), "comment", emptyMap, partitioning));
Assertions.assertTrue(ex1.getMessage().contains("table already exists"));

// Test partitioning field not exist in table
Expand All @@ -530,7 +542,8 @@ public void testCreatePartitionedTable() throws JsonProcessingException {
Assertions.assertThrows(
IllegalArgumentException.class,
() ->
tableCatalog.createTable(tableId, columns, "comment", emptyMap, errorPartitioning));
tableCatalog.createTable(
tableId, fromDTOs(columns), "comment", emptyMap, errorPartitioning));
Assertions.assertTrue(ex2.getMessage().contains("not found in table"));

// Test empty columns
Expand All @@ -539,7 +552,7 @@ public void testCreatePartitionedTable() throws JsonProcessingException {
IllegalArgumentException.class,
() ->
tableCatalog.createTable(
tableId, new ColumnDTO[0], "comment", emptyMap, errorPartitioning));
tableId, new Column[0], "comment", emptyMap, errorPartitioning));
Assertions.assertTrue(
ex3.getMessage().contains("\"columns\" field is required and cannot be empty"));
}
Expand Down Expand Up @@ -598,7 +611,7 @@ public void testCreateIndexTable() throws JsonProcessingException {
.asTableCatalog()
.createTable(
tableId,
columns,
fromDTOs(columns),
"comment",
Collections.emptyMap(),
EMPTY_PARTITIONING,
Expand All @@ -621,7 +634,7 @@ public void testCreateIndexTable() throws JsonProcessingException {
() ->
tableCatalog.createTable(
tableId,
columns,
fromDTOs(columns),
"comment",
emptyMap,
EMPTY_PARTITIONING,
Expand Down
3 changes: 2 additions & 1 deletion clients/client-java/src/test/java/com/datastrato/gravitino/client/TestRelationalTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/
package com.datastrato.gravitino.client;

import static com.datastrato.gravitino.dto.util.DTOConverters.fromDTOs;
import static com.datastrato.gravitino.dto.util.DTOConverters.toDTO;
import static org.apache.hc.core5.http.HttpStatus.SC_OK;
import static org.apache.http.HttpStatus.SC_CONFLICT;
Expand Down Expand Up @@ -115,7 +116,7 @@ public static void setUp() throws Exception {
.asTableCatalog()
.createTable(
tableId,
columns,
fromDTOs(columns),
"comment",
Collections.emptyMap(),
partitioning,
Expand Down
77 changes: 24 additions & 53 deletions ...t/src/test/java/com/datastrato/gravitino/integration/test/catalog/hive/CatalogHiveIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import com.datastrato.gravitino.catalog.hive.HiveTablePropertiesMetadata;
import com.datastrato.gravitino.catalog.hive.HiveTablePropertiesMetadata.TableType;
import com.datastrato.gravitino.client.GravitinoMetaLake;
import com.datastrato.gravitino.dto.rel.ColumnDTO;
import com.datastrato.gravitino.dto.rel.expressions.FieldReferenceDTO;
import com.datastrato.gravitino.dto.rel.partitioning.IdentityPartitioningDTO;
import com.datastrato.gravitino.dto.rel.partitioning.Partitioning;
Expand All @@ -49,6 +48,7 @@
import com.datastrato.gravitino.integration.test.container.HiveContainer;
import com.datastrato.gravitino.integration.test.util.AbstractIT;
import com.datastrato.gravitino.integration.test.util.GravitinoITUtils;
import com.datastrato.gravitino.rel.Column;
import com.datastrato.gravitino.rel.Schema;
import com.datastrato.gravitino.rel.SchemaChange;
import com.datastrato.gravitino.rel.SupportsSchemas;
Expand Down Expand Up @@ -275,26 +275,11 @@ private static void createSchema() throws TException, InterruptedException {
Assertions.assertEquals("val2", database.getParameters().get("key2"));
}

private ColumnDTO[] createColumns() {
ColumnDTO col1 =
new ColumnDTO.Builder<>()
.withName(HIVE_COL_NAME1)
.withDataType(Types.ByteType.get())
.withComment("col_1_comment")
.build();
ColumnDTO col2 =
new ColumnDTO.Builder<>()
.withName(HIVE_COL_NAME2)
.withDataType(Types.DateType.get())
.withComment("col_2_comment")
.build();
ColumnDTO col3 =
new ColumnDTO.Builder<>()
.withName(HIVE_COL_NAME3)
.withDataType(Types.StringType.get())
.withComment("col_3_comment")
.build();
return new ColumnDTO[] {col1, col2, col3};
private Column[] createColumns() {
Column col1 = Column.of(HIVE_COL_NAME1, Types.ByteType.get(), "col_1_comment");
Column col2 = Column.of(HIVE_COL_NAME2, Types.DateType.get(), "col_2_comment");
Column col3 = Column.of(HIVE_COL_NAME3, Types.StringType.get(), "col_3_comment");
return new Column[] {col1, col2, col3};
}

private void checkTableReadWrite(org.apache.hadoop.hive.metastore.api.Table table) {
Expand Down Expand Up @@ -345,7 +330,7 @@ private Map<String, String> createProperties() {
public void testCreateHiveTableWithDistributionAndSortOrder()
throws TException, InterruptedException {
// Create table from Gravitino API
ColumnDTO[] columns = createColumns();
Column[] columns = createColumns();

NameIdentifier nameIdentifier =
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName);
Expand Down Expand Up @@ -445,7 +430,7 @@ public void testCreateHiveTableWithDistributionAndSortOrder()
@Test
public void testCreateHiveTable() throws TException, InterruptedException {
// Create table from Gravitino API
ColumnDTO[] columns = createColumns();
Column[] columns = createColumns();

NameIdentifier nameIdentifier =
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName);
Expand Down Expand Up @@ -498,7 +483,7 @@ public void testCreateHiveTable() throws TException, InterruptedException {

@Test
public void testHiveTableProperties() throws TException, InterruptedException {
ColumnDTO[] columns = createColumns();
Column[] columns = createColumns();
NameIdentifier nameIdentifier =
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName);
// test default properties
Expand Down Expand Up @@ -616,7 +601,7 @@ public void testHiveSchemaProperties() throws TException, InterruptedException {
@Test
public void testCreatePartitionedHiveTable() throws TException, InterruptedException {
// Create table from Gravitino API
ColumnDTO[] columns = createColumns();
Column[] columns = createColumns();

NameIdentifier nameIdentifier =
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName);
Expand Down Expand Up @@ -667,7 +652,7 @@ public void testCreatePartitionedHiveTable() throws TException, InterruptedExcep
@Test
public void testListPartitionNames() throws TException, InterruptedException {
// test empty partitions
ColumnDTO[] columns = createColumns();
Column[] columns = createColumns();
NameIdentifier nameIdentifier =
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName);
Table nonPartitionedTable =
Expand All @@ -694,7 +679,7 @@ public void testListPartitionNames() throws TException, InterruptedException {
@Test
public void testListPartitions() throws TException, InterruptedException {
// test empty partitions
ColumnDTO[] columns = createColumns();
Column[] columns = createColumns();
NameIdentifier nameIdentifier =
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName);
Table nonPartitionedTable =
Expand Down Expand Up @@ -821,7 +806,7 @@ public void testAddPartition() throws TException, InterruptedException {
}

private Table preparePartitionedTable() throws TException, InterruptedException {
ColumnDTO[] columns = createColumns();
Column[] columns = createColumns();

NameIdentifier nameIdentifier =
NameIdentifier.of(
Expand Down Expand Up @@ -913,7 +898,7 @@ void testAlterUnknownTable() {

@Test
public void testAlterHiveTable() throws TException, InterruptedException {
ColumnDTO[] columns = createColumns();
Column[] columns = createColumns();
Table createdTable =
catalog
.asTableCatalog()
Expand Down Expand Up @@ -983,25 +968,11 @@ public void testAlterHiveTable() throws TException, InterruptedException {
Assertions.assertTrue(exception.getMessage().contains("Cannot alter partition column"));

// test updateColumnPosition exception
ColumnDTO col1 =
new ColumnDTO.Builder()
.withName("name")
.withDataType(Types.StringType.get())
.withComment("comment")
.build();
ColumnDTO col2 =
new ColumnDTO.Builder()
.withName("address")
.withDataType(Types.StringType.get())
.withComment("comment")
.build();
ColumnDTO col3 =
new ColumnDTO.Builder()
.withName("date_of_birth")
.withDataType(Types.DateType.get())
.withComment("comment")
.build();
ColumnDTO[] newColumns = new ColumnDTO[] {col1, col2, col3};
Column col1 = Column.of("name", Types.StringType.get(), "comment");
Column col2 = Column.of("address", Types.StringType.get(), "comment");
Column col3 = Column.of("date_of_birth", Types.DateType.get(), "comment");

Column[] newColumns = new Column[] {col1, col2, col3};
NameIdentifier tableIdentifier =
NameIdentifier.of(
metalakeName,
Expand Down Expand Up @@ -1218,7 +1189,7 @@ void testAlterEntityName() {
// Now try to rename table
final String tableName = GravitinoITUtils.genRandomName("CatalogHiveIT_table");
final String newTableName = GravitinoITUtils.genRandomName("CatalogHiveIT_table_new");
ColumnDTO[] columns = createColumns();
Column[] columns = createColumns();
catalog
.asTableCatalog()
.createTable(
Expand Down Expand Up @@ -1278,7 +1249,7 @@ void testDropAndRename() {

@Test
public void testDropHiveManagedTable() throws TException, InterruptedException, IOException {
ColumnDTO[] columns = createColumns();
Column[] columns = createColumns();
catalog
.asTableCatalog()
.createTable(
Expand All @@ -1303,7 +1274,7 @@ public void testDropHiveManagedTable() throws TException, InterruptedException,

@Test
public void testDropHiveExternalTable() throws TException, InterruptedException, IOException {
ColumnDTO[] columns = createColumns();
Column[] columns = createColumns();
catalog
.asTableCatalog()
.createTable(
Expand All @@ -1330,7 +1301,7 @@ public void testDropHiveExternalTable() throws TException, InterruptedException,

@Test
public void testPurgeHiveManagedTable() throws TException, InterruptedException, IOException {
ColumnDTO[] columns = createColumns();
Column[] columns = createColumns();
catalog
.asTableCatalog()
.createTable(
Expand All @@ -1357,7 +1328,7 @@ public void testPurgeHiveManagedTable() throws TException, InterruptedException,

@Test
public void testPurgeHiveExternalTable() throws TException, InterruptedException, IOException {
ColumnDTO[] columns = createColumns();
Column[] columns = createColumns();
catalog
.asTableCatalog()
.createTable(
Expand Down
Loading

0 comments on commit c74841c

Please sign in to comment.