[GitHub] [spark] MaxGekk commented on a change in pull request #27512: [SPARK-30766][SQL] Fix the timestamp truncation to the `HOUR` and `DAY` levels

[GitBox <gi...@apache.org>]

MaxGekk commented on a change in pull request #27512: [SPARK-30766][SQL] Fix the timestamp truncation to the `HOUR` and `DAY` levels
URL: https://github.com/apache/spark/pull/27512#discussion_r376968794
 
 

 ##########
 File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala
 ##########
 @@ -713,32 +713,34 @@ object DateTimeUtils {
     }
   }
 
+  private def truncToUnit(t: SQLTimestamp, tz: TimeZone, unit: ChronoUnit): SQLTimestamp = {
+    val truncated = microsToInstant(t).atZone(tz.toZoneId).truncatedTo(unit)
+    instantToMicros(truncated.toInstant)
+  }
+
   /**
    * Returns the trunc date time from original date time and trunc level.
    * Trunc level should be generated using `parseTruncLevel()`, should be between 0 and 12.
    */
   def truncTimestamp(t: SQLTimestamp, level: Int, timeZone: TimeZone): SQLTimestamp = {
-    if (level == TRUNC_TO_MICROSECOND) return t
-    var millis = MICROSECONDS.toMillis(t)
-    val truncated = level match {
-      case TRUNC_TO_MILLISECOND => millis
-      case TRUNC_TO_SECOND =>
-        millis - millis % MILLIS_PER_SECOND
-      case TRUNC_TO_MINUTE =>
-        millis - millis % MILLIS_PER_MINUTE
-      case TRUNC_TO_HOUR =>
-        val offset = timeZone.getOffset(millis)
 
 Review comment:
   It uses combined calendar - Julian + Gregorian. Look at the chain of calls:
   TimeZone.getOffset -> TimeZone.getRawOffset ->ZoneInfo.getRawOffset -> ZoneInfo.getOffsets -> ZoneInfo.getOffsets(long date, int[] offsets, int type) -> SimpleTimeZone.getOffsets which uses combined calendar - for >= 1582 years Gregorian calendar, before 1582 - Julian calendar http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/classes/java/util/SimpleTimeZone.java#l559:
   ```java
   private static final Gregorian gcal = CalendarSystem.getGregorianCalendar();
   ...
   BaseCalendar cal = date >= GregorianCalendar.DEFAULT_GREGORIAN_CUTOVER ?
                   gcal : (BaseCalendar) CalendarSystem.forName("julian");
   ```
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org