markj-db opened a new pull request, #59527: URL: https://github.com/apache/spark/pull/59527
### What changes were proposed in this pull request? `WindowEvaluatorFactoryBase.estimateMaxCachedBlocks` sizes the block cache of the segment-tree window frame from the width of a ROWS frame. It computed that width in `Int` arithmetic: ```scala case (CurrentRow, IntegerLiteral(hi)) => Some(math.abs(hi) + 1) case (IntegerLiteral(lo), CurrentRow) => Some(math.abs(lo) + 1) case (IntegerLiteral(lo), IntegerLiteral(hi)) => Some(math.abs(hi - lo) + 1) ``` For an offset of `Int.MaxValue`, `math.abs(hi) + 1` wraps to `Int.MinValue`. The block budget `ceil(w / blockSize) + 2` then becomes negative (`-32766` with the default block size of 65536), and `WindowSegmentTree` rejects it with `requirement failed: maxCachedBlocks must be >= 1`. `hi - lo` can wrap the same way: for `-Int.MaxValue .. Int.MaxValue` it evaluates to `-2`, which silently under-sizes the cache instead of failing. This PR computes the width in `Long`. The width is at most `2^32` and the block size is at least 16 (enforced by `spark.sql.windowExec.segmentTree.blockSize`), so the resulting block count is at most `2^28 + 2` and still fits in an `Int`. ### Why are the changes needed? With `spark.sql.windowExec.segmentTree.enabled=true` (default `false`), these frames fail the query even on a five-row table: * `ROWS BETWEEN CURRENT ROW AND 2147483647 FOLLOWING` * `ROWS BETWEEN 2147483647 PRECEDING AND CURRENT ROW` * `ROWS BETWEEN 1 PRECEDING AND 2147483647 FOLLOWING` * `ROWS BETWEEN 2147483647 PRECEDING AND 1 FOLLOWING` ```sql SET spark.sql.windowExec.segmentTree.enabled=true; SET spark.sql.windowExec.segmentTree.minPartitionRows=1; SELECT id, sum(id) OVER (ORDER BY id ROWS BETWEEN CURRENT ROW AND 2147483647 FOLLOWING) FROM VALUES (1), (2), (3), (4), (5) AS t(id); -- java.lang.IllegalArgumentException: requirement failed: -- maxCachedBlocks must be >= 1 when specified, got -32766 ``` The same queries return correct results with the segment tree disabled, so this is a regression only for users who opt in. It fails loudly and does not return wrong rows. ### Does this PR introduce _any_ user-facing change? Yes, for users who enable the segment tree: the four frames above now return the same results as they do with the segment tree disabled, instead of failing with `IllegalArgumentException`. The segment tree is off by default and was added in 4.2.0. ### How was this patch tested? Added `SPARK-60267: SUM over ROWS frames with offsets near Int.MaxValue` to `SegmentTreeWindowFunctionSuite`. It runs five frames with offsets near `Int.MaxValue` with the segment tree enabled and disabled and compares the results using the suite's existing `checkEquivalence`. Without the fix it fails with `maxCachedBlocks must be >= 1 when specified, got -32766`; with the fix it passes. Also ran `SegmentTreeWindowFunctionSuite` and `DataFrameWindowFramesSuite` (81 tests) and scalastyle. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Isaac This pull request and its description were written by Isaac. -- 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]
