diff --git a/plugin/trino-memsql/src/main/java/io/trino/plugin/memsql/MemSqlClient.java b/plugin/trino-memsql/src/main/java/io/trino/plugin/memsql/MemSqlClient.java index bbbfab0aa9a1..bd1dc4393b06 100644 --- a/plugin/trino-memsql/src/main/java/io/trino/plugin/memsql/MemSqlClient.java +++ b/plugin/trino-memsql/src/main/java/io/trino/plugin/memsql/MemSqlClient.java @@ -44,6 +44,7 @@ import io.trino.spi.type.DecimalType; import io.trino.spi.type.Decimals; import io.trino.spi.type.StandardTypes; +import io.trino.spi.type.TimeType; import io.trino.spi.type.TimestampType; import io.trino.spi.type.Type; import io.trino.spi.type.TypeManager; @@ -99,7 +100,8 @@ import static io.trino.plugin.jdbc.StandardColumnMappings.shortDecimalWriteFunction; import static io.trino.plugin.jdbc.StandardColumnMappings.smallintColumnMapping; import static io.trino.plugin.jdbc.StandardColumnMappings.smallintWriteFunction; -import static io.trino.plugin.jdbc.StandardColumnMappings.timeColumnMappingUsingSqlTime; +import static io.trino.plugin.jdbc.StandardColumnMappings.timeColumnMapping; +import static io.trino.plugin.jdbc.StandardColumnMappings.timeWriteFunction; import static io.trino.plugin.jdbc.StandardColumnMappings.timestampColumnMapping; import static io.trino.plugin.jdbc.StandardColumnMappings.timestampWriteFunction; import static io.trino.plugin.jdbc.StandardColumnMappings.tinyintColumnMapping; @@ -119,6 +121,7 @@ import static io.trino.spi.type.IntegerType.INTEGER; import static io.trino.spi.type.RealType.REAL; import static io.trino.spi.type.SmallintType.SMALLINT; +import static io.trino.spi.type.TimeType.createTimeType; import static io.trino.spi.type.TimestampType.TIMESTAMP_MICROS; import static io.trino.spi.type.TimestampType.createTimestampType; import static io.trino.spi.type.TinyintType.TINYINT; @@ -275,7 +278,7 @@ private static Map getTimestampPrecisions(Connection connection "FROM information_schema.columns " + "WHERE table_schema = ? " + "AND table_name = ? " + - "AND column_type IN ('datetime', 'datetime(6)', 'timestamp', 'timestamp(6)')"; + "AND column_type IN ('datetime', 'datetime(6)', 'time', 'time(6)', 'timestamp', 'timestamp(6)')"; try (PreparedStatement statement = connection.prepareStatement(sql)) { statement.setString(1, tableHandle.getCatalogName()); statement.setString(2, tableHandle.getTableName()); @@ -284,7 +287,7 @@ private static Map getTimestampPrecisions(Connection connection try (ResultSet resultSet = statement.executeQuery()) { while (resultSet.next()) { String columnType = resultSet.getString("column_type"); - int size = columnType.equals("datetime") || columnType.equals("timestamp") ? 0 : 6; + int size = columnType.equals("datetime") || columnType.equals("time") || columnType.equals("timestamp") ? 0 : 6; timestampColumnPrecisions.put(resultSet.getString("column_name"), size); } } @@ -360,8 +363,8 @@ public Optional toColumnMapping(ConnectorSession session, Connect dateReadFunctionUsingLocalDate(), dateWriteFunction())); case Types.TIME: - // TODO (https://github.com/trinodb/trino/issues/5450) Fix TIME type mapping - return Optional.of(timeColumnMappingUsingSqlTime()); + TimeType timeType = createTimeType(typeHandle.getRequiredDecimalDigits()); + return Optional.of(timeColumnMapping(timeType)); case Types.TIMESTAMP: // TODO (https://github.com/trinodb/trino/issues/5450) Fix DST handling TimestampType timestampType = createTimestampType(typeHandle.getRequiredDecimalDigits()); @@ -488,6 +491,14 @@ else if (varcharType.getBoundedLength() <= MEMSQL_MEDIUMTEXT_MAX_LENGTH) { if (type == DATE) { return WriteMapping.longMapping("date", dateWriteFunction()); } + if (type instanceof TimeType) { + TimeType timeType = (TimeType) type; + checkArgument(timeType.getPrecision() <= MEMSQL_DATE_TIME_MAX_PRECISION, "The max time precision in MemSQL is 6"); + if (timeType.getPrecision() == 0) { + return WriteMapping.longMapping("time", timeWriteFunction(0)); + } + return WriteMapping.longMapping("time(6)", timeWriteFunction(6)); + } // TODO implement TIME type if (type instanceof TimestampType) { TimestampType timestampType = (TimestampType) type; diff --git a/plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlConnectorTest.java b/plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlConnectorTest.java index 73c687df10b4..21198c4a469b 100644 --- a/plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlConnectorTest.java +++ b/plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlConnectorTest.java @@ -127,8 +127,13 @@ protected Optional filterDataMappingSmokeTestData(DataMapp return Optional.empty(); } - if (typeName.equals("time") - || typeName.equals("timestamp(3) with time zone")) { + if (typeName.equals("time")) { + // MemSQL supports only second precision + // Skip 'time' that is alias of time(3) here and add test cases in TestMemSqlTypeMapping.testTime instead + return Optional.empty(); + } + + if (typeName.equals("timestamp(3) with time zone")) { return Optional.of(dataMappingTestSetup.asUnsupported()); } diff --git a/plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlTypeMapping.java b/plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlTypeMapping.java index 72b14e7ffdc8..16ab2a5e4cf3 100644 --- a/plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlTypeMapping.java +++ b/plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlTypeMapping.java @@ -61,6 +61,8 @@ import static io.trino.spi.type.IntegerType.INTEGER; import static io.trino.spi.type.RealType.REAL; import static io.trino.spi.type.SmallintType.SMALLINT; +import static io.trino.spi.type.TimeType.TIME_MICROS; +import static io.trino.spi.type.TimeType.TIME_SECONDS; import static io.trino.spi.type.TimeZoneKey.UTC_KEY; import static io.trino.spi.type.TimeZoneKey.getTimeZoneKey; import static io.trino.spi.type.TimestampType.createTimestampType; @@ -582,6 +584,93 @@ private DataTypeTest dateTestCases(DataType dateDataType, ZoneId jvmZ .addRoundTrip(dateDataType, dateOfLocalTimeChangeBackwardAtMidnightInSomeZone); } + @Test(dataProvider = "sessionZonesDataProvider") + public void testTime(ZoneId sessionZone) + { + Session session = Session.builder(getSession()) + .setTimeZoneKey(getTimeZoneKey(sessionZone.getId())) + .build(); + + SqlDataTypeTest.create() + .addRoundTrip("time", "TIME '00:00:00'", TIME_MICROS, "TIME '00:00:00.000000'") // default to micro second (same as timestamp) in Trino + .addRoundTrip("time(0)", "NULL", TIME_SECONDS, "CAST(NULL AS time(0))") + .addRoundTrip("time(0)", "TIME '00:00:00'", TIME_SECONDS, "TIME '00:00:00'") + .addRoundTrip("time(0)", "TIME '01:02:03'", TIME_SECONDS, "TIME '01:02:03'") + .addRoundTrip("time(0)", "TIME '23:59:59'", TIME_SECONDS, "TIME '23:59:59'") + .addRoundTrip("time(0)", "TIME '23:59:59.9'", TIME_SECONDS, "TIME '00:00:00'") // round by engine + .addRoundTrip("time(6)", "NULL", TIME_MICROS, "CAST(NULL AS time(6))") + .addRoundTrip("time(6)", "TIME '00:00:00'", TIME_MICROS, "TIME '00:00:00.000000'") + .addRoundTrip("time(6)", "TIME '01:02:03'", TIME_MICROS, "TIME '01:02:03.000000'") + .addRoundTrip("time(6)", "TIME '23:59:59'", TIME_MICROS, "TIME '23:59:59.000000'") + .addRoundTrip("time(6)", "TIME '23:59:59.9'", TIME_MICROS, "TIME '23:59:59.900000'") + .addRoundTrip("time(6)", "TIME '23:59:59.99'", TIME_MICROS, "TIME '23:59:59.990000'") + .addRoundTrip("time(6)", "TIME '23:59:59.999'", TIME_MICROS, "TIME '23:59:59.999000'") + .addRoundTrip("time(6)", "TIME '23:59:59.9999'", TIME_MICROS, "TIME '23:59:59.999900'") + .addRoundTrip("time(6)", "TIME '23:59:59.99999'", TIME_MICROS, "TIME '23:59:59.999990'") + .addRoundTrip("time(6)", "TIME '00:00:00.000000'", TIME_MICROS, "TIME '00:00:00.000000'") + .addRoundTrip("time(6)", "TIME '01:02:03.123456'", TIME_MICROS, "TIME '01:02:03.123456'") + .addRoundTrip("time(6)", "TIME '23:59:59.999999'", TIME_MICROS, "TIME '23:59:59.999999'") + .addRoundTrip("time(6)", "TIME '00:00:00.000000'", TIME_MICROS, "TIME '00:00:00.000000'") // round by engine + .execute(getQueryRunner(), session, trinoCreateAsSelect("tpch.test_time")) + .execute(getQueryRunner(), session, trinoCreateAndInsert(getSession(), "tpch.test_time")); + + SqlDataTypeTest.create() + .addRoundTrip("time", "NULL", TIME_SECONDS, "CAST(NULL AS time(0))") // default to second in MemSQL + .addRoundTrip("time", "'00:00:00'", TIME_SECONDS, "TIME '00:00:00'") + .addRoundTrip("time", "'01:02:03'", TIME_SECONDS, "TIME '01:02:03'") + .addRoundTrip("time", "'23:59:59'", TIME_SECONDS, "TIME '23:59:59'") + .addRoundTrip("time", "'23:59:59.9'", TIME_SECONDS, "TIME '23:59:59'") // MemSQL ignores millis and stores only seconds in 'time' type + .addRoundTrip("time(6)", "NULL", TIME_MICROS, "CAST(NULL AS time(6))") + .addRoundTrip("time(6)", "'00:00:00'", TIME_MICROS, "TIME '00:00:00.000000'") + .addRoundTrip("time(6)", "'01:02:03'", TIME_MICROS, "TIME '01:02:03.000000'") + .addRoundTrip("time(6)", "'23:59:59'", TIME_MICROS, "TIME '23:59:59.000000'") + .addRoundTrip("time(6)", "'23:59:59.9'", TIME_MICROS, "TIME '23:59:59.900000'") + .addRoundTrip("time(6)", "'23:59:59.99'", TIME_MICROS, "TIME '23:59:59.990000'") + .addRoundTrip("time(6)", "'23:59:59.999'", TIME_MICROS, "TIME '23:59:59.999000'") + .addRoundTrip("time(6)", "'23:59:59.9999'", TIME_MICROS, "TIME '23:59:59.999900'") + .addRoundTrip("time(6)", "'23:59:59.99999'", TIME_MICROS, "TIME '23:59:59.999990'") + .addRoundTrip("time(6)", "'00:00:00.000000'", TIME_MICROS, "TIME '00:00:00.000000'") + .addRoundTrip("time(6)", "'01:02:03.123456'", TIME_MICROS, "TIME '01:02:03.123456'") + .addRoundTrip("time(6)", "'23:59:59.999999'", TIME_MICROS, "TIME '23:59:59.999999'") + .addRoundTrip("time(6)", "'23:59:59.9999999'", TIME_MICROS, "TIME '23:59:59.999999'") // MemSQL ignores nanos and stores only micros in 'time(6)' type + .execute(getQueryRunner(), session, memSqlCreateAndInsert("tpch.test_time")); + } + + @Test(dataProvider = "unsupportedTimeDataProvider") + public void testUnsupportedTime(String unsupportedTime) + { + try (TestTable table = new TestTable(memSqlServer::execute, "tpch.test_unsupported_time", "(col time)", ImmutableList.of(format("'%s'", unsupportedTime)))) { + assertQueryFails( + "SELECT * FROM " + table.getName(), + format("\\Q%s cannot be parse as LocalTime (format is \"HH:mm:ss[.S]\" for data type \"TIME\")", unsupportedTime)); + } + + try (TestTable table = new TestTable(memSqlServer::execute, "tpch.test_unsupported_time", "(col time(6))", ImmutableList.of(format("'%s'", unsupportedTime)))) { + assertQueryFails( + "SELECT * FROM " + table.getName(), + format("\\Q%s.000000 cannot be parse as LocalTime (format is \"HH:mm:ss[.S]\" for data type \"TIME\")", unsupportedTime)); + } + } + + @DataProvider + public Object[][] unsupportedTimeDataProvider() + { + return new Object[][] { + {"-838:59:59"}, // min value in MemSQL + {"-00:00:01"}, + {"24:00:00"}, + {"838:59:59"}, // max value in MemSQL + }; + } + + @Test(dataProvider = "unsupportedDateTimePrecisions") + public void testUnsupportedTimePrecision(int precision) + { + // This test should be fixed if future MemSQL supports those precisions + assertThatThrownBy(() -> memSqlServer.execute(format("CREATE TABLE test_unsupported_timestamp_precision (col1 TIME(%s))", precision))) + .hasMessageContaining("Feature 'TIME type with precision other than 0 or 6' is not supported by MemSQL."); + } + @Test(dataProvider = "sessionZonesDataProvider") public void testDatetime(ZoneId sessionZone) {