Skip to content

Commit

Permalink
[#1293] improvement(IT): Change to use api method instead of DTOs (#1294
Browse files Browse the repository at this point in the history
)

### What changes were proposed in this pull request?

Remove the usage of DTOs of partitioning, sort ordering and bucketing. 

### Why are the changes needed?


We should not use DTOs when using Gravition java client. 

Fix: #1293 

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

N/A

### How was this patch tested?

Existing changes can cover it.
  • Loading branch information
yuqi1129 authored Jan 2, 2024
1 parent be0a8c6 commit c905c3f
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@
import com.datastrato.gravitino.catalog.hive.HiveTablePropertiesMetadata;
import com.datastrato.gravitino.client.GravitinoMetaLake;
import com.datastrato.gravitino.dto.rel.ColumnDTO;
import com.datastrato.gravitino.dto.rel.DistributionDTO;
import com.datastrato.gravitino.dto.rel.SortOrderDTO;
import com.datastrato.gravitino.dto.rel.expressions.FieldReferenceDTO;
import com.datastrato.gravitino.dto.rel.partitions.IdentityPartitioningDTO;
import com.datastrato.gravitino.dto.rel.partitions.Partitioning;
Expand All @@ -53,12 +51,14 @@
import com.datastrato.gravitino.rel.SchemaChange;
import com.datastrato.gravitino.rel.Table;
import com.datastrato.gravitino.rel.TableChange;
import com.datastrato.gravitino.rel.expressions.NamedReference;
import com.datastrato.gravitino.rel.expressions.distributions.Distribution;
import com.datastrato.gravitino.rel.expressions.distributions.Distributions;
import com.datastrato.gravitino.rel.expressions.distributions.Strategy;
import com.datastrato.gravitino.rel.expressions.sorts.NullOrdering;
import com.datastrato.gravitino.rel.expressions.sorts.SortDirection;
import com.datastrato.gravitino.rel.expressions.sorts.SortOrder;
import com.datastrato.gravitino.rel.expressions.sorts.SortOrders;
import com.datastrato.gravitino.rel.expressions.transforms.Transform;
import com.datastrato.gravitino.rel.types.Types;
import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -337,20 +337,16 @@ public void testCreateHiveTableWithDistributionAndSortOrder()

NameIdentifier nameIdentifier =
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName);
DistributionDTO distribution =
new DistributionDTO.Builder()
.withNumber(10)
.withArgs(FieldReferenceDTO.of(HIVE_COL_NAME1))
.withStrategy(Strategy.EVEN)
.build();

final SortOrderDTO[] sortOrders =
new SortOrderDTO[] {
new SortOrderDTO.Builder()
.withNullOrder(NullOrdering.NULLS_FIRST)
.withDirection(SortDirection.DESCENDING)
.withSortTerm(FieldReferenceDTO.of(HIVE_COL_NAME2))
.build()
Distribution distribution =
Distributions.of(Strategy.EVEN, 10, NamedReference.field(HIVE_COL_NAME1));

final SortOrder[] sortOrders =
new SortOrder[] {
SortOrders.of(
NamedReference.field(HIVE_COL_NAME2),
SortDirection.DESCENDING,
NullOrdering.NULLS_FIRST)
};

Map<String, String> properties = createProperties();
Expand Down Expand Up @@ -396,12 +392,8 @@ public void testCreateHiveTableWithDistributionAndSortOrder()

// Test bad request
// Bad name in distribution
final DistributionDTO badDistribution =
new DistributionDTO.Builder()
.withNumber(10)
.withArgs(FieldReferenceDTO.of(HIVE_COL_NAME1 + "bad_name"))
.withStrategy(Strategy.EVEN)
.build();
final Distribution badDistribution =
Distributions.of(Strategy.EVEN, 10, NamedReference.field(HIVE_COL_NAME1 + "bad_name"));
Assertions.assertThrows(
Exception.class,
() -> {
Expand All @@ -417,13 +409,12 @@ public void testCreateHiveTableWithDistributionAndSortOrder()
sortOrders);
});

