MaxGekk commented on a change in pull request #33231:
URL: https://github.com/apache/spark/pull/33231#discussion_r665439586
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -218,14 +223,20 @@ object IntervalUtils {
* adapted from HiveIntervalYearMonth.valueOf
*/
def fromYearMonthString(input: String): CalendarInterval = {
+ fromYearMonthString(input, YM.YEAR, YM.MONTH)
+ }
+
+ /**
+ * Parse YearMonth string in form: [+|-]YYYY-MM
+ *
+ * adapted from HiveIntervalYearMonth.valueOf
+ * Below interval conversion patterns are supported:
+ * - YEAR TO MONTH
Review comment:
Only this one?
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -218,14 +223,20 @@ object IntervalUtils {
* adapted from HiveIntervalYearMonth.valueOf
*/
def fromYearMonthString(input: String): CalendarInterval = {
+ fromYearMonthString(input, YM.YEAR, YM.MONTH)
+ }
+
+ /**
+ * Parse YearMonth string in form: [+|-]YYYY-MM
Review comment:
nit: YearMonth string -> year-month interval string
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -57,6 +57,11 @@ object IntervalUtils {
}
import IntervalUnit._
+ private val MAX_DAY = 106751991L
+ private val MAX_HOUR = 2562047788L
+ private val MAX_MINUTE = 153722867280L
+ private val MAX_SECOND = 9223372036854L
Review comment:
This is not used in the code. Found the place where the max seconds is
calculated:
https://github.com/apache/spark/blob/3953754f36656e1a0bee16b89fae0142f172a91a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala#L635
Could you replace such places in the file (maybe other places), and put a
formula here instead of numbers.
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -397,7 +408,7 @@ object IntervalUtils {
secondStr: String,
sign: Int): Long = {
var micros = 0L
- val days = toLongWithRange(DAY, dayStr, 0, Int.MaxValue).toInt
+ val days = toLongWithRange(DAY, dayStr, 0, MAX_DAY).toInt
Review comment:
Do we have tests for MAX_DAY + 1 and MAX_DAY, and for MIN_DAY-1 and
MIN_DAY?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
