Use long for representing date value in Accumulo

trinodb · Feb 18, 2022 · ec6553e · ec6553e
1 parent 0ee0eb2
commit ec6553e
Show file tree

Hide file tree

Showing 9 changed files with 23 additions and 72 deletions.
diff --git a/plugin/trino-accumulo/pom.xml b/plugin/trino-accumulo/pom.xml
@@ -114,11 +114,6 @@
             <artifactId>validation-api</artifactId>
         </dependency>
 
-        <dependency>
-            <groupId>joda-time</groupId>
-            <artifactId>joda-time</artifactId>
-        </dependency>
-
         <dependency>
             <groupId>log4j</groupId>
             <artifactId>log4j</artifactId>

diff --git a/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/io/AccumuloRecordCursor.java b/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/io/AccumuloRecordCursor.java
@@ -54,7 +54,6 @@
 import static io.trino.spi.type.TinyintType.TINYINT;
 import static java.lang.String.format;
 import static java.util.Objects.requireNonNull;
-import static java.util.concurrent.TimeUnit.MILLISECONDS;
 
 /**
  * Implementation of Trino RecordCursor, responsible for iterating over a Trino split,
@@ -203,7 +202,7 @@ public long getLong(int field)
             return serializer.getLong(fieldToColumnName[field]);
         }
         if (type.equals(DATE)) {
-            return MILLISECONDS.toDays(serializer.getDate(fieldToColumnName[field]).getTime());
+            return serializer.getDate(fieldToColumnName[field]);
         }
         if (type.equals(INTEGER)) {
             return serializer.getInt(fieldToColumnName[field]);

diff --git a/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/model/Field.java b/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/model/Field.java
@@ -24,10 +24,8 @@
 import io.trino.spi.type.Type;
 import io.trino.spi.type.VarcharType;
 
-import java.sql.Date;
 import java.sql.Time;
 import java.sql.Timestamp;
-import java.time.LocalDate;
 import java.util.Arrays;
 import java.util.Objects;
 
@@ -87,7 +85,7 @@ else if (type.equals(BOOLEAN)) {
             this.value = field.getBoolean();
         }
         else if (type.equals(DATE)) {
-            this.value = new Date(field.getDate().getTime());
+            this.value = field.getDate();
         }
         else if (type.equals(DOUBLE)) {
             this.value = field.getDouble();
@@ -146,9 +144,9 @@ public Byte getByte()
         return (Byte) value;
     }
 
-    public Date getDate()
+    public long getDate()
     {
-        return (Date) value;
+        return (Long) value;
     }
 
     public Double getDouble()
@@ -316,7 +314,8 @@ private static Object convert(Object value, Type type)
         }
 
         if (type.equals(DATE)) {
-            return Date.valueOf(LocalDate.ofEpochDay((long) value));
+            // long
+            return value;
         }
 
         if (type.equals(DOUBLE)) {

diff --git a/...no-accumulo/src/main/java/io/trino/plugin/accumulo/serializers/AccumuloRowSerializer.java b/...no-accumulo/src/main/java/io/trino/plugin/accumulo/serializers/AccumuloRowSerializer.java
@@ -25,7 +25,6 @@
 import org.apache.accumulo.core.data.Value;
 import org.apache.hadoop.io.Text;
 
-import java.sql.Date;
 import java.sql.Time;
 import java.sql.Timestamp;
 import java.util.HashMap;
@@ -168,15 +167,15 @@ static AccumuloRowSerializer getDefault()
      * @param name Column name
      * @return Date value
      */
-    Date getDate(String name);
+    long getDate(String name);
 
     /**
      * Encode the given Date value into the given Text object.
      *
      * @param text Text object to set
      * @param value Value to encode
      */
