SNOW-928973: Date field is returning one day less when getting throug…

…h getString method (#1715)
snowflakedb · Jul 22, 2024 · 2843788 · 2843788
1 parent 6c2485d
commit 2843788
Show file tree

Hide file tree

Showing 9 changed files with 179 additions and 22 deletions.
diff --git a/src/main/java/net/snowflake/client/core/SFBaseSession.java b/src/main/java/net/snowflake/client/core/SFBaseSession.java
@@ -79,6 +79,8 @@ public abstract class SFBaseSession {
   private boolean enableCombineDescribe;
   private boolean clientTelemetryEnabled = false;
   private boolean useSessionTimezone;
+  private boolean defaultFormatDateWithTimezone = true;
+  private boolean getDateUseNullTimezone = true;
   // The server can read array binds from a stage instead of query payload.
   // When there as many bind values as this threshold, we should upload them to a stage.
   private int arrayBindStageThreshold = 0;
@@ -679,10 +681,26 @@ public boolean getUseSessionTimezone() {
     return useSessionTimezone;
   }
 
+  public boolean getDefaultFormatDateWithTimezone() {
+    return defaultFormatDateWithTimezone;
+  }
+
   public void setUseSessionTimezone(boolean useSessionTimezone) {
     this.useSessionTimezone = useSessionTimezone;
   }
 
+  public void setDefaultFormatDateWithTimezone(boolean defaultFormatDateWithTimezone) {
+    this.defaultFormatDateWithTimezone = defaultFormatDateWithTimezone;
+  }
+
+  public boolean getGetDateUseNullTimezone() {
+    return getDateUseNullTimezone;
+  }
+
+  public void setGetDateUseNullTimezone(boolean getDateUseNullTimezone) {
+    this.getDateUseNullTimezone = getDateUseNullTimezone;
+  }
+
   public boolean getEnableCombineDescribe() {
     return enableCombineDescribe;
   }

diff --git a/src/main/java/net/snowflake/client/core/SFSession.java b/src/main/java/net/snowflake/client/core/SFSession.java
@@ -489,6 +489,18 @@ public void addSFSessionProperty(String propertyName, Object propertyValue) thro
           }
           break;
 
+        case JDBC_DEFAULT_FORMAT_DATE_WITH_TIMEZONE:
+          if (propertyValue != null) {
+            setDefaultFormatDateWithTimezone(getBooleanValue(propertyValue));
+          }
+          break;
+
+        case JDBC_GET_DATE_USE_NULL_TIMEZONE:
+          if (propertyValue != null) {
+            setGetDateUseNullTimezone(getBooleanValue(propertyValue));
+          }
+          break;
+
         default:
           break;
       }

