[flink-table-store] branch master updated: [FLINK-27589] Fix incorrect implementation of TableStoreDateObjectInspector yet again

[lz...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

lzljs3620320 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/flink-table-store.git


The following commit(s) were added to refs/heads/master by this push:
     new bb2879b  [FLINK-27589] Fix incorrect implementation of TableStoreDateObjectInspector yet again
bb2879b is described below

commit bb2879bbbc7be3a3451af33f7bdd03bba0c1a025
Author: tsreaper <ts...@gmail.com>
AuthorDate: Tue May 17 15:42:18 2022 +0800

    [FLINK-27589] Fix incorrect implementation of TableStoreDateObjectInspector yet again
    
    This closes #123
---
 .github/workflows/e2e-tests.yml                    |  7 ++++-
 .github/workflows/java8-build.yml                  |  7 ++++-
 .github/workflows/utils.sh                         | 31 ++++++++++++++++++++++
 .../TableStoreDateObjectInspector.java             |  7 +++--
 .../TableStoreDateObjectInspectorTest.java         |  6 ++---
 .../TableStoreTimestampObjectInspectorTest.java    |  7 +++--
 6 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml
index c7cd479..6998374 100644
--- a/.github/workflows/e2e-tests.yml
+++ b/.github/workflows/e2e-tests.yml
@@ -16,4 +16,9 @@ jobs:
       - name: Build
         run: mvn clean install -DskipTests
       - name: Test
-        run: mvn test -pl flink-table-store-e2e-tests
+        run: |
+          # run tests with random timezone to find out timezone related bugs
+          . .github/workflows/utils.sh
+          jvm_timezone=$(random_timezone)
+          echo "JVM timezone is set to $jvm_timezone"
+          mvn test -pl flink-table-store-e2e-tests -Duser.timezone=$jvm_timezone
diff --git a/.github/workflows/java8-build.yml b/.github/workflows/java8-build.yml
index 26977d5..3138854 100644
--- a/.github/workflows/java8-build.yml
+++ b/.github/workflows/java8-build.yml
@@ -14,4 +14,9 @@ jobs:
         with:
           java-version: 1.8
       - name: Build
-        run: mvn clean install -pl '!flink-table-store-e2e-tests'
+        run: |
+          # run tests with random timezone to find out timezone related bugs
+          . .github/workflows/utils.sh
+          jvm_timezone=$(random_timezone)
+          echo "JVM timezone is set to $jvm_timezone"
+          mvn clean install -pl '!flink-table-store-e2e-tests' -Duser.timezone=$jvm_timezone
diff --git a/.github/workflows/utils.sh b/.github/workflows/utils.sh
new file mode 100644
index 0000000..1b41433
--- /dev/null
+++ b/.github/workflows/utils.sh
@@ -0,0 +1,31 @@
+#!/usr/bin/env bash
+################################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+function random_timezone() {
+    local rnd=$(expr $RANDOM % 25)
+    local hh=$(expr $rnd / 2)
+    local mm=$(expr $rnd % 2 \* 3)"0"
+    local sgn=$(expr $RANDOM % 2)
+    if [ $sgn -eq 0 ]
+    then
+        echo "GMT+$hh:$mm"
+    else
+        echo "GMT-$hh:$mm"
+    fi
+}
diff --git a/flink-table-store-hive/src/main/java/org/apache/flink/table/store/hive/objectinspector/TableStoreDateObjectInspector.java b/flink-table-store-hive/src/main/java/org/apache/flink/table/store/hive/objectinspector/TableStoreDateObjectInspector.java
index 7da6c0b..47ca631 100644
--- a/flink-table-store-hive/src/main/java/org/apache/flink/table/store/hive/objectinspector/TableStoreDateObjectInspector.java
+++ b/flink-table-store-hive/src/main/java/org/apache/flink/table/store/hive/objectinspector/TableStoreDateObjectInspector.java
@@ -18,20 +18,19 @@
 
 package org.apache.flink.table.store.hive.objectinspector;
 
+import org.apache.flink.table.utils.DateTimeUtils;
+
 import org.apache.hadoop.hive.serde2.io.DateWritable;
 import org.apache.hadoop.hive.serde2.objectinspector.primitive.AbstractPrimitiveJavaObjectInspector;
 import org.apache.hadoop.hive.serde2.objectinspector.primitive.DateObjectInspector;
 import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory;
 
 import java.sql.Date;
