Skip to content

Support basic timestamp in the iceberg connector #17190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
beinan merged 1 commit into from
Feb 9, 2022
Merged

Support basic timestamp in the iceberg connector #17190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions presto-iceberg/src/main/java/com/facebook/presto/iceberg/ExpressionConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import com.facebook.presto.common.type.MapType;
import com.facebook.presto.common.type.RealType;
import com.facebook.presto.common.type.RowType;
import com.facebook.presto.common.type.TimeType;
import com.facebook.presto.common.type.TimestampType;
import com.facebook.presto.common.type.Type;
import com.facebook.presto.common.type.VarbinaryType;
import com.facebook.presto.common.type.VarcharType;
Expand All @@ -47,6 +49,7 @@
import static java.lang.Float.intBitsToFloat;
import static java.lang.Math.toIntExact;
import static java.util.Objects.requireNonNull;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static org.apache.iceberg.expressions.Expressions.alwaysFalse;
import static org.apache.iceberg.expressions.Expressions.alwaysTrue;
import static org.apache.iceberg.expressions.Expressions.and;
Expand Down Expand Up @@ -179,6 +182,10 @@ private static Object getIcebergLiteralValue(Type type, Marker marker)
return toIntExact(((Long) marker.getValue()));
}

if (type instanceof TimestampType || type instanceof TimeType) {
return MILLISECONDS.toMicros((Long) marker.getValue());
}

if (type instanceof VarcharType) {
return ((Slice) marker.getValue()).toStringUtf8();
}
Expand Down
4 changes: 3 additions & 1 deletion presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergPageSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
import static com.facebook.presto.common.type.DoubleType.DOUBLE;
import static com.facebook.presto.common.type.IntegerType.INTEGER;
import static com.facebook.presto.common.type.RealType.REAL;
import static com.facebook.presto.common.type.TimeType.TIME;
import static com.facebook.presto.common.type.TimestampType.TIMESTAMP;
import static com.facebook.presto.iceberg.IcebergErrorCode.ICEBERG_BAD_DATA;
import static com.facebook.presto.iceberg.IcebergErrorCode.ICEBERG_INVALID_PARTITION_VALUE;
import static com.facebook.presto.spi.StandardErrorCode.GENERIC_INTERNAL_ERROR;
Expand Down Expand Up @@ -208,7 +210,7 @@ private static Object deserializePartitionValue(Type type, String valueString, S
if (type.equals(DOUBLE)) {
return parseDouble(valueString);
}
if (type.equals(DATE)) {
if (type.equals(DATE) || type.equals(TIME) || type.equals(TIMESTAMP)) {
return parseLong(valueString);
}
if (type instanceof VarcharType) {
Expand Down
15 changes: 7 additions & 8 deletions presto-iceberg/src/main/java/com/facebook/presto/iceberg/TypeConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@
import static com.facebook.presto.common.type.RealType.REAL;
import static com.facebook.presto.common.type.SmallintType.SMALLINT;
import static com.facebook.presto.common.type.TimestampType.TIMESTAMP;
import static com.facebook.presto.common.type.TimestampWithTimeZoneType.TIMESTAMP_WITH_TIME_ZONE;
import static com.facebook.presto.common.type.TinyintType.TINYINT;
import static com.facebook.presto.common.type.VarbinaryType.VARBINARY;
import static com.facebook.presto.hive.HiveType.HIVE_BINARY;
Expand Down Expand Up @@ -116,6 +115,10 @@ public static Type toPrestoType(org.apache.iceberg.types.Type type, TypeManager
return RealType.REAL;
case INTEGER:
return IntegerType.INTEGER;
case TIME:
return TimeType.TIME;
case TIMESTAMP:
return TimestampType.TIMESTAMP;
case STRING:
return VarcharType.createUnboundedVarcharType();
case LIST:
Expand Down Expand Up @@ -175,13 +178,13 @@ public static org.apache.iceberg.types.Type toIcebergType(Type type)
return fromMap((MapType) type);
}
if (type instanceof TimeType) {
throw new PrestoException(NOT_SUPPORTED, format("Time not supported for Iceberg."));
return Types.TimeType.get();
}
if (type instanceof TimestampType) {
throw new PrestoException(NOT_SUPPORTED, format("Timestamp not supported for Iceberg."));
return Types.TimestampType.withoutZone();
}
if (type instanceof TimestampWithTimeZoneType) {
throw new PrestoException(NOT_SUPPORTED, format("Timestamp with timezone not supported for Iceberg."));
return Types.TimestampType.withZone();
}
throw new PrestoException(NOT_SUPPORTED, "Type not supported for Iceberg: " + type.getDisplayName());
}
Expand Down Expand Up @@ -268,10 +271,6 @@ private static TypeInfo toHiveTypeInfo(Type type)
if (TIMESTAMP.equals(type)) {
return HIVE_TIMESTAMP.getTypeInfo();
}
if (TIMESTAMP_WITH_TIME_ZONE.equals(type)) {
// Hive does not have TIMESTAMP_WITH_TIME_ZONE, this is just a work around for iceberg.
return HIVE_TIMESTAMP.getTypeInfo();
}
if (type instanceof DecimalType) {
DecimalType decimalType = (DecimalType) type;
return new DecimalTypeInfo(decimalType.getPrecision(), decimalType.getScale());
Expand Down
5 changes: 4 additions & 1 deletion presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergSmoke.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ protected QueryRunner createQueryRunner()
@Test
public void testTimestamp()
{
// TODO
assertUpdate("CREATE TABLE test_timestamp (x timestamp)");
assertUpdate("INSERT INTO test_timestamp VALUES (timestamp '2017-05-01 10:12:34')", 1);
assertQuery("SELECT * FROM test_timestamp", "SELECT CAST('2017-05-01 10:12:34' AS TIMESTAMP)");
Copy link
Copy Markdown
Member

@beinan beinan Jan 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any test to cover the time with zone?

Copy link
Copy Markdown
Member Author

@ChunxuTang ChunxuTang Jan 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The timestamp with time zone will require changes in the ORC/parquet readers underneath. So I remove the timestamp with time zone support temporarily for now.

dropTable(getSession(), "test_timestamp");
}

@Test
Expand Down