[#4743]fix(trino-connector): Fix the default precision of time and timestamp in the Iceberg catalog. #4893
Conversation
|
Does this need to be backported to branch-0.6? |
...ctor/src/main/java/org/apache/gravitino/trino/connector/util/GeneralDataTypeTransformer.java
Show resolved
Hide resolved
| @@ -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; | |||
There was a problem hiding this comment.
seems you shoud add corresponding logic to convert MICROS time to Iceberg time, and forbidden other time precision.
There was a problem hiding this comment.
There was a problem hiding this comment.
Trino iceberg connector can automatically convert other time precision to microsecond
There was a problem hiding this comment.
what will happen if the user creates an Iceberg table with timestamp(3) ?
There was a problem hiding this comment.
It automatically converts to timestamp(6), The behavior is the same as in the Trino Iceberg catalog.
There was a problem hiding this comment.
according to the code https://github.com/trinodb/trino/blob/457/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TypeConverter.java#L148-L177, it will throw an exception in origin Trino Iceberg catalog, Yes?
There was a problem hiding this comment.
so I don't think it's the right behavior to convert timestamp(3) to timestamp(6) in Gravitino Iceberg connector, it conflict with original behavior.
There was a problem hiding this comment.
Trino Iceberg connector can automatically converts timestamp and timestamp(n) to timestamp(6),
The Gravitino connector Iceberg catalog's behavior is the same as Trino Iceberg connector
Yes |
|
LGTM |
…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
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