Skip to content

Commit

Permalink
[apache#800] feat(all): Replace the old type system with a new one (a…
Browse files Browse the repository at this point in the history
…pache#801)

### What changes were proposed in this pull request?
 - Replace the old type system with a new one
 - modify the type converter between Iceberg and Gravitino
 - modify the type converter between Hive and Gravitino
 - modify the type converter between Trino and Gravitino

### Why are the changes needed?

Fix: apache#800 

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

yes, type API use Gravitino type instead of Substrait

### How was this patch tested?
UTs and ITs

---------

Co-authored-by: Jerry Shao <jerryshao@datastrato.com>
  • Loading branch information
mchades and jerryshao authored Nov 24, 2023
1 parent a03d24f commit c03911f
Show file tree
Hide file tree
Showing 58 changed files with 673 additions and 766 deletions.
1 change: 0 additions & 1 deletion LICENSE.bin
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,6 @@
Airlift
The Netty Project
Open Telemetry
Substrait Java
Trino
Jakarta Dependency Injection
Jakarta Bean Validation
Expand Down
8 changes: 0 additions & 8 deletions api/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,6 @@ plugins {
}

dependencies {
implementation(libs.substrait.java.core) {
exclude("com.fasterxml.jackson.core")
exclude("com.fasterxml.jackson.datatype")
exclude("com.fasterxml.jackson.dataformat")
exclude("com.google.protobuf")
exclude("com.google.code.findbugs")
exclude("org.slf4j")
}
implementation(libs.guava)
implementation(libs.slf4j.api)

Expand Down
2 changes: 1 addition & 1 deletion api/src/main/java/com/datastrato/gravitino/rel/Column.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
package com.datastrato.gravitino.rel;

import com.datastrato.gravitino.NameIdentifier;
import io.substrait.type.Type;
import com.datastrato.gravitino.rel.types.Type;
import java.util.Map;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

package com.datastrato.gravitino.rel;

import io.substrait.type.Type;
import com.datastrato.gravitino.rel.types.Type;
import lombok.EqualsAndHashCode;
import lombok.Getter;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@

package com.datastrato.gravitino.rel.expressions;

import io.substrait.type.Type;
import io.substrait.type.TypeCreator;
import com.datastrato.gravitino.rel.types.Type;
import com.datastrato.gravitino.rel.types.Types;
import java.util.Objects;

/**
Expand Down Expand Up @@ -60,7 +60,7 @@ static <T> LiteralImpl<T> of(T value, Type dataType) {
* @return a new {@link Literal} instance
*/
static LiteralImpl<Integer> integer(Integer value) {
return of(value, TypeCreator.REQUIRED.I32);
return of(value, Types.IntegerType.get());
}

/**
Expand All @@ -70,7 +70,7 @@ static LiteralImpl<Integer> integer(Integer value) {
* @return a new {@link Literal} instance
*/
static LiteralImpl<String> string(String value) {
return of(value, TypeCreator.REQUIRED.STRING);
return of(value, Types.StringType.get());
}

final class LiteralImpl<T> implements Literal<T> {
Expand Down
25 changes: 13 additions & 12 deletions api/src/test/java/com/datastrato/gravitino/TestTableChange.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
import com.datastrato.gravitino.rel.TableChange.UpdateColumnPosition;
import com.datastrato.gravitino.rel.TableChange.UpdateColumnType;
import com.datastrato.gravitino.rel.TableChange.UpdateComment;
import io.substrait.type.Type;
import com.datastrato.gravitino.rel.types.Type;
import com.datastrato.gravitino.rel.types.Types;
import org.junit.jupiter.api.Test;

public class TestTableChange {
Expand Down Expand Up @@ -67,7 +68,7 @@ public void testColumnPosition() {
@Test
public void testAddColumn() {
String[] fieldNames = {"Name"};
Type dataType = Type.withNullability(false).STRING;
Type dataType = Types.StringType.get();
String comment = "Person name";
AddColumn addColumn = (AddColumn) TableChange.addColumn(fieldNames, dataType, comment);

Expand All @@ -80,7 +81,7 @@ public void testAddColumn() {
@Test
public void testAddColumnWithPosition() {
String[] fieldNames = {"Full Name", "First Name"};
Type dataType = Type.withNullability(false).STRING;
Type dataType = Types.StringType.get();
String comment = "First or given name";
TableChange.ColumnPosition position = TableChange.ColumnPosition.after("Address");
AddColumn addColumn =
Expand All @@ -95,7 +96,7 @@ public void testAddColumnWithPosition() {
@Test
public void testAddColumnWithNullCommentAndPosition() {
String[] fieldNames = {"Middle Name"};
Type dataType = Type.withNullability(false).STRING;
Type dataType = Types.StringType.get();
AddColumn addColumn = (AddColumn) TableChange.addColumn(fieldNames, dataType, null, null);

assertArrayEquals(fieldNames, addColumn.fieldNames());
Expand Down Expand Up @@ -191,7 +192,7 @@ public void testDeleteNestedColumn() {
@Test
public void testUpdateColumnType() {
String[] fieldNames = {"existing_column"};
Type dataType = Type.withNullability(false).STRING;
Type dataType = Types.StringType.get();
UpdateColumnType updateColumnType =
(UpdateColumnType) TableChange.updateColumnType(fieldNames, dataType);

Expand All @@ -202,7 +203,7 @@ public void testUpdateColumnType() {
@Test
public void testUpdateNestedColumnType() {
String[] fieldNames = {"nested", "existing_column"};
Type dataType = Type.withNullability(false).STRING;
Type dataType = Types.StringType.get();
UpdateColumnType updateColumnType =
(UpdateColumnType) TableChange.updateColumnType(fieldNames, dataType);

Expand Down Expand Up @@ -347,7 +348,7 @@ void testColumnRenameNotEqualsAndHashCode() {
@Test
void testColumnUpdateTypeEqualsAndHashCode() {
String[] nameA = {"First Name"};
Type dataType = Type.withNullability(false).STRING;
Type dataType = Types.StringType.get();
UpdateColumnType columnA = (UpdateColumnType) TableChange.updateColumnType(nameA, dataType);
String[] nameB = {"First Name"};
UpdateColumnType columnB = (UpdateColumnType) TableChange.updateColumnType(nameB, dataType);
Expand All @@ -360,7 +361,7 @@ void testColumnUpdateTypeEqualsAndHashCode() {
@Test
void testColumnUpdateTypeNotEqualsAndHashCode() {
String[] nameA = {"First Name"};
Type dataType = Type.withNullability(false).STRING;
Type dataType = Types.StringType.get();
UpdateColumnType columnA = (UpdateColumnType) TableChange.updateColumnType(nameA, dataType);
String[] nameB = {"Given Name"};
UpdateColumnType columnB = (UpdateColumnType) TableChange.updateColumnType(nameB, dataType);
Expand Down Expand Up @@ -492,11 +493,11 @@ void testSetPropertyNotEqualsAndHashCode() {
@Test
void testAddColumnEqualsAndHashCode() {
String[] fieldNamesA = {"Name"};
Type dataTypeA = Type.withNullability(false).STRING;
Type dataTypeA = Types.StringType.get();
String commentA = "Person name";
AddColumn columnA = (AddColumn) TableChange.addColumn(fieldNamesA, dataTypeA, commentA);
String[] fieldNamesB = {"Name"};
Type dataTypeB = Type.withNullability(false).STRING;
Type dataTypeB = Types.StringType.get();
String commentB = "Person name";
AddColumn columnB = (AddColumn) TableChange.addColumn(fieldNamesB, dataTypeB, commentB);

Expand All @@ -508,11 +509,11 @@ void testAddColumnEqualsAndHashCode() {
@Test
void testAddColumnNotEqualsAndHashCode() {
String[] fieldNamesA = {"Name"};
Type dataTypeA = Type.withNullability(false).STRING;
Type dataTypeA = Types.StringType.get();
String commentA = "Person name";
AddColumn columnA = (AddColumn) TableChange.addColumn(fieldNamesA, dataTypeA, commentA);
String[] fieldNamesB = {"First Name"};
Type dataTypeB = Type.withNullability(false).STRING;
Type dataTypeB = Types.StringType.get();
String commentB = "Person name";
AddColumn columnB = (AddColumn) TableChange.addColumn(fieldNamesB, dataTypeB, commentB);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
import com.datastrato.gravitino.rel.expressions.Literal;
import com.datastrato.gravitino.rel.expressions.NamedReference;
import com.datastrato.gravitino.rel.expressions.transforms.Transform;
import io.substrait.type.Type;
import io.substrait.type.TypeCreator;
import com.datastrato.gravitino.rel.types.Type;
import com.datastrato.gravitino.rel.types.Types;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

Expand All @@ -38,7 +38,7 @@ public String name() {

@Override
public Type dataType() {
return TypeCreator.NULLABLE.I8;
return Types.ByteType.get();
}

@Override
Expand Down Expand Up @@ -85,7 +85,7 @@ public String name() {

@Override
public Type dataType() {
return TypeCreator.NULLABLE.I8;
return Types.ByteType.get();
}

@Override
Expand Down
6 changes: 0 additions & 6 deletions catalogs/catalog-hive/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,6 @@ dependencies {
exclude("*")
}

implementation(libs.substrait.java.core) {
exclude("org.slf4j")
exclude("com.fasterxml.jackson.core")
exclude("com.fasterxml.jackson.datatype")
}

implementation(libs.slf4j.api)
implementation(libs.guava)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ private void doAddColumn(List<FieldSchema> cols, TableChange.AddColumn change) {
targetPosition,
new FieldSchema(
change.fieldNames()[0],
change.getDataType().accept(ToHiveType.INSTANCE).getQualifiedName(),
ToHiveType.convert(change.getDataType()).getQualifiedName(),
change.getComment()));
}

Expand Down Expand Up @@ -826,8 +826,7 @@ private void doUpdateColumnType(List<FieldSchema> cols, TableChange.UpdateColumn
if (indexOfColumn == -1) {
throw new IllegalArgumentException("UpdateColumnType does not exist: " + columnName);
}
cols.get(indexOfColumn)
.setType(change.getNewDataType().accept(ToHiveType.INSTANCE).getQualifiedName());
cols.get(indexOfColumn).setType(ToHiveType.convert(change.getNewDataType()).getQualifiedName());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ private FieldSchema getPartitionKey(String[] fieldName) {
.collect(Collectors.toList());
return new FieldSchema(
partitionColumns.get(0).name(),
partitionColumns.get(0).dataType().accept(ToHiveType.INSTANCE).getQualifiedName(),
ToHiveType.convert(partitionColumns.get(0).dataType()).getQualifiedName(),
partitionColumns.get(0).comment());
}

Expand All @@ -228,9 +228,7 @@ private StorageDescriptor buildStorageDescriptor(
.map(
c ->
new FieldSchema(
c.name(),
c.dataType().accept(ToHiveType.INSTANCE).getQualifiedName(),
c.comment()))
c.name(), ToHiveType.convert(c.dataType()).getQualifiedName(), c.comment()))
.collect(Collectors.toList()));

// `location` must not be null, otherwise it will result in an NPE when calling HMS `alterTable`
Expand Down
Loading

0 comments on commit c03911f

Please sign in to comment.