markj-db commented on PR #59190: URL: https://github.com/apache/spark/pull/59190#issuecomment-5942619117
Thanks for the thorough review — the decision challenge is correct, and I confirmed it against the code. `UnsafeExternalSorter.getIterator` accounts for buffered rows in `int` (`recordsSpilled()`, `UnsafeSorterIterator.getNumRecords()`, the `int i` accumulator and skip condition, and `ChainedIterator.numRecords`), so even the `getIterator(0)` path used by LEAD/LAG and unbounded `nth_value` can silently drop spill files (or throw `ArrayIndexOutOfBoundsException`) once the running count overflows negative past 2^31 rows. Rather than widen the core sorter in this PR, I removed the offset-frame exemption and now **guard every window frame**: a partition larger than `Int.MaxValue` rows fails fast with `WINDOW_FUNCTION_PARTITION_SIZE_EXCEEDS_LIMIT` instead of silently returning wrong results. The row counter stays `Long` so the guard can detect the overflow. Widening the sorter's row accounting to `Long` and then re-enabling correct results for the offset frames is tracked as a follow-up: SPARK-59941. Both P3 nits are resolved by this simplification. Pushed in e550d74; PR title/description updated accordingly. -- 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]
