Fix timestamp semantics

trinodb · Aug 13, 2020 · e9a6ff9 · e9a6ff9
1 parent ddee904
commit e9a6ff9
Show file tree

Hide file tree

Showing 275 changed files with 8,346 additions and 10,345 deletions.
diff --git a/pom.xml b/pom.xml
@@ -497,7 +497,7 @@
             <dependency>
                 <groupId>io.prestosql.hive</groupId>
                 <artifactId>hive-apache</artifactId>
-                <version>3.0.0-6</version>
+                <version>3.1.2-1</version>
             </dependency>
 
             <dependency>

diff --git a/presto-accumulo/src/main/java/io/prestosql/plugin/accumulo/model/Field.java b/presto-accumulo/src/main/java/io/prestosql/plugin/accumulo/model/Field.java
@@ -331,14 +331,10 @@ private static Object convert(Object value, Type type)
         }
 
         if (type.equals(TIME)) {
-            // TODO this likely is incorrect, passing the millis value interpreted in UTC into millis value interpreted in JVM's zone
-            // TODO account for non-legacy timestamp
             return new Time((long) value);
         }
 
         if (type.equals(TIMESTAMP)) {
-            // TODO this likely is incorrect, passing the millis value interpreted in UTC into millis value interpreted in JVM's zone
-            // TODO account for non-legacy timestamp
             return new Timestamp((long) value);
         }
 

diff --git a/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/BaseJdbcClient.java b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/BaseJdbcClient.java
@@ -325,7 +325,7 @@ public Optional<ColumnMapping> toPrestoType(ConnectorSession session, Connection
         if (mapping.isPresent()) {
             return mapping;
         }
-        Optional<ColumnMapping> connectorMapping = jdbcTypeToPrestoType(session, typeHandle);
+        Optional<ColumnMapping> connectorMapping = jdbcTypeToPrestoType(typeHandle);
         if (connectorMapping.isPresent()) {
             return connectorMapping;
         }

diff --git a/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/StandardColumnMappings.java b/presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/StandardColumnMappings.java
@@ -16,7 +16,6 @@
 import com.google.common.base.CharMatcher;
 import com.google.common.primitives.Shorts;
 import com.google.common.primitives.SignedBytes;
-import io.prestosql.spi.connector.ConnectorSession;
 import io.prestosql.spi.type.CharType;
 import io.prestosql.spi.type.DecimalType;
 import io.prestosql.spi.type.Decimals;
@@ -37,7 +36,6 @@
 import java.time.Instant;
 import java.time.LocalDateTime;
 import java.time.LocalTime;
-import java.time.ZoneId;
 import java.util.Optional;
 
 import static com.google.common.base.Preconditions.checkArgument;
@@ -351,42 +349,27 @@ public static LongWriteFunction timeWriteFunction()
      * {@link #timestampColumnMapping} instead.
      */
     @Deprecated
-    public static ColumnMapping timestampColumnMappingUsingSqlTimestamp(ConnectorSession session)
+    public static ColumnMapping timestampColumnMappingUsingSqlTimestamp()
     {
-        if (session.isLegacyTimestamp()) {
-            ZoneId sessionZone = ZoneId.of(session.getTimeZoneKey().getId());
-            return ColumnMapping.longMapping(
-                    TIMESTAMP,
-                    (resultSet, columnIndex) -> {
-                        Timestamp timestamp = resultSet.getTimestamp(columnIndex);
-                        return toPrestoLegacyTimestamp(timestamp.toLocalDateTime(), sessionZone);
-                    },
-                    timestampWriteFunctionUsingSqlTimestamp(session));
-        }
-
         return ColumnMapping.longMapping(
                 TIMESTAMP,
                 (resultSet, columnIndex) -> {
                     Timestamp timestamp = resultSet.getTimestamp(columnIndex);
                     return toPrestoTimestamp(timestamp.toLocalDateTime());
                 },
-                timestampWriteFunctionUsingSqlTimestamp(session));
+                timestampWriteFunctionUsingSqlTimestamp());
     }
 
-    public static ColumnMapping timestampColumnMapping(ConnectorSession session)
+    public static ColumnMapping timestampColumnMapping()
     {
         return ColumnMapping.longMapping(
                 TIMESTAMP,
-                timestampReadFunction(session),
-                timestampWriteFunction(session));
+                timestampReadFunction(),
+                timestampWriteFunction());
     }
 
-    public static LongReadFunction timestampReadFunction(ConnectorSession session)
+    public static LongReadFunction timestampReadFunction()
     {
-        if (session.isLegacyTimestamp()) {
-            ZoneId sessionZone = ZoneId.of(session.getTimeZoneKey().getId());
-            return (resultSet, columnIndex) -> toPrestoLegacyTimestamp(resultSet.getObject(columnIndex, LocalDateTime.class), sessionZone);
-        }
         return (resultSet, columnIndex) -> toPrestoTimestamp(resultSet.getObject(columnIndex, LocalDateTime.class));
     }
 
