uros-b commented on code in PR #58708:
URL: https://github.com/apache/spark/pull/58708#discussion_r3987872831
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -3503,6 +3503,9 @@ case class Flatten(child: Expression) extends
UnaryExpression
Supported types are: byte, short, integer, long, date, timestamp.
+ The timestamp types include the nanosecond-precision timestamp types;
their generated values
+ advance on the microsecond grid and keep the start value's
sub-microsecond fraction.
+
Review Comment:
In collectionOperations.scala, Sequence.eval / Sequence.doGenCode (nanos
branches) - sequence can return an element strictly outside [start, stop] when
start and stop carry different sub-microsecond fractions. Membership is decided
on epochMicros alone, but every element is re-wrapped with the start value's
fraction, so an element that lands exactly on stopMicros is emitted with
start's fraction even when that exceeds stop's fraction. Concrete (traced
through IntegralSequenceImpl.eval): sequence(TIMESTAMP_NTZ '1970-01-01
00:00:00.000000500', TIMESTAMP_NTZ '1970-01-01 00:00:01', INTERVAL 1 SECOND) ->
micros grid [0, 1000000], re-wrapped with frac 500 -> [..00.000000500,
..01.000000500]; the last element 01.000000500 is greater than stop =
01.000000000. Full-precision semantics would return only [..00.000000500]. The
defect is symmetric for a negative step (an element below stop when startFrac <
stopFrac) and for the equal-micros/default-step degenerate case (e.g. start
.000000500,
stop .000000100 with the +1-day default step returns [start] instead of
empty). This violates the function's documented "from start to stop
(inclusive)" contract. Fix: decide the boundary element by comparing the
wrapped candidate's full (epochMicros, fraction) against stop's full value
(only the endpoint element can differ), or explicitly justify/document emitting
an out-of-range value.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala:
##########
@@ -1401,6 +1401,104 @@ class CollectionExpressionsSuite
Timestamp.valueOf("2018-01-01 00:00:00.000")))
}
+ test("SPARK-57834: sequence of nanosecond-precision timestamps") {
Review Comment:
Every non-null case uses the same fraction for start and stop (123/123,
500/500, 120/120, 100/100, 999/999, 42/42, 7/7), so the differing-fraction
boundary - exactly the case that reveals the concern above, is never exercised.
Add cases where start and stop fractions differ and an element lands on
stopMicros, for both step signs and the equal-micros/default-step degenerate
case.
--
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]