-    void setDate(Text text, Date value);
+    void setDate(Text text, long value);
 
     /**
      * Gets the Double value of the given Trino column.
@@ -366,7 +365,7 @@ static AccumuloRowSerializer getDefault()
      * </tr>
      * <tr>
      * <td>DATE</td>
-     * <td>java.sql.Date, Long</td>
+     * <td>long</td>
      * </tr>
      * <tr>
      * <td>DOUBLE</td>

diff --git a/...o-accumulo/src/main/java/io/trino/plugin/accumulo/serializers/LexicoderRowSerializer.java b/...o-accumulo/src/main/java/io/trino/plugin/accumulo/serializers/LexicoderRowSerializer.java
@@ -50,7 +50,6 @@
 import static io.trino.spi.type.TinyintType.TINYINT;
 import static io.trino.spi.type.VarbinaryType.VARBINARY;
 import static io.trino.spi.type.VarcharType.VARCHAR;
-import static java.util.concurrent.TimeUnit.DAYS;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 
 /**
@@ -189,13 +188,13 @@ public void setByte(Text text, Byte value)
     }
 
     @Override
-    public Date getDate(String name)
+    public long getDate(String name)
     {
-        return new Date(DAYS.toMillis(decode(BIGINT, getFieldValue(name))));
+        return decode(BIGINT, getFieldValue(name));
     }
 
     @Override
-    public void setDate(Text text, Date value)
+    public void setDate(Text text, long value)
     {
         text.set(encode(DATE, value));
     }

diff --git a/...rino-accumulo/src/main/java/io/trino/plugin/accumulo/serializers/StringRowSerializer.java b/...rino-accumulo/src/main/java/io/trino/plugin/accumulo/serializers/StringRowSerializer.java
@@ -22,7 +22,6 @@
 import org.apache.accumulo.core.data.Value;
 import org.apache.hadoop.io.Text;
 
-import java.sql.Date;
 import java.sql.Time;
 import java.sql.Timestamp;
 import java.util.HashMap;
@@ -45,8 +44,6 @@
 import static io.trino.spi.type.VarcharType.VARCHAR;
 import static java.lang.String.format;
 import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.concurrent.TimeUnit.DAYS;
-import static java.util.concurrent.TimeUnit.MILLISECONDS;
 
 /**
  * Implementation of {@link StringRowSerializer} that encodes and decodes Trino column values as human-readable String objects.
@@ -161,15 +158,15 @@ public void setByte(Text text, Byte value)
     }
 
     @Override
-    public Date getDate(String name)
+    public long getDate(String name)
     {
-        return new Date(DAYS.toMillis(Long.parseLong(getFieldValue(name))));
+        return Long.parseLong(getFieldValue(name));
     }
 
     @Override
-    public void setDate(Text text, Date value)
+    public void setDate(Text text, long value)
     {
-        text.set(Long.toString(MILLISECONDS.toDays(value.getTime())).getBytes(UTF_8));
+        text.set(Long.toString(value).getBytes(UTF_8));
     }
 
     @Override
@@ -317,7 +314,7 @@ else if (type.equals(BOOLEAN)) {
             setBoolean(text, value.equals(Boolean.TRUE));
         }
         else if (type.equals(DATE)) {
-            setDate(text, (Date) value);
+            setDate(text, (long) value);
         }
         else if (type.equals(DOUBLE)) {
             setDouble(text, (Double) value);

diff --git a/plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/TestAccumuloConnectorTest.java b/plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/TestAccumuloConnectorTest.java
@@ -193,30 +193,6 @@ public void testInsertDuplicateRows()
         }
     }
 
-    @Test
-    @Override
-    public void testCreateTableAsSelectNegativeDate()
-    {
-        // TODO (https://github.com/trinodb/trino/issues/10208) Fix negative date handling.
-        assertThatThrownBy(super::testCreateTableAsSelectNegativeDate)
-                .isInstanceOf(AssertionError.class)
-                .hasMessageContaining("" +
-                        "Actual rows (up to 100 of 1 extra rows shown, 1 rows in total):\n" +
-                        "    [-0002-12-31]");
-    }
-
-    @Test
-    @Override
-    public void testInsertNegativeDate()
-    {
-        // TODO (https://github.com/trinodb/trino/issues/10208) Fix negative date handling.
-        assertThatThrownBy(super::testInsertNegativeDate)
-                .isInstanceOf(AssertionError.class)
-                .hasMessageContaining("" +
-                        "Actual rows (up to 100 of 1 extra rows shown, 1 rows in total):\n" +
-                        "    [-0002-12-31]");
-    }
-
     @Override
     public void testShowColumns()
     {
@@ -321,15 +297,6 @@ public void testShowCreateTable()
     protected Optional<DataMappingTestSetup> filterDataMappingSmokeTestData(DataMappingTestSetup dataMappingTestSetup)
     {
         String typeName = dataMappingTestSetup.getTrinoTypeName();
-        if (typeName.equals("date")) {
-            // TODO (https://github.com/trinodb/trino/issues/10074) Investigate why this test case fails
-            if (dataMappingTestSetup.getSampleValueLiteral().equals("DATE '0001-01-01'")
-                    || dataMappingTestSetup.getSampleValueLiteral().equals("DATE '1582-10-04'")
-                    || dataMappingTestSetup.getSampleValueLiteral().equals("DATE '1582-10-05'")
-                    || dataMappingTestSetup.getSampleValueLiteral().equals("DATE '1582-10-14'")) {
-                return Optional.empty();
-            }
-        }
         if (typeName.startsWith("decimal(")
                 || typeName.equals("timestamp(3) with time zone")
                 || typeName.startsWith("char(")) {

diff --git a/plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/model/TestField.java b/plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/model/TestField.java
@@ -24,9 +24,9 @@
 import io.trino.spi.type.TypeSignatureParameter;
 import org.testng.annotations.Test;
 
-import java.sql.Date;
 import java.sql.Time;
 import java.sql.Timestamp;
+import java.time.LocalDate;
 import java.util.GregorianCalendar;
 
 import static io.airlift.slice.Slices.utf8Slice;
@@ -91,7 +91,7 @@ public void testBoolean()
     public void testDate()
     {
         Type type = DATE;
-        Date expected = new Date(new GregorianCalendar(1999, 0, 1).getTime().getTime());
+        long expected = LocalDate.parse("1999-01-01").toEpochDay();
         Field f1 = new Field(10592L, type);
         assertEquals(f1.getDate(), expected);
         assertEquals(f1.getObject(), expected);

diff --git a/...src/test/java/io/trino/plugin/accumulo/serializers/AbstractTestAccumuloRowSerializer.java b/...src/test/java/io/trino/plugin/accumulo/serializers/AbstractTestAccumuloRowSerializer.java
@@ -22,13 +22,11 @@
 import org.apache.accumulo.core.data.Key;
 import org.apache.accumulo.core.data.Mutation;
 import org.apache.accumulo.core.data.Value;
-import org.joda.time.DateTime;
-import org.joda.time.DateTimeZone;
 import org.testng.annotations.Test;
 
-import java.sql.Date;
 import java.sql.Time;
 import java.sql.Timestamp;
+import java.time.LocalDate;
 import java.util.AbstractMap.SimpleImmutableEntry;
 import java.util.List;
 import java.util.Map;
@@ -48,7 +46,6 @@
 import static io.trino.spi.type.VarcharType.VARCHAR;
 import static io.trino.type.InternalTypeManager.TESTING_TYPE_MANAGER;
 import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static org.testng.Assert.assertEquals;
 
 public abstract class AbstractTestAccumuloRowSerializer
@@ -104,15 +101,14 @@ public void testBoolean()
     public void testDate()
             throws Exception
     {
-        Date expected = new Date(new DateTime(2001, 2, 3, 4, 5, 6, DateTimeZone.UTC).getMillis());
+        long expected = LocalDate.parse("2001-02-03").toEpochDay();
         AccumuloRowSerializer serializer = serializerClass.getConstructor().newInstance();
         byte[] data = serializer.encode(DATE, expected);
 
         deserializeData(serializer, data);
-        Date actual = serializer.getDate(COLUMN_NAME);
+        long actual = serializer.getDate(COLUMN_NAME);
 
-        // Convert milliseconds to days so they can be compared regardless of the time of day
-        assertEquals(MILLISECONDS.toDays(actual.getTime()), MILLISECONDS.toDays(expected.getTime()));
+        assertEquals(actual, expected);
     }
 
     @Test