prestodb / presto

The official home of the Presto distributed SQL query engine for big data
http://prestodb.io
Apache License 2.0
16.05k stars 5.38k forks source link

Iceberg `timestamptz` should map to Presto `TIMESTAMP WITH TIME ZONE` type #23529

Open tdcmeehan opened 2 months ago

tdcmeehan commented 2 months ago

Your Environment

Expected Behavior

Currently, the Presto Iceberg connector does not differentiate between timestamp and timestamptz, mapping both to TIMESTAMP. This is incorrect, especially when deprecated.legacy-timestamp is set to false, because timestamptz represents a point in time value, whereas Presto's TIMESTAMP represents a timestamp that is not a point in time.

Current Behavior

All Iceberg timestamp and timestamptzs are being mapped to TIMESTAMP

Possible Solution

Improve the type mapping to use TIMESTAMP WITH TIME ZONE

Steps to Reproduce

1. 2. 3. 4.

Screenshots (if appropriate)

Context

auden-woolfson commented 2 months ago

I think updating the timestamp case in TypeConverter toPrestoType to...

  case TIMESTAMP:
      Types.TimestampType timestampType = (Types.TimestampType) type.asPrimitiveType();
      if (timestampType.shouldAdjustToUTC()) {
          return TimestampWithTimeZoneType.TIMESTAMP_WITH_TIME_ZONE;
      }
      return TimestampType.TIMESTAMP;

should fix this