Skip to content

Commit c75b5a9

Browse files
krvikashkrvikash
committed
Validate out of range DateTime value in clickhouse connector
1 parent 6309133 commit c75b5a9

File tree

2 files changed

+45
-18
lines changed

2 files changed

+45
-18
lines changed

‎plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,7 @@ public Optional<ColumnMapping> toColumnMapping(ConnectorSession session, Connect
636636
return mapping;
637637
}
638638

639+
ClickHouseVersion version = getClickHouseServerVersion(session);
639640
ClickHouseColumn column = ClickHouseColumn.of("", jdbcTypeName);
640641
ClickHouseDataType columnDataType = column.getDataType();
641642
switch (columnDataType) {
@@ -724,7 +725,7 @@ public Optional<ColumnMapping> toColumnMapping(ConnectorSession session, Connect
724725
DISABLE_PUSHDOWN));
725726

726727
case Types.DATE:
727-
return Optional.of(dateColumnMappingUsingLocalDate(getClickHouseServerVersion(session)));
728+
return Optional.of(dateColumnMappingUsingLocalDate(version));
728729

729730
case Types.TIMESTAMP:
730731
if (columnDataType == ClickHouseDataType.DateTime) {
@@ -733,7 +734,7 @@ public Optional<ColumnMapping> toColumnMapping(ConnectorSession session, Connect
733734
return Optional.of(ColumnMapping.longMapping(
734735
TIMESTAMP_SECONDS,
735736
timestampReadFunction(TIMESTAMP_SECONDS),
736-
timestampSecondsWriteFunction(getClickHouseServerVersion(session))));
737+
timestampSecondsWriteFunction(version)));
737738
}
738739
// TODO (https://github.com/trinodb/trino/issues/10537) Add support for Datetime64 type
739740
return Optional.of(timestampColumnMapping(TIMESTAMP_MILLIS));
@@ -745,7 +746,7 @@ public Optional<ColumnMapping> toColumnMapping(ConnectorSession session, Connect
745746
return Optional.of(ColumnMapping.longMapping(
746747
TIMESTAMP_TZ_SECONDS,
747748
shortTimestampWithTimeZoneReadFunction(),
748-
shortTimestampWithTimeZoneWriteFunction(column.getTimeZone())));
749+
shortTimestampWithTimeZoneWriteFunction(version, column.getTimeZone())));
749750
}
750751
}
751752

@@ -926,12 +927,15 @@ private static LongReadFunction shortTimestampWithTimeZoneReadFunction()
926927
};
927928
}
928929

929-
private static LongWriteFunction shortTimestampWithTimeZoneWriteFunction(TimeZone columnTimeZone)
930+
private static LongWriteFunction shortTimestampWithTimeZoneWriteFunction(ClickHouseVersion version, TimeZone columnTimeZone)
930931
{
931932
return (statement, index, value) -> {
932933
long millisUtc = unpackMillisUtc(value);
933934
// Clickhouse JDBC driver inserts datetime as string value as yyyy-MM-dd HH:mm:ss and zone from the Column metadata would be used.
934-
statement.setObject(index, Instant.ofEpochMilli(millisUtc).atZone(columnTimeZone.toZoneId()));
935+
Instant instant = Instant.ofEpochMilli(millisUtc);
936+
// ClickHouse stores incorrect results when the values are out of supported range.
937+
DATETIME.validate(version, instant.atZone(UTC).toLocalDateTime());
938+
statement.setObject(index, instant.atZone(columnTimeZone.toZoneId()));
935939
};
936940
}
937941

‎plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/BaseClickHouseTypeMapping.java‎

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,13 @@
5959
import static io.trino.testing.TestingSession.testSessionBuilder;
6060
import static io.trino.type.IpAddressType.IPADDRESS;
6161
import static java.lang.String.format;
62-
import static java.time.ZoneOffset.UTC;
6362
import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS;
6463