-import java.time.Duration;
 
 /** {@link AbstractPrimitiveJavaObjectInspector} for DATE type. */
 public class TableStoreDateObjectInspector extends AbstractPrimitiveJavaObjectInspector
         implements DateObjectInspector {
 
-    private static final long MILLIS_PER_DAY = Duration.ofDays(1).toMillis();
-
     public TableStoreDateObjectInspector() {
         super(TypeInfoFactory.dateTypeInfo);
     }
@@ -40,7 +39,7 @@ public class TableStoreDateObjectInspector extends AbstractPrimitiveJavaObjectIn
     public Date getPrimitiveJavaObject(Object o) {
         // Flink stores date as an integer (epoch day, 1970-01-01 = day 0)
         // while constructor of Date accepts epoch millis
-        return o == null ? null : new Date(((Integer) o) * MILLIS_PER_DAY);
+        return o == null ? null : DateTimeUtils.toSQLDate((Integer) o);
     }
 
     @Override
diff --git a/flink-table-store-hive/src/test/java/org/apache/flink/table/store/hive/objectinspector/TableStoreDateObjectInspectorTest.java b/flink-table-store-hive/src/test/java/org/apache/flink/table/store/hive/objectinspector/TableStoreDateObjectInspectorTest.java
index 76f247e..4c232cd 100644
--- a/flink-table-store-hive/src/test/java/org/apache/flink/table/store/hive/objectinspector/TableStoreDateObjectInspectorTest.java
+++ b/flink-table-store-hive/src/test/java/org/apache/flink/table/store/hive/objectinspector/TableStoreDateObjectInspectorTest.java
@@ -24,15 +24,13 @@ import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector;
 import org.junit.jupiter.api.Test;
 
 import java.sql.Date;
-import java.time.Duration;
+import java.time.LocalDate;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
 /** Tests for {@link TableStoreDateObjectInspector}. */
 public class TableStoreDateObjectInspectorTest {
 
-    private static final long MILLIS_PER_DAY = Duration.ofDays(1).toMillis();
-
     @Test
     public void testCategoryAndClass() {
         TableStoreDateObjectInspector oi = new TableStoreDateObjectInspector();
@@ -67,7 +65,7 @@ public class TableStoreDateObjectInspectorTest {
     public void testCopyObject() {
         TableStoreDateObjectInspector oi = new TableStoreDateObjectInspector();
 
-        Date input = new Date(375 * MILLIS_PER_DAY);
+        Date input = Date.valueOf(LocalDate.ofEpochDay(375));
         Object copy = oi.copyObject(input);
         assertThat(copy).isEqualTo(input);
         assertThat(copy).isNotSameAs(input);
diff --git a/flink-table-store-hive/src/test/java/org/apache/flink/table/store/hive/objectinspector/TableStoreTimestampObjectInspectorTest.java b/flink-table-store-hive/src/test/java/org/apache/flink/table/store/hive/objectinspector/TableStoreTimestampObjectInspectorTest.java
index 86a7359..ba1f675 100644
--- a/flink-table-store-hive/src/test/java/org/apache/flink/table/store/hive/objectinspector/TableStoreTimestampObjectInspectorTest.java
+++ b/flink-table-store-hive/src/test/java/org/apache/flink/table/store/hive/objectinspector/TableStoreTimestampObjectInspectorTest.java
@@ -51,8 +51,7 @@ public class TableStoreTimestampObjectInspectorTest {
 
         LocalDateTime local = LocalDateTime.of(2022, 4, 27, 15, 0, 0);
         TimestampData input = TimestampData.fromLocalDateTime(local);
-        Timestamp expected = Timestamp.valueOf(local);
-        assertThat(oi.getPrimitiveJavaObject(input)).isEqualTo(expected);
+        assertThat(oi.getPrimitiveJavaObject(input).toString()).isEqualTo("2022-04-27 15:00:00.0");
         assertThat(oi.getPrimitiveJavaObject(null)).isNull();
     }
 
@@ -62,8 +61,8 @@ public class TableStoreTimestampObjectInspectorTest {
 
         LocalDateTime local = LocalDateTime.of(2022, 4, 27, 15, 0, 0);
         TimestampData input = TimestampData.fromLocalDateTime(local);
-        TimestampWritable expected = new TimestampWritable(Timestamp.valueOf(local));
-        assertThat(oi.getPrimitiveWritableObject(input)).isEqualTo(expected);
+        assertThat(oi.getPrimitiveWritableObject(input).getTimestamp().toString())
+                .isEqualTo("2022-04-27 15:00:00.0");
         assertThat(oi.getPrimitiveWritableObject(null)).isNull();
     }