final SortOrderDTO[] badSortOrders =
new SortOrderDTO[] {
new SortOrderDTO.Builder()
.withNullOrder(NullOrdering.NULLS_FIRST)
.withDirection(SortDirection.DESCENDING)
.withSortTerm(FieldReferenceDTO.of(HIVE_COL_NAME2 + "bad_name"))
.build()
final SortOrder[] badSortOrders =
new SortOrder[] {
SortOrders.of(
NamedReference.field(HIVE_COL_NAME2 + "bad_name"),
SortDirection.DESCENDING,
NullOrdering.NULLS_FIRST)
};

Assertions.assertThrows(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import com.datastrato.gravitino.catalog.jdbc.config.JdbcConfig;
import com.datastrato.gravitino.client.GravitinoMetaLake;
import com.datastrato.gravitino.dto.rel.ColumnDTO;
import com.datastrato.gravitino.dto.rel.SortOrderDTO;
import com.datastrato.gravitino.dto.rel.partitions.Partitioning;
import com.datastrato.gravitino.exceptions.NoSuchSchemaException;
import com.datastrato.gravitino.exceptions.NotFoundException;
Expand Down Expand Up @@ -278,7 +277,7 @@ void testCreateAndLoadMysqlTable() {
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName);
Distribution distribution = Distributions.NONE;

final SortOrderDTO[] sortOrders = SortOrderDTO.EMPTY_SORT;
final SortOrder[] sortOrders = new SortOrder[0];

Partitioning[] partitioning = Partitioning.EMPTY_PARTITIONING;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import com.datastrato.gravitino.catalog.jdbc.config.JdbcConfig;
import com.datastrato.gravitino.client.GravitinoMetaLake;
import com.datastrato.gravitino.dto.rel.ColumnDTO;
import com.datastrato.gravitino.dto.rel.SortOrderDTO;
import com.datastrato.gravitino.dto.rel.partitions.Partitioning;
import com.datastrato.gravitino.exceptions.NoSuchSchemaException;
import com.datastrato.gravitino.exceptions.SchemaAlreadyExistsException;
Expand Down Expand Up @@ -281,8 +280,7 @@ void testCreateAndLoadPostgreSqlTable() {
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName);
Distribution distribution = Distributions.NONE;

final SortOrderDTO[] sortOrders = SortOrderDTO.EMPTY_SORT;

SortOrder[] sortOrders = new SortOrder[0];
Partitioning[] partitioning = Partitioning.EMPTY_PARTITIONING;

Map<String, String> properties = createProperties();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
import com.datastrato.gravitino.catalog.lakehouse.iceberg.ops.IcebergTableOpsHelper;
import com.datastrato.gravitino.client.GravitinoMetaLake;
import com.datastrato.gravitino.dto.rel.ColumnDTO;
import com.datastrato.gravitino.dto.rel.SortOrderDTO;
import com.datastrato.gravitino.dto.rel.expressions.FieldReferenceDTO;
import com.datastrato.gravitino.dto.rel.partitions.DayPartitioningDTO;
import com.datastrato.gravitino.dto.rel.partitions.IdentityPartitioningDTO;
import com.datastrato.gravitino.dto.rel.partitions.Partitioning;
Expand All @@ -36,11 +34,15 @@
import com.datastrato.gravitino.rel.Table;
import com.datastrato.gravitino.rel.TableCatalog;
import com.datastrato.gravitino.rel.TableChange;
import com.datastrato.gravitino.rel.expressions.NamedReference;
import com.datastrato.gravitino.rel.expressions.distributions.Distribution;
import com.datastrato.gravitino.rel.expressions.distributions.Distributions;
import com.datastrato.gravitino.rel.expressions.sorts.NullOrdering;
import com.datastrato.gravitino.rel.expressions.sorts.SortDirection;
import com.datastrato.gravitino.rel.expressions.sorts.SortOrder;
import com.datastrato.gravitino.rel.expressions.sorts.SortOrders;
import com.datastrato.gravitino.rel.expressions.transforms.Transform;
import com.datastrato.gravitino.rel.expressions.transforms.Transforms;
import com.datastrato.gravitino.rel.types.Types;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
Expand Down Expand Up @@ -335,13 +337,12 @@ void testCreateAndLoadIcebergTable() {
NameIdentifier.of(metalakeName, catalogName, schemaName, tableName);
Distribution distribution = Distributions.NONE;

final SortOrderDTO[] sortOrders =
new SortOrderDTO[] {
new SortOrderDTO.Builder()
.withNullOrder(NullOrdering.NULLS_FIRST)
.withDirection(SortDirection.DESCENDING)
.withSortTerm(FieldReferenceDTO.of(ICEBERG_COL_NAME2))
.build()
final SortOrder[] sortOrders =
new SortOrder[] {
SortOrders.of(
NamedReference.field(ICEBERG_COL_NAME2),
SortDirection.DESCENDING,
NullOrdering.NULLS_FIRST)
};

Partitioning[] partitioning = new Partitioning[] {DayPartitioningDTO.of(columns[1].name())};
Expand Down Expand Up @@ -640,21 +641,15 @@ public void testAlterIcebergTable() {
void testPartitionAndSortOrderIcebergTable() {
ColumnDTO[] columns = createColumns();
String testTableName = GravitinoITUtils.genRandomName("test_table");
SortOrderDTO[] sortOrders = {
new SortOrderDTO.Builder()
.withSortTerm(FieldReferenceDTO.of(columns[0].name()))
.withDirection(SortDirection.ASCENDING)
.withNullOrder(NullOrdering.NULLS_FIRST)
.build(),
new SortOrderDTO.Builder()
.withSortTerm(FieldReferenceDTO.of(columns[2].name()))
.withDirection(SortDirection.DESCENDING)
.withNullOrder(NullOrdering.NULLS_LAST)
.build()
SortOrder[] sortOrders = {
SortOrders.ascending(NamedReference.field(columns[0].name())),
SortOrders.descending(NamedReference.field(columns[2].name()))
};
Partitioning[] partitioning = {
DayPartitioningDTO.of(columns[1].name()), IdentityPartitioningDTO.of(columns[2].name())

Transform[] partitioning = {
Transforms.day(columns[1].name()), Transforms.identity(columns[2].name())
};

catalog
.asTableCatalog()
.createTable(
Expand Down Expand Up @@ -692,16 +687,14 @@ void testOperationDataIcebergTable() {
ColumnDTO[] columns = createColumns();
String testTableName = GravitinoITUtils.genRandomName("test_table");
SortOrder[] sortOrders = {
new SortOrderDTO.Builder()
.withSortTerm(FieldReferenceDTO.of(columns[0].name()))
.withDirection(SortDirection.ASCENDING)
.withNullOrder(NullOrdering.NULLS_FIRST)
.build(),
new SortOrderDTO.Builder()
.withSortTerm(FieldReferenceDTO.of(columns[2].name()))
.withDirection(SortDirection.DESCENDING)
.withNullOrder(NullOrdering.NULLS_LAST)
.build()
SortOrders.of(
NamedReference.field(columns[0].name()),
SortDirection.DESCENDING,
NullOrdering.NULLS_FIRST),
SortOrders.of(
NamedReference.field(columns[2].name()),
SortDirection.DESCENDING,
NullOrdering.NULLS_FIRST),
};
Partitioning[] transforms = {
DayPartitioningDTO.of(columns[1].name()), IdentityPartitioningDTO.of(columns[2].name())
Expand Down

0 comments on commit c905c3f

Please sign in to comment.