Implement remaining TODOs from TIMESTAMP semantic fix

Fixes #37

Author: dain

client/trino-jdbc/src/test/java/io/trino/jdbc/BaseTestJdbcResultSet.java

@@ -349,11 +349,17 @@ public void testTime()
                         .hasMessage("Expected column to be a timestamp type but is time(3)");
             });
 
-            // TODO https://github.com/trinodb/trino/issues/37
-            // TODO line 1:8: '00:39:05' is not a valid time literal
-//        checkRepresentation(statementWrapper.getStatement(), "TIME '00:39:05'", Types.TIME, (rs, column) -> {
-//            ...
-//        });
+            checkRepresentation(connectedStatement.getStatement(), "TIME '00:39:05'", Types.TIME, (rs, column) -> {
+                assertEquals(rs.getObject(column), toSqlTime(LocalTime.of(0, 39, 5)));
+                assertEquals(rs.getObject(column, Time.class), toSqlTime(LocalTime.of(0, 39, 5)));
+                assertThatThrownBy(() -> rs.getDate(column))
+                        .isInstanceOf(SQLException.class)
+                        .hasMessage("Expected value to be a date but is: 00:39:05");
+                assertEquals(rs.getTime(column), Time.valueOf(LocalTime.of(0, 39, 5)));
+                assertThatThrownBy(() -> rs.getTimestamp(column))
+                        .isInstanceOf(IllegalArgumentException.class) // TODO (https://github.com/trinodb/trino/issues/5315) SQLException
+                        .hasMessage("Expected column to be a timestamp type but is time(0)");
+            });
 
             // second fraction could be overflowing to next millisecond
             checkRepresentation(connectedStatement.getStatement(), "TIME '10:11:12.1235'", Types.TIME, (rs, column) -> {
@@ -573,11 +579,16 @@ public void testTimestamp()
                 assertEquals(rs.getTimestamp(column), Timestamp.valueOf(LocalDateTime.of(1583, 1, 1, 0, 0, 0)));
             });
 
