zabetak commented on code in PR #5309:
URL: https://github.com/apache/hive/pull/5309#discussion_r1665334948


##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java:
##########
@@ -123,7 +123,13 @@ public static String normalizeDate(String date) {
    * @return Timestamp in string format.
    */
   public static String convertTimestampToString(Timestamp timestamp) {
-    return TIMESTAMP_FORMATTER.format(timestamp.toLocalDateTime());
+    TimeZone defaultTimeZone = TimeZone.getDefault();
+    TimeZone.setDefault(TimeZone.getTimeZone("UTC"));

Review Comment:
   TimeZone.setDefault changes the zone for the whole application. In Hive, 
many things are happening in parallel so this is not something that should be 
done in utility APIs. It can unpredictable side effects and hard to diagnose 
race conditions.



##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreUtils.java:
##########
@@ -48,28 +48,52 @@ public class TestMetaStoreUtils {
   private static final DateTimeFormatter FORMATTER = 
DateTimeFormatter.ofPattern("uuuu-MM-dd HH:mm:ss");
   private final TimeZone timezone;
   private final String date;
-  private final String timestamp;
+  private final String timestampString;
+  private final Timestamp timestamp;
 
   public TestMetaStoreUtils(String zoneId, LocalDateTime timestamp) {
     this.timezone = TimeZone.getTimeZone(zoneId);
-    this.timestamp = timestamp.format(FORMATTER);
+    this.timestampString = timestamp.format(FORMATTER);
     this.date = 
timestamp.toLocalDate().format(DateTimeFormatter.ISO_LOCAL_DATE);
+
+    TimeZone.setDefault(TimeZone.getTimeZone("UTC"));
+    this.timestamp = Timestamp.valueOf(timestamp);
+    TimeZone.setDefault(DEFAULT);
   }
 
   @Parameterized.Parameters(name = "zoneId={0}, timestamp={1}")
   public static Collection<Object[]> generateZoneTimestampPairs() {
     List<Object[]> params = new ArrayList<>();
-    long minDate = LocalDate.of(0, 1, 
1).atStartOfDay().toEpochSecond(ZoneOffset.UTC);
+    long minDate = LocalDate.of(1, 1, 
1).atStartOfDay().toEpochSecond(ZoneOffset.UTC);

Review Comment:
   According to the data type specification of 
[DATE](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=82706456#LanguageManualTypes-date)
 `0000-­01-­01` is a valid and supported date.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

Reply via email to