6564
@TestInstance(PER_CLASS)
6665
public abstract class BaseClickHouseTypeMapping
6766
extends AbstractTestQueryFramework
6867
{
68+
private static final ZoneId UTC = ZoneId.of("UTC");
6969
private static final ZoneId JVM_ZONE = ZoneId.systemDefault();
7070
// no DST in 1970, but has DST in later years (e.g. 2018)
7171
private static final ZoneId VILNIUS = ZoneId.of("Europe/Vilnius");
@@ -1018,6 +1018,30 @@ public void testUnsupportedTimestamp(String unsupportedTimestamp)
10181018
}
10191019
}
10201020

1021+
@Test
1022+
void testUnsupportedDateTimeWithTimeZone()
1023+
{
1024+
for (ZoneId zoneId : timezones()) {
1025+
String inputType = DATETIME_TYPE_FACTORY.apply(zoneId);
1026+
testUnsupportedDateTimeWithTimeZone(inputType, "1969-12-31 23:59:59 UTC", "1969-12-31 23:59:59"); // min - 1 second
1027+
testUnsupportedDateTimeWithTimeZone(inputType, "2106-02-07 06:28:16 UTC", "2106-02-07 06:28:16"); // max + 1 second
1028+
testUnsupportedDateTimeWithTimeZone(inputType, "1970-01-01 00:00:00 Asia/Kathmandu", "1969-12-31 18:30:00");
1029+
testUnsupportedDateTimeWithTimeZone(inputType, "1970-01-01 00:13:42 Asia/Kathmandu", "1969-12-31 18:43:42");
1030+
}
1031+
}
1032+
1033+
private void testUnsupportedDateTimeWithTimeZone(String inputType, String unsupportedTimestampWithTz, String unsupportedTimestampUtc)
1034+
{
1035+
String minSupportedTimestamp = "1970-01-01 00:00:00";
1036+
String maxSupportedTimestamp = "2106-02-07 06:28:15";
1037+
1038+
try (TestTable table = new TestTable(onRemoteDatabase(), "tpch.test_unsupported_timestamp_with_tz", "(dt %s) ENGINE=Log".formatted(inputType))) {
1039+
assertQueryFails(
1040+
"INSERT INTO %s VALUES (TIMESTAMP '%s')".formatted(table.getName(), unsupportedTimestampWithTz),
1041+
"Timestamp must be between %s and %s in ClickHouse: %s".formatted(minSupportedTimestamp, maxSupportedTimestamp, unsupportedTimestampUtc));
1042+
}
1043+
}
1044+
10211045
@Test
10221046
public void testClickHouseDateTimeWithTimeZone()
10231047
{
@@ -1039,39 +1063,38 @@ public void testClickHouseDateTimeWithTimeZone()
10391063

10401064
private SqlDataTypeTest dateTimeWithTimeZoneTest(Function<ZoneId, String> inputTypeFactory)
10411065
{
1042-
ZoneId utc = ZoneId.of("UTC");
10431066
SqlDataTypeTest tests = SqlDataTypeTest.create()
1044-
.addRoundTrip(format("Nullable(%s)", inputTypeFactory.apply(utc)), "NULL", TIMESTAMP_TZ_SECONDS, "CAST(NULL AS TIMESTAMP(0) WITH TIME ZONE)")
1067+
.addRoundTrip(format("Nullable(%s)", inputTypeFactory.apply(UTC)), "NULL", TIMESTAMP_TZ_SECONDS, "CAST(NULL AS TIMESTAMP(0) WITH TIME ZONE)")
10451068

10461069
// Since ClickHouse datetime(timezone) does not support values before epoch, we do not test this here.
10471070

10481071
// epoch
1049-
.addRoundTrip(inputTypeFactory.apply(utc), "0", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '1970-01-01 00:00:00 Z'")
1050-
.addRoundTrip(inputTypeFactory.apply(utc), "'1970-01-01 00:00:00'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '1970-01-01 00:00:00 Z'")
1072+
.addRoundTrip(inputTypeFactory.apply(UTC), "0", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '1970-01-01 00:00:00 Z'")
1073+
.addRoundTrip(inputTypeFactory.apply(UTC), "'1970-01-01 00:00:00'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '1970-01-01 00:00:00 Z'")
1074+
// DateTime supports the range [1970-01-01 00:00:00, 2106-02-07 06:28:15]
1075+
// Values outside this range gets stored incorrectly in ClickHouse.
1076+
// For example, 1970-01-01 00:00:00 in Asia/Kathmandu could be stored as 1970-01-01 05:30:00
10511077
.addRoundTrip(inputTypeFactory.apply(KATHMANDU), "'1970-01-01 00:00:00'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '1970-01-01 05:30:00 +05:30'")
10521078

10531079
// after epoch
1054-
.addRoundTrip(inputTypeFactory.apply(utc), "'2019-03-18 10:01:17'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '2019-03-18 10:01:17 Z'")
1080+
.addRoundTrip(inputTypeFactory.apply(UTC), "'2019-03-18 10:01:17'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '2019-03-18 10:01:17 Z'")
10551081
.addRoundTrip(inputTypeFactory.apply(KATHMANDU), "'2019-03-18 10:01:17'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '2019-03-18 10:01:17 +05:45'")
10561082
.addRoundTrip(inputTypeFactory.apply(ZoneId.of("GMT")), "'2019-03-18 10:01:17'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '2019-03-18 10:01:17 Z'")
10571083
.addRoundTrip(inputTypeFactory.apply(ZoneId.of("UTC+00:00")), "'2019-03-18 10:01:17'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '2019-03-18 10:01:17 Z'")
10581084

10591085
// time doubled in JVM zone
1060-
.addRoundTrip(inputTypeFactory.apply(utc), "'2018-10-28 01:33:17'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '2018-10-28 01:33:17 Z'")
1086+
.addRoundTrip(inputTypeFactory.apply(UTC), "'2018-10-28 01:33:17'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '2018-10-28 01:33:17 Z'")
10611087
.addRoundTrip(inputTypeFactory.apply(JVM_ZONE), "'2018-10-28 01:33:17'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '2018-10-28 01:33:17 -05:00'")
10621088
.addRoundTrip(inputTypeFactory.apply(KATHMANDU), "'2018-10-28 01:33:17'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '2018-10-28 01:33:17 +05:45'")
10631089

10641090
// time doubled in Vilnius
1065-
.addRoundTrip(inputTypeFactory.apply(utc), "'2018-10-28 03:33:33'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '2018-10-28 03:33:33 Z'")
1091+
.addRoundTrip(inputTypeFactory.apply(UTC), "'2018-10-28 03:33:33'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '2018-10-28 03:33:33 Z'")
10661092
.addRoundTrip(inputTypeFactory.apply(VILNIUS), "'2018-10-28 03:33:33'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '2018-10-28 03:33:33 +03:00'")
10671093
.addRoundTrip(inputTypeFactory.apply(KATHMANDU), "'2018-10-28 03:33:33'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '2018-10-28 03:33:33 +05:45'")
10681094

10691095
// time gap in JVM zone
1070-
.addRoundTrip(inputTypeFactory.apply(utc), "'1970-01-01 00:13:42'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '1970-01-01 00:13:42 Z'")
1071-
// TODO: Check the range of DateTime(timezone) values written from Trino to ClickHouse to prevent ClickHouse from storing incorrect results.
1072-
// e.g. 1970-01-01 00:13:42 will become 1970-01-01 05:30:00
1073-
// .addRoundTrip(inputTypeFactory.apply(kathmandu), "'1970-01-01 00:13:42'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '1970-01-01 00:13:42 +05:30'")
1074-
.addRoundTrip(inputTypeFactory.apply(utc), "'2018-04-01 02:13:55'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '2018-04-01 02:13:55 Z'")
1096+
.addRoundTrip(inputTypeFactory.apply(UTC), "'1970-01-01 00:13:42'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '1970-01-01 00:13:42 Z'")
1097+
.addRoundTrip(inputTypeFactory.apply(UTC), "'2018-04-01 02:13:55'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '2018-04-01 02:13:55 Z'")
10751098
.addRoundTrip(inputTypeFactory.apply(KATHMANDU), "'2018-04-01 02:13:55'", TIMESTAMP_TZ_SECONDS, "TIMESTAMP '2018-04-01 02:13:55 +05:45'")
10761099

10771100
// time gap in Vilnius

0 commit comments

Comments
 (0)