diff --git a/src/main/java/net/snowflake/client/core/SFSessionProperty.java b/src/main/java/net/snowflake/client/core/SFSessionProperty.java
@@ -85,7 +85,16 @@ public enum SFSessionProperty {
   DISABLE_GCS_DEFAULT_CREDENTIALS("disableGcsDefaultCredentials", false, Boolean.class),
 
   JDBC_ARROW_TREAT_DECIMAL_AS_INT("JDBC_ARROW_TREAT_DECIMAL_AS_INT", false, Boolean.class),
-  DISABLE_SAML_URL_CHECK("disableSamlURLCheck", false, Boolean.class);
+
+  DISABLE_SAML_URL_CHECK("disableSamlURLCheck", false, Boolean.class),
+
+  // Used to determine whether to use the previously hardcoded value for the formatter (for
+  // backwards compatibility) or use the value of JDBC_FORMAT_DATE_WITH_TIMEZONE
+  JDBC_DEFAULT_FORMAT_DATE_WITH_TIMEZONE(
+      "JDBC_DEFAULT_FORMAT_DATE_WITH_TIMEZONE", false, Boolean.class),
+
+  // Used as a fix for issue SNOW-354859. Remove with snowflake-jdbc version 4.x with BCR changes.
+  JDBC_GET_DATE_USE_NULL_TIMEZONE("JDBC_GET_DATE_USE_NULL_TIMEZONE", false, Boolean.class);
 
   // property key in string
   private String propertyKey;

diff --git a/src/main/java/net/snowflake/client/core/arrow/DateConverter.java b/src/main/java/net/snowflake/client/core/arrow/DateConverter.java
@@ -22,9 +22,23 @@ public class DateConverter extends AbstractArrowVectorConverter {
   private DateDayVector dateVector;
   private static TimeZone timeZoneUTC = TimeZone.getTimeZone("UTC");
 
+  private boolean useDateFormat;
+
+  @Deprecated
   public DateConverter(ValueVector fieldVector, int columnIndex, DataConversionContext context) {
     super(SnowflakeType.DATE.name(), fieldVector, columnIndex, context);
     this.dateVector = (DateDayVector) fieldVector;
+    this.useDateFormat = false;
+  }
+
+  public DateConverter(
+      ValueVector fieldVector,
+      int columnIndex,
+      DataConversionContext context,
+      boolean useDateFormat) {
+    super(SnowflakeType.DATE.name(), fieldVector, columnIndex, context);
+    this.dateVector = (DateDayVector) fieldVector;
+    this.useDateFormat = useDateFormat;
   }
 
   private Date getDate(int index, TimeZone jvmTz, boolean useDateFormat) throws SFException {
@@ -86,7 +100,11 @@ public BigDecimal toBigDecimal(int index) {
 
   @Override
   public Timestamp toTimestamp(int index, TimeZone tz) throws SFException {
-    Date date = toDate(index, tz, true);
+    boolean useDateFormat = true;
+    if (this.context.getSession() != null) {
+      useDateFormat = getUseDateFormat(true);
+    }
+    Date date = toDate(index, tz, useDateFormat);
     if (date == null) {
       return null;
     } else {
@@ -99,21 +117,21 @@ public String toString(int index) throws SFException {
     if (context.getDateFormatter() == null) {
       throw new SFException(ErrorCode.INTERNAL_ERROR, "missing date formatter");
     }
-    Date date = getDate(index, timeZoneUTC, false);
+    Date date = getDate(index, timeZoneUTC, getUseDateFormat(false));
     return date == null ? null : ResultUtil.getDateAsString(date, context.getDateFormatter());
   }
 
   @Override
   public Object toObject(int index) throws SFException {
-    return toDate(index, TimeZone.getDefault(), false);
+    return toDate(index, TimeZone.getDefault(), getUseDateFormat(false));
   }
 
   @Override
   public boolean toBoolean(int index) throws SFException {
     if (isNull(index)) {
       return false;
     }
-    Date val = toDate(index, TimeZone.getDefault(), false);
+    Date val = toDate(index, TimeZone.getDefault(), getUseDateFormat(false));
     throw new SFException(
         ErrorCode.INVALID_VALUE_CONVERT, logicalTypeStr,
         SnowflakeUtil.BOOLEAN_STR, val);
@@ -128,4 +146,12 @@ public static Date getDate(
     // Note: use default time zone to match with current getDate() behavior
     return ArrowResultUtil.getDate(value, jvmTz, sessionTimeZone);
   }
+
+  private Boolean getUseDateFormat(Boolean defaultValue) {
+    return this.context.getSession() == null
+        ? defaultValue
+        : (this.context.getSession().getDefaultFormatDateWithTimezone()
+            ? defaultValue
+            : this.useDateFormat);
+  }
 }
diff --git a/src/main/java/net/snowflake/client/jdbc/ArrowResultChunk.java b/src/main/java/net/snowflake/client/jdbc/ArrowResultChunk.java
@@ -248,7 +248,11 @@ private static List<ArrowVectorConverter> initConverters(
             break;
 
           case DATE:
-            converters.add(new DateConverter(vector, i, context));
+            boolean getFormatDateWithTimeZone = false;
+            if (context.getSession() != null) {
+              getFormatDateWithTimeZone = context.getSession().getFormatDateWithTimezone();
+            }
+            converters.add(new DateConverter(vector, i, context, getFormatDateWithTimeZone));
             break;
 
           case FIXED:

diff --git a/src/main/java/net/snowflake/client/jdbc/SnowflakeBaseResultSet.java b/src/main/java/net/snowflake/client/jdbc/SnowflakeBaseResultSet.java
@@ -134,7 +134,9 @@ protected void raiseSQLExceptionIfResultSetIsClosed() throws SQLException {
   @Override
   public Date getDate(int columnIndex) throws SQLException {
     raiseSQLExceptionIfResultSetIsClosed();
-    return getDate(columnIndex, (TimeZone) null);
+    return getDate(
+        columnIndex,
+        this.session.getGetDateUseNullTimezone() ? (TimeZone) null : TimeZone.getDefault());
   }
 
   @Override

diff --git a/src/main/java/net/snowflake/client/jdbc/SnowflakeBasicDataSource.java b/src/main/java/net/snowflake/client/jdbc/SnowflakeBasicDataSource.java
@@ -371,4 +371,13 @@ public void setDiagnosticsAllowlistFile(String diagnosticsAllowlistFile) {
     this.properties.put(
         SFSessionProperty.DIAGNOSTICS_ALLOWLIST_FILE.getPropertyKey(), diagnosticsAllowlistFile);
   }
+
+  public void setJDBCDefaultFormatDateWithTimezone(Boolean jdbcDefaultFormatDateWithTimezone) {
+    this.properties.put(
+        "JDBC_DEFAULT_FORMAT_DATE_WITH_TIMEZONE", jdbcDefaultFormatDateWithTimezone);
+  }
+
+  public void setGetDateUseNullTimezone(Boolean getDateUseNullTimezone) {
+    this.properties.put("JDBC_GET_DATE_USE_NULL_TIMEZONE", getDateUseNullTimezone);
+  }
 }
diff --git a/src/test/java/net/snowflake/client/core/arrow/DateConverterTest.java b/src/test/java/net/snowflake/client/core/arrow/DateConverterTest.java
@@ -7,19 +7,24 @@
 import static org.junit.Assert.assertTrue;
 
 import java.sql.Date;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Random;
 import java.util.Set;
 import java.util.TimeZone;
 import net.snowflake.client.TestUtil;
 import net.snowflake.client.core.SFException;
+import net.snowflake.client.core.json.DateTimeConverter;
 import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.memory.RootAllocator;
 import org.apache.arrow.vector.DateDayVector;
 import org.apache.arrow.vector.types.Types;
 import org.apache.arrow.vector.types.pojo.FieldType;
+import org.junit.After;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
@@ -63,6 +68,34 @@ public DateConverterTest(String tz) {
     "2016-04-20"
   };
 
+  // Map of timezone to ArrowResultUtil.getDate value and hours offset for testTimezoneDates test
+  // case
+  Map<String, List<Object>> timezoneDatesData =
+      new HashMap<String, List<Object>>() {
+        {
+          put("UTC", Arrays.asList("2016-04-20", 0));
+          put("America/Los_Angeles", Arrays.asList("2016-04-20", -7));
+          put("America/New_York", Arrays.asList("2016-04-20", -4));
+          put("Pacific/Honolulu", Arrays.asList("2016-04-20", -10));
+          put("Asia/Singapore", Arrays.asList("2016-04-19", 8));
+          put("MEZ", Arrays.asList("2016-04-20", 0));
+          put("MESZ", Arrays.asList("2016-04-20", 0));
+        }
+      };
+
+  public static final int MILLIS_IN_ONE_HOUR = 3600000;
+  private TimeZone defaultTimeZone;
+
+  @Before
+  public void getDefaultTimeZone() {
+    this.defaultTimeZone = TimeZone.getDefault();
+  }
+
+  @After
+  public void restoreDefaultTimeZone() {
+    TimeZone.setDefault(defaultTimeZone);
+  }
+
   @Test
   public void testDate() throws SFException {
     Map<String, String> customFieldMeta = new HashMap<>();
@@ -85,7 +118,7 @@ public void testDate() throws SFException {
       j++;
     }
 
-    ArrowVectorConverter converter = new DateConverter(vector, 0, this);
+    ArrowVectorConverter converter = new DateConverter(vector, 0, this, false);
     int rowCount = j;
     i = 0;
     j = 0;
@@ -144,7 +177,7 @@ public void testRandomDates() throws SFException {
       }
     }
 
-    ArrowVectorConverter converter = new DateConverter(vector, 0, this);
+    ArrowVectorConverter converter = new DateConverter(vector, 0, this, false);
 
     for (int i = 0; i < rowCount; i++) {
       int intVal = converter.toInt(i);
@@ -162,4 +195,45 @@ public void testRandomDates() throws SFException {
       }
     }
   }
+
+  @Test
+  public void testTimezoneDates() throws SFException {
+    int testDay = 16911;
+    Map<String, String> customFieldMeta = new HashMap<>();
+    customFieldMeta.put("logicalType", "DATE");
+    // test normal date
+    FieldType fieldType =
+        new FieldType(true, Types.MinorType.DATEDAY.getType(), null, customFieldMeta);
+
+    DateDayVector vector = new DateDayVector("date", fieldType, allocator);
+
+    vector.setSafe(0, testDay);
+
+    // Test JDBC_FORMAT_DATE_WITH_TIMEZONE=TRUE with different session timezones
+    TimeZone.setDefault(TimeZone.getTimeZone("UTC"));
+    String tz = System.getProperty("user.timezone");
+    ArrowVectorConverter converter = new DateConverter(vector, 0, this, true);
+    converter.setUseSessionTimezone(true);
+    converter.setSessionTimeZone(TimeZone.getTimeZone(tz));
+    Object obj = converter.toObject(0);
+    DateTimeConverter jsonConverter =
+        new DateTimeConverter(
+            TimeZone.getTimeZone(tz), this.getSession(), 0, true, false, true, true);
+    Date jsonDate =
+        jsonConverter.getDate(Integer.toString(testDay), 91, 91, TimeZone.getTimeZone(tz), 0);
+    Object utcObj =
+        ArrowResultUtil.getDate(testDay, TimeZone.getTimeZone("UTC"), TimeZone.getTimeZone(tz));
+
+    List<Object> testValues = this.timezoneDatesData.get(tz);
+    assertTrue(testValues.get(0) instanceof String);
+    assertTrue(testValues.get(1) instanceof Integer);
+    assertThat(obj.toString(), is("2016-04-20"));
+    assertThat(jsonDate.toString(), is(obj.toString()));
+    assertThat(utcObj.toString(), is(testValues.get(0)));
+    assertThat(
+        ((Date) obj).getTime(),
+        is(((Date) utcObj).getTime() + ((Integer) testValues.get(1) * MILLIS_IN_ONE_HOUR)));
+
+    vector.clear();
+  }
 }
diff --git a/src/test/java/net/snowflake/client/loader/LoaderIT.java b/src/test/java/net/snowflake/client/loader/LoaderIT.java
@@ -9,10 +9,12 @@
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.sql.Statement;
 import java.sql.Time;
 import java.util.Arrays;
 import java.util.Calendar;
@@ -142,19 +144,20 @@ public void testLoadTime() throws Exception {
         errorMessage = listener.getErrors().get(0).getException().toString();
       }
       assertThat(String.format("Error: %s", errorMessage), listener.getErrorCount(), equalTo(0));
-      ResultSet rs =
-          testConnection
-              .createStatement()
-              .executeQuery(String.format("SELECT c1, c2 FROM %s LIMIT 1", tableName));
-      rs.next();
-      Time rsTm = rs.getTime(1);
-      Date rsDt = rs.getDate(2);
-      assertThat("Time column didn't match", rsTm, equalTo(tm));
-
-      Calendar cal = cutOffTimeFromDate(dt);
-      long dtEpoch = cal.getTimeInMillis();
-      long rsDtEpoch = rsDt.getTime();
-      assertThat("Date column didn't match", rsDtEpoch, equalTo(dtEpoch));
+      try (Statement statement = testConnection.createStatement()) {
+        try (ResultSet rs =
+            statement.executeQuery(String.format("SELECT c1, c2 FROM %s LIMIT 1", tableName))) {
+          assertTrue(rs.next());
+          Time rsTm = rs.getTime(1);
+          Date rsDt = rs.getDate(2);
+          assertThat("Time column didn't match", rsTm, equalTo(tm));
+
+          Calendar cal = cutOffTimeFromDate(dt);
+          long dtEpoch = cal.getTimeInMillis();
+          long rsDtEpoch = rsDt.getTime();
+          assertThat("Date column didn't match", rsDtEpoch, equalTo(dtEpoch));
+        }
+      }
     } finally {
       testConnection.createStatement().execute(String.format("DROP TABLE IF EXISTS %s", tableName));
     }