-            // TODO https://github.com/trinodb/trino/issues/37
-            // TODO line 1:8: '1970-01-01 00:14:15.123' is not a valid timestamp literal; the expected values will pro
-//        checkRepresentation(statementWrapper.getStatement(), "TIMESTAMP '1970-01-01 00:14:15.123'", Types.TIMESTAMP, (rs, column) -> {
-//            ...
-//        });
+            checkRepresentation(connectedStatement.getStatement(), "TIMESTAMP '1970-01-01 00:14:15.123'", Types.TIMESTAMP, (rs, column) -> {
+                assertEquals(rs.getObject(column), Timestamp.valueOf(LocalDateTime.of(1970, 1, 1, 0, 14, 15, 123_000_000)));
+                assertThatThrownBy(() -> rs.getDate(column))
+                        .isInstanceOf(SQLException.class)
+                        .hasMessage("Expected value to be a date but is: 1970-01-01 00:14:15.123");
+                assertThatThrownBy(() -> rs.getTime(column))
+                        .isInstanceOf(IllegalArgumentException.class) // TODO (https://github.com/trinodb/trino/issues/5315) SQLException
+                        .hasMessage("Expected column to be a time type but is timestamp(3)");
+                assertEquals(rs.getTimestamp(column), Timestamp.valueOf(LocalDateTime.of(1970, 1, 1, 0, 14, 15, 123_000_000)));
+            });
 
             checkRepresentation(connectedStatement.getStatement(), "TIMESTAMP '123456-01-23 01:23:45.123456789'", Types.TIMESTAMP, (rs, column) -> {
                 assertEquals(rs.getObject(column), Timestamp.valueOf(LocalDateTime.of(123456, 1, 23, 1, 23, 45, 123_456_789)));

plugin/trino-hive/src/main/java/io/trino/plugin/hive/s3select/S3SelectPushdown.java

@@ -61,10 +61,9 @@ public final class S3SelectPushdown
     /*
      * Double and Real Types lose precision. Thus, they are not pushed down to S3. Please use Decimal Type if push down is desired.
      *
-     * Pushing down timestamp to s3select is problematic due to following reasons:
-     * 1) Presto bug: TIMESTAMP behaviour does not match sql standard (https://github.com/trinodb/trino/issues/37)
-     * 2) Presto uses the timezone from client to convert the timestamp if no timezone is provided, however, s3select is a different service and this could lead to unexpected results.
-     * 3) ION SQL compare timestamps using precision, timestamps with different precisions are not equal even actually they present the same instant of time. This could lead to unexpected results.
+     * When S3 select support was added, Trino did not properly implement TIMESTAMP semantic. This was fixed in 2020, and TIMESTAMPS may be supportable now
+     * (https://github.com/trinodb/trino/issues/10962). Pushing down timestamps to s3select maybe still be problematic due to ION SQL comparing timestamps
+     * using precision.  This means timestamps with different precisions are not equal even actually they present the same instant of time.
      */
     private static final Set<String> SUPPORTED_COLUMN_TYPES = ImmutableSet.of(
             BOOLEAN_TYPE_NAME,

testing/trino-tests/src/test/java/io/trino/tests/AbstractTestEngineOnlyQueries.java

@@ -147,13 +147,16 @@ public void testDateLiterals()
     @Test
     public void testTimeLiterals()
     {
+        Session chicago = Session.builder(getSession()).setTimeZoneKey(TimeZoneKey.getTimeZoneKey("America/Chicago")).build();
+        Session kathmandu = Session.builder(getSession()).setTimeZoneKey(TimeZoneKey.getTimeZoneKey("Asia/Kathmandu")).build();
+
         assertEquals(computeScalar("SELECT TIME '3:04:05'"), LocalTime.of(3, 4, 5, 0));
         assertEquals(computeScalar("SELECT TIME '3:04:05.123'"), LocalTime.of(3, 4, 5, 123_000_000));
         assertQuery("SELECT TIME '3:04:05'");
         assertQuery("SELECT TIME '0:04:05'");
-        // TODO https://github.com/trinodb/trino/issues/37
-        // TODO assertQuery(chicago, "SELECT TIME '3:04:05'");
-        // TODO assertQuery(kathmandu, "SELECT TIME '3:04:05'");
+
+        assertQuery(chicago, "SELECT TIME '3:04:05'");
+        assertQuery(kathmandu, "SELECT TIME '3:04:05'");
     }
 
     @Test
@@ -168,13 +171,15 @@ public void testTimeWithTimeZoneLiterals()
     @Test
     public void testTimestampLiterals()
     {
+        Session chicago = Session.builder(getSession()).setTimeZoneKey(TimeZoneKey.getTimeZoneKey("America/Chicago")).build();
+        Session kathmandu = Session.builder(getSession()).setTimeZoneKey(TimeZoneKey.getTimeZoneKey("Asia/Kathmandu")).build();
+
         assertEquals(computeScalar("SELECT TIMESTAMP '1960-01-22 3:04:05'"), LocalDateTime.of(1960, 1, 22, 3, 4, 5));
         assertEquals(computeScalar("SELECT TIMESTAMP '1960-01-22 3:04:05.123'"), LocalDateTime.of(1960, 1, 22, 3, 4, 5, 123_000_000));
         assertQuery("SELECT TIMESTAMP '1960-01-22 3:04:05'");
         assertQuery("SELECT TIMESTAMP '1960-01-22 3:04:05.123'");
-        // TODO https://github.com/trinodb/trino/issues/37
-        // TODO assertQuery(chicago, "SELECT TIMESTAMP '1960-01-22 3:04:05.123'");
-        // TODO assertQuery(kathmandu, "SELECT TIMESTAMP '1960-01-22 3:04:05.123'");
+        assertQuery(chicago, "SELECT TIMESTAMP '1960-01-22 3:04:05.123'");
+        assertQuery(kathmandu, "SELECT TIMESTAMP '1960-01-22 3:04:05.123'");
     }
 
     @Test
@@ -577,7 +582,6 @@ public void testAssignUniqueId()
     @Test
     public void testAtTimeZone()
     {
-        // TODO the expected values here are non-sensical due to https://github.com/trinodb/trino/issues/37
         assertEquals(computeScalar("SELECT TIMESTAMP '2012-10-31 01:00' AT TIME ZONE INTERVAL '07:09' hour to minute"), zonedDateTime("2012-10-30 18:09:00.000 +07:09"));
         assertEquals(computeScalar("SELECT TIMESTAMP '2012-10-31 01:00' AT TIME ZONE 'Asia/Oral'"), zonedDateTime("2012-10-30 16:00:00.000 Asia/Oral"));
         assertEquals(computeScalar("SELECT MIN(x) AT TIME ZONE 'America/Chicago' FROM (VALUES TIMESTAMP '1970-01-01 00:01:00+00:00') t(x)"), zonedDateTime("1969-12-31 18:01:00.000 America/Chicago"));