MaxGekk commented on a change in pull request #33031:
URL: https://github.com/apache/spark/pull/33031#discussion_r656549571



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -1023,83 +1023,95 @@ object IntervalUtils {
     var rest = micros
     val from = DayTimeIntervalType.fieldToString(startField).toUpperCase
     val to = DayTimeIntervalType.fieldToString(endField).toUpperCase
+    val prefix = "INTERVAL '"
+    val postfix = s"' ${if (startField == endField) from else s"$from TO $to"}"
+
     if (micros < 0) {
       if (micros == Long.MinValue) {
         // Especial handling of minimum `Long` value because negate op 
overflows `Long`.
         // seconds = 106751991 * (24 * 60 * 60) + 4 * 60 * 60 + 54 = 
9223372036854
         // microseconds = -9223372036854000000L-775808 == Long.MinValue
+        val baseStr = "-106751991 04:00:54.775808000"
         val minIntervalString = style match {
           case ANSI_STYLE =>
-            val baseStr = "-106751991 04:00:54.775808"
-            val fromPos = startField match {
-              case DayTimeIntervalType.DAY => 0
-              case DayTimeIntervalType.HOUR => 11
-              case DayTimeIntervalType.MINUTE => 14
-              case DayTimeIntervalType.SECOND => 17
+            val firstStr = startField match {
+              case DayTimeIntervalType.DAY => "-106751991"
+              case DayTimeIntervalType.HOUR => "-2562047788"
+              case DayTimeIntervalType.MINUTE => "-153722867280"
+              case DayTimeIntervalType.SECOND => "-9223372036854.775808"
             }
-            val toPos = endField match {
-              case DayTimeIntervalType.DAY => 10
-              case DayTimeIntervalType.HOUR => 13
-              case DayTimeIntervalType.MINUTE => 16
-              case DayTimeIntervalType.SECOND => baseStr.length
+            val followingStr = if (startField == endField) {
+              ""
+            } else {
+              val substrStart = startField match {
+                case DayTimeIntervalType.DAY => 10
+                case DayTimeIntervalType.HOUR => 13
+                case DayTimeIntervalType.MINUTE => 16
+              }
+              val substrEnd = endField match {
+                case DayTimeIntervalType.HOUR => 13
+                case DayTimeIntervalType.MINUTE => 16
+                case DayTimeIntervalType.SECOND => 26
+              }
+              baseStr.substring(substrStart, substrEnd)
             }
-            val postfix = if (startField == endField) from else s"$from TO $to"
-            s"INTERVAL '${baseStr.substring(fromPos, toPos)}' $postfix"
-          case HIVE_STYLE => "-106751991 04:00:54.775808000"
+
+            s"$prefix$firstStr$followingStr$postfix"
+          case HIVE_STYLE => baseStr
         }
         return minIntervalString
       } else {
         sign = "-"
         rest = -rest
       }
     }
-    val secondsWithFraction = rest % MICROS_PER_MINUTE
-    rest /= MICROS_PER_MINUTE
-    val minutes = rest % MINUTES_PER_HOUR
-    rest /= MINUTES_PER_HOUR
-    val hours = rest % HOURS_PER_DAY
-    val days = rest / HOURS_PER_DAY
-    val leadSecZero = if (secondsWithFraction < 10 * MICROS_PER_SECOND) "0" 
else ""
     val intervalString = style match {
       case ANSI_STYLE =>
-        val secStr = java.math.BigDecimal.valueOf(secondsWithFraction, 6)
-          .stripTrailingZeros()
-          .toPlainString()
-        val formatBuilder = new StringBuilder("INTERVAL '")
-        if (startField == endField) {
-          startField match {
-            case DayTimeIntervalType.DAY => formatBuilder.append(s"$sign$days' 
")
-            case DayTimeIntervalType.HOUR => 
formatBuilder.append(f"$hours%02d' ")
-            case DayTimeIntervalType.MINUTE => 
formatBuilder.append(f"$minutes%02d' ")
-            case DayTimeIntervalType.SECOND => 
formatBuilder.append(s"$leadSecZero$secStr' ")
-          }
-          formatBuilder.append(from).toString
+        val formatBuilder = new mutable.StringBuilder(sign)
+        val formatArgs = new mutable.ArrayBuffer[Long]()
+        if (startField == DayTimeIntervalType.SECOND) {

Review comment:
       I didn't get why do you check `SECOND` in the if but the rest fields in 
`match .. case`. Can you handle `SECOND` in the same way as others?

##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -1023,83 +1023,95 @@ object IntervalUtils {
     var rest = micros
     val from = DayTimeIntervalType.fieldToString(startField).toUpperCase
     val to = DayTimeIntervalType.fieldToString(endField).toUpperCase
+    val prefix = "INTERVAL '"
+    val postfix = s"' ${if (startField == endField) from else s"$from TO $to"}"
+
     if (micros < 0) {
       if (micros == Long.MinValue) {
         // Especial handling of minimum `Long` value because negate op 
overflows `Long`.
         // seconds = 106751991 * (24 * 60 * 60) + 4 * 60 * 60 + 54 = 
9223372036854
         // microseconds = -9223372036854000000L-775808 == Long.MinValue
+        val baseStr = "-106751991 04:00:54.775808000"
         val minIntervalString = style match {
           case ANSI_STYLE =>
-            val baseStr = "-106751991 04:00:54.775808"
-            val fromPos = startField match {
-              case DayTimeIntervalType.DAY => 0
-              case DayTimeIntervalType.HOUR => 11
-              case DayTimeIntervalType.MINUTE => 14
-              case DayTimeIntervalType.SECOND => 17
+            val firstStr = startField match {
+              case DayTimeIntervalType.DAY => "-106751991"
+              case DayTimeIntervalType.HOUR => "-2562047788"
+              case DayTimeIntervalType.MINUTE => "-153722867280"
+              case DayTimeIntervalType.SECOND => "-9223372036854.775808"

Review comment:
       Could you import field names DAY .. SECOND, and remove 
`DayTimeIntervalType`. It just makes reading harder.




-- 
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to