@@ -397,49 +380,21 @@ public static LongReadFunction timestampReadFunction(ConnectorSession session)
      * {@link #timestampWriteFunction} instead.
      */
     @Deprecated
-    public static LongWriteFunction timestampWriteFunctionUsingSqlTimestamp(ConnectorSession connectorSession)
+    public static LongWriteFunction timestampWriteFunctionUsingSqlTimestamp()
     {
-        if (connectorSession.isLegacyTimestamp()) {
-            ZoneId sessionZone = ZoneId.of(connectorSession.getTimeZoneKey().getId());
-            return (statement, index, value) -> statement.setTimestamp(index, Timestamp.valueOf(fromPrestoLegacyTimestamp(value, sessionZone)));
-        }
         return (statement, index, value) -> statement.setTimestamp(index, Timestamp.valueOf(fromPrestoTimestamp(value)));
     }
 
-    public static LongWriteFunction timestampWriteFunction(ConnectorSession session)
-    {
-        if (session.isLegacyTimestamp()) {
-            ZoneId sessionZone = ZoneId.of(session.getTimeZoneKey().getId());
-            return (statement, index, value) -> statement.setObject(index, fromPrestoLegacyTimestamp(value, sessionZone));
-        }
-        return (statement, index, value) -> {
-            statement.setObject(index, fromPrestoTimestamp(value));
-        };
-    }
-
-    /**
-     * @deprecated applicable in legacy timestamp semantics only
-     */
-    @Deprecated
-    public static long toPrestoLegacyTimestamp(LocalDateTime localDateTime, ZoneId sessionZone)
+    public static LongWriteFunction timestampWriteFunction()
     {
-        return localDateTime.atZone(sessionZone).toInstant().toEpochMilli();
+        return (statement, index, value) -> statement.setObject(index, fromPrestoTimestamp(value));
     }
 
     public static long toPrestoTimestamp(LocalDateTime localDateTime)
     {
         return localDateTime.atZone(UTC).toInstant().toEpochMilli();
     }
 
-    /**
-     * @deprecated applicable in legacy timestamp semantics only
-     */
-    @Deprecated
-    public static LocalDateTime fromPrestoLegacyTimestamp(long value, ZoneId sessionZone)
-    {
-        return Instant.ofEpochMilli(value).atZone(sessionZone).toLocalDateTime();
-    }
-
     public static LocalDateTime fromPrestoTimestamp(long value)
     {
         return Instant.ofEpochMilli(value).atZone(UTC).toLocalDateTime();
@@ -451,7 +406,7 @@ public static LocalTime fromPrestoTime(long value)
         return LocalTime.ofNanoOfDay(roundDiv(value, PICOSECONDS_PER_NANOSECOND) % NANOSECONDS_PER_DAY);
     }
 
-    public static Optional<ColumnMapping> jdbcTypeToPrestoType(ConnectorSession session, JdbcTypeHandle type)
+    public static Optional<ColumnMapping> jdbcTypeToPrestoType(JdbcTypeHandle type)
     {
         int columnSize = type.getColumnSize();
         switch (type.getJdbcType()) {
@@ -516,7 +471,7 @@ public static Optional<ColumnMapping> jdbcTypeToPrestoType(ConnectorSession sess
 
             case Types.TIMESTAMP:
                 // TODO default to `timestampColumnMapping`
-                return Optional.of(timestampColumnMappingUsingSqlTimestamp(session));
+                return Optional.of(timestampColumnMappingUsingSqlTimestamp());
         }
         return Optional.empty();
     }

diff --git a/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/TestJdbcQueryBuilder.java b/presto-base-jdbc/src/test/java/io/prestosql/plugin/jdbc/TestJdbcQueryBuilder.java
@@ -26,8 +26,6 @@
 import io.prestosql.spi.predicate.ValueSet;
 import io.prestosql.spi.type.CharType;
 import io.prestosql.spi.type.SqlTime;
-import io.prestosql.spi.type.SqlTimestamp;
-import io.prestosql.testing.DateTimeTestingUtils;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
@@ -70,18 +68,17 @@
 import static io.prestosql.spi.type.RealType.REAL;
 import static io.prestosql.spi.type.SmallintType.SMALLINT;
 import static io.prestosql.spi.type.TimeType.TIME;
-import static io.prestosql.spi.type.TimeZoneKey.UTC_KEY;
 import static io.prestosql.spi.type.TimestampType.TIMESTAMP;
 import static io.prestosql.spi.type.TinyintType.TINYINT;
 import static io.prestosql.spi.type.VarcharType.VARCHAR;
 import static io.prestosql.testing.DateTimeTestingUtils.sqlTimeOf;
+import static io.prestosql.testing.DateTimeTestingUtils.sqlTimestampOf;
 import static io.prestosql.testing.TestingConnectorSession.SESSION;
 import static java.lang.Float.floatToRawIntBits;
 import static java.lang.String.format;
 import static java.time.temporal.ChronoUnit.DAYS;
 import static java.util.function.Function.identity;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.joda.time.DateTimeZone.UTC;
 import static org.testng.Assert.assertEquals;
 
 @Test(singleThreaded = true)
@@ -512,11 +509,7 @@ public void testAggregationWithFilter()
 
     private static long toPrestoTimestamp(int year, int month, int day, int hour, int minute, int second)
     {
-        SqlTimestamp sqlTimestamp = DateTimeTestingUtils.sqlTimestampOf(3, year, month, day, hour, minute, second, 0, UTC, UTC_KEY, SESSION);
-        if (SESSION.isLegacyTimestamp()) {
-            return sqlTimestamp.getMillisUtc();
-        }
-        return sqlTimestamp.getMillis();
+        return sqlTimestampOf(3, year, month, day, hour, minute, second, 0).getMillis();
     }
 
     private static Timestamp toTimestamp(int year, int month, int day, int hour, int minute, int second)

diff --git a/presto-blackhole/src/test/java/io/prestosql/plugin/blackhole/TestBlackHoleSmoke.java b/presto-blackhole/src/test/java/io/prestosql/plugin/blackhole/TestBlackHoleSmoke.java
@@ -264,8 +264,7 @@ public void testSelectAllTypes()
         assertEquals(row.getField(6), 0.0);
         assertEquals(row.getField(7), false);
         assertEquals(row.getField(8), LocalDate.ofEpochDay(0));
-        // TODO should be 1970-01-01 00:00:00 after https://github.com/prestosql/presto/issues/37
-        assertEquals(row.getField(9), LocalDateTime.of(1969, 12, 31, 13, 0, 0));
+        assertEquals(row.getField(9), LocalDateTime.of(1970, 1, 1, 0, 0, 0));
         assertEquals(row.getField(10), "****************".getBytes(UTF_8));
         assertEquals(row.getField(11), new BigDecimal("0.00"));
         assertEquals(row.getField(12), new BigDecimal("00000000000000000000.0000000000"));

diff --git a/presto-docs/src/main/sphinx/language.rst b/presto-docs/src/main/sphinx/language.rst
@@ -22,4 +22,3 @@ Presto also provides :doc:`numerous SQL functions and operators<functions>`.
 
     language/types
     language/reserved
-    language/timestamp
diff --git a/presto-docs/src/main/sphinx/language/timestamp.rst b/presto-docs/src/main/sphinx/language/timestamp.rst
diff --git a/presto-docs/src/main/sphinx/language/types.rst b/presto-docs/src/main/sphinx/language/types.rst
@@ -141,7 +141,7 @@ JSON value type, which can be a JSON object, a JSON array, a JSON number, a JSON
 Date and Time
 -------------
 
-See also :doc:`/functions/datetime` and :doc:`/language/timestamp`
+See also :doc:`/functions/datetime`
 
 ``DATE``
 ^^^^^^^^

diff --git a/presto-elasticsearch/src/main/java/io/prestosql/elasticsearch/CountQueryPageSource.java b/presto-elasticsearch/src/main/java/io/prestosql/elasticsearch/CountQueryPageSource.java
@@ -16,7 +16,6 @@
 import io.prestosql.elasticsearch.client.ElasticsearchClient;
 import io.prestosql.spi.Page;
 import io.prestosql.spi.connector.ConnectorPageSource;
-import io.prestosql.spi.connector.ConnectorSession;
 
 import static io.prestosql.elasticsearch.ElasticsearchQueryBuilder.buildSearchQuery;
 import static java.lang.Math.toIntExact;
@@ -33,18 +32,17 @@ class CountQueryPageSource
     private final long readTimeNanos;
     private long remaining;
 
-    public CountQueryPageSource(ElasticsearchClient client, ConnectorSession session, ElasticsearchTableHandle table, ElasticsearchSplit split)
+    public CountQueryPageSource(ElasticsearchClient client, ElasticsearchTableHandle table, ElasticsearchSplit split)
     {
         requireNonNull(client, "client is null");
-        requireNonNull(session, "session is null");
         requireNonNull(table, "table is null");
         requireNonNull(split, "split is null");
 
         long start = System.nanoTime();
         long count = client.count(
                 split.getIndex(),
                 split.getShard(),
-                buildSearchQuery(session, table.getConstraint().transform(ElasticsearchColumnHandle.class::cast), table.getQuery()));
+                buildSearchQuery(table.getConstraint().transform(ElasticsearchColumnHandle.class::cast), table.getQuery()));
         readTimeNanos = System.nanoTime() - start;
 
         if (table.getLimit().isPresent()) {

diff --git a/...asticsearch/src/main/java/io/prestosql/elasticsearch/ElasticsearchPageSourceProvider.java b/...asticsearch/src/main/java/io/prestosql/elasticsearch/ElasticsearchPageSourceProvider.java
@@ -62,12 +62,11 @@ public ConnectorPageSource createPageSource(
         }
 
         if (columns.isEmpty()) {
-            return new CountQueryPageSource(client, session, elasticsearchTable, elasticsearchSplit);
+            return new CountQueryPageSource(client, elasticsearchTable, elasticsearchSplit);
         }
 
         return new ScanQueryPageSource(
                 client,
-                session,
                 elasticsearchTable,
                 elasticsearchSplit,
                 columns.stream()