Skip to content

Commit

Permalink
[#4743]fix(trino-connector): Fix the default precision of time and ti…
Browse files Browse the repository at this point in the history
…mestamp in the Iceberg catalog. (#4893)

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

Fix the default precision of time and timestamp in the Iceberg catalog. 
It causes Trino to be unable to read Iceberg tables with data of time
and timestamp

### Why are the changes needed?

Fix: #4743

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

NO

### How was this patch tested?

New UT, IT
  • Loading branch information
diqiu50 authored Sep 13, 2024
1 parent adbf615 commit b0c4b11
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ CREATE SCHEMA gt_db2;

USE gt_db2;

-- Unsupported Type: TINYINT, SMALLINT
-- Unsupported Type: CHAR TINYINT, SMALLINT
CREATE TABLE tb01 (
f1 VARCHAR,
f3 VARBINARY,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ CREATE TABLE
f11 integer,
f12 bigint,
f13 date,
f14 time(3),
f15 timestamp(3)
f14 time(6),
f15 timestamp(6)
)
COMMENT ''
WITH (
Expand All @@ -28,7 +28,7 @@ INSERT: 1 row

INSERT: 1 row

"Sample text 1","65","123.456","7.89","12.34","true","1000","1000","100000","2024-01-01","08:00:00.000","2024-01-01 08:00:00.000"
"Sample text 1","65","123.456","7.89","12.34","true","1000","1000","100000","2024-01-01","08:00:00.000000","2024-01-01 08:00:00.000000"
"","","","","","","","","","","",""

CREATE TABLE
Expand All @@ -44,8 +44,8 @@ CREATE TABLE
f11 integer NOT NULL,
f12 bigint NOT NULL,
f13 date NOT NULL,
f14 time(3) NOT NULL,
f15 timestamp(3) NOT NULL
f14 time(6) NOT NULL,
f15 timestamp(6) NOT NULL
)
COMMENT ''
WITH (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
package org.apache.gravitino.trino.connector.catalog.iceberg;

import io.trino.spi.TrinoException;
import io.trino.spi.type.TimeType;
import io.trino.spi.type.TimestampType;
import io.trino.spi.type.TimestampWithTimeZoneType;
import io.trino.spi.type.VarbinaryType;
import io.trino.spi.type.VarcharType;
import org.apache.gravitino.rel.types.Type;
Expand All @@ -45,7 +48,6 @@ public Type getGravitinoType(io.trino.spi.type.Type type) {
GravitinoErrorCode.GRAVITINO_ILLEGAL_ARGUMENT,
"Iceberg does not support the datatype VARCHAR with length");
}

return Types.StringType.get();
}

Expand All @@ -56,7 +58,17 @@ public Type getGravitinoType(io.trino.spi.type.Type type) {
public io.trino.spi.type.Type getTrinoType(Type type) {
if (Name.FIXED == type.name()) {
return VarbinaryType.VARBINARY;
} else if (Name.TIME == type.name()) {
return TimeType.TIME_MICROS;
} else if (Name.TIMESTAMP == type.name()) {
Types.TimestampType timestampType = (Types.TimestampType) type;
if (timestampType.hasTimeZone()) {
return TimestampWithTimeZoneType.TIMESTAMP_TZ_MICROS;
} else {
return TimestampType.TIMESTAMP_MICROS;
}
}

return super.getTrinoType(type);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import io.trino.spi.type.SmallintType;
import io.trino.spi.type.TimeType;
import io.trino.spi.type.TimestampType;
import io.trino.spi.type.TimestampWithTimeZoneType;
import io.trino.spi.type.TinyintType;
import io.trino.spi.type.Type;
import io.trino.spi.type.TypeOperators;
Expand Down Expand Up @@ -103,7 +104,11 @@ public Type getTrinoType(org.apache.gravitino.rel.types.Type type) {
case TIME:
return TimeType.TIME_MILLIS;
case TIMESTAMP:
return TimestampType.TIMESTAMP_MILLIS;
if (((Types.TimestampType) type).hasTimeZone()) {
return TimestampWithTimeZoneType.TIMESTAMP_TZ_MILLIS;
} else {
return TimestampType.TIMESTAMP_MILLIS;
}
case LIST:
return new ArrayType(getTrinoType(((Types.ListType) type).elementType()));
case MAP:
Expand Down Expand Up @@ -173,10 +178,12 @@ public org.apache.gravitino.rel.types.Type getGravitinoType(Type type) {
return Types.BinaryType.get();
} else if (typeClass == io.trino.spi.type.DateType.class) {
return Types.DateType.get();
} else if (typeClass == io.trino.spi.type.TimeType.class) {
} else if (io.trino.spi.type.TimeType.class.isAssignableFrom(typeClass)) {
return Types.TimeType.get();
} else if (io.trino.spi.type.TimestampType.class.isAssignableFrom(typeClass)) {
return Types.TimestampType.withoutTimeZone();
} else if (io.trino.spi.type.TimestampWithTimeZoneType.class.isAssignableFrom(typeClass)) {
return Types.TimestampType.withTimeZone();
} else if (typeClass == io.trino.spi.type.ArrayType.class) {
// Ignore nullability for the type, we could only get nullability from column metadata
return Types.ListType.of(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
package org.apache.gravitino.trino.connector.catalog.iceberg;

import io.trino.spi.TrinoException;
import io.trino.spi.type.TimeType;
import io.trino.spi.type.TimestampType;
import io.trino.spi.type.TimestampWithTimeZoneType;
import io.trino.spi.type.VarcharType;
import org.apache.gravitino.rel.types.Types;
import org.apache.gravitino.trino.connector.util.GeneralDataTypeTransformer;
Expand Down Expand Up @@ -51,5 +54,16 @@ public void testTrinoTypeToGravitinoType() {
Assertions.assertEquals(
generalDataTypeTransformer.getGravitinoType(varcharTypeWithoutLength),
Types.StringType.get());

Assertions.assertEquals(
generalDataTypeTransformer.getTrinoType(Types.TimeType.get()), TimeType.TIME_MICROS);

Assertions.assertEquals(
generalDataTypeTransformer.getTrinoType(Types.TimestampType.withoutTimeZone()),
TimestampType.TIMESTAMP_MICROS);

Assertions.assertEquals(
generalDataTypeTransformer.getTrinoType(Types.TimestampType.withTimeZone()),
TimestampWithTimeZoneType.TIMESTAMP_TZ_MICROS);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import io.trino.spi.type.SmallintType;
import io.trino.spi.type.TimeType;
import io.trino.spi.type.TimestampType;
import io.trino.spi.type.TimestampWithTimeZoneType;
import io.trino.spi.type.TinyintType;
import io.trino.spi.type.TypeOperators;
import io.trino.spi.type.UuidType;
Expand Down Expand Up @@ -91,6 +92,9 @@ public void testGetGravitinoType() {
Assertions.assertEquals(
dataTypeTransformer.getGravitinoType(TimestampType.TIMESTAMP_MILLIS),
Types.TimestampType.withoutTimeZone());
Assertions.assertEquals(
dataTypeTransformer.getGravitinoType(TimestampWithTimeZoneType.TIMESTAMP_TZ_MILLIS),
Types.TimestampType.withTimeZone());

Assertions.assertEquals(
dataTypeTransformer.getGravitinoType(new ArrayType(IntegerType.INTEGER)),
Expand Down Expand Up @@ -168,6 +172,9 @@ public void testGetTrinoType() {
Assertions.assertEquals(
dataTypeTransformer.getTrinoType(Types.TimestampType.withoutTimeZone()),
TimestampType.TIMESTAMP_MILLIS);
Assertions.assertEquals(
dataTypeTransformer.getTrinoType(Types.TimestampType.withTimeZone()),
TimestampWithTimeZoneType.TIMESTAMP_TZ_MILLIS);

Assertions.assertEquals(
dataTypeTransformer.getTrinoType(Types.ListType.nullable(Types.IntegerType.get())),
Expand Down

0 comments on commit b0c4b11

Please sign in to comment.