0lai0 commented on PR #4987:
URL:
https://github.com/apache/datafusion-comet/pull/4987#issuecomment-6252215444
Thanks @andygrove. Addressed in the latest commit:
- **Max-precision test comment:** it now says the values and offset stay
away from the `Decimal128` limit so the test exercises the native path, and
that the overflow case falls back and is covered by the tests below it.
- **Non-zero scale:** added two `DECIMAL(38, 10)` tests over the type's
extremes, `0` and a `NULL`, under both ANSI settings. An offset one unscaled
unit over the limit (`7014118346046923173168730371.5884105729`) falls back and
matches Spark; with the guard removed it returns `2` instead of Spark's `1` for
the largest value, because the overflowing bound takes the trailing `NULL` in.
The limit itself (`7014118346046923173168730371.5884105728` FOLLOWING), and the
largest 34-digit offset under it on both sides, run natively and match Spark
across ASC/DESC and NULLS FIRST/LAST.
One note on the scale factors: Spark's `WindowFrameCoercion`
(`createBoundaryCast` in `TypeCoercionHelper`) casts a numeric offset to the
`ORDER BY` type, so by the time the guard sees it the two scales are equal and
both factors are `10^0`. The new tests do cover the non-zero scale in the
type's largest unscaled value and in the offset's unscaled value. I kept the
factors so the guard stays correct if an offset ever reaches it with a
different scale.
- **#5022:** I don't have permission to edit the issue body, so I left a
comment there with the corrected Spark behavior and the current state of DATE
and DECIMAL, and asked you to update the description.
--
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]
