MaxGekk commented on code in PR #59955:
URL: https://github.com/apache/spark/pull/59955#discussion_r4424065421
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala:
##########
@@ -1105,6 +1105,34 @@ object DateTimeUtils extends SparkDateTimeUtils {
}
}
+ /** The whole seconds and the nanos of the second of a seconds value with a
fraction. */
+ case class SecondsAndNanos(seconds: Int, nanos: Int)
+
+ /**
+ * Splits the seconds of a minute with a fraction of 6 digits (microseconds)
into the whole
+ * seconds and the nanos of the second, as `LocalTime.of` and
`LocalDateTime.of` take them.
+ *
+ * The whole seconds are checked to be in the int range and not negative
before they are
+ * narrowed. A value outside of the int range would overflow when converted
from long to int,
+ * and could produce an int in the valid seconds range and return a wrong
value, or a misleading
+ * error message. A negative value is rejected here for a better error
message: `Math.floorDiv`
+ * gets the next lower integer for it, so the full seconds would not match
the value given.
+ * The message is the one that `LocalTime.of` and `LocalDateTime.of` throw
for invalid seconds,
+ * with the seconds given, truncated towards zero. Any other value, such as
61 seconds, is
+ * left to them, which check their fields in order, so an invalid date or
hour is reported
+ * before such seconds, while a negative value or one above `Int.MaxValue`
is reported first.
+ */
+ def splitSecsAndMicros(secsAndMicros: Decimal): SecondsAndNanos = {
+ val unscaledSecFrac = secsAndMicros.toUnscaledLong
+ val fullSecs = Math.floorDiv(unscaledSecFrac, MICROS_PER_SECOND)
+ if (fullSecs > Int.MaxValue || fullSecs < 0) {
+ throw new DateTimeException(
+ s"Invalid value for SecondOfMinute (valid values 0 - 59):
${secsAndMicros.toLong}")
Review Comment:
Good catch, thanks. In b191a3602d7 a value between -1 and 0 is shown in
full, e.g. `-0.5`, since the truncation would show it as the valid 0; every
other rejected value keeps the truncated whole seconds, so the existing
`make_time` expectations and the `time.sql` golden output are unchanged. Added
`-0.5` and `-0.000001` to the `make_timestamp` test, a `make_time` case for
`-0.5` in `DateTimeUtilsSuite`, and updated the scaladoc and the test comment.
--
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]
