rdtr opened a new pull request, #60199: URL: https://github.com/apache/spark/pull/60199
This depends on https://github.com/apache/spark/pull/59194. Only the last commit is new. ### What changes were proposed in this pull request? `LimitPushDown` rewrites `LocalLimit(le, Offset(oe, child))` to `Offset(oe, LocalLimit(le + oe, child))`, and `EliminateOffsets` rewrites `Offset(oe1, Offset(oe2, child))` to `Offset(oe1 + oe2, child)`. This adds `LimitAndOffsetValues.canAdd`, which returns false when both values are integer literals and their sum does not fit in an `Int`, and uses it as a guard on both cases. In that case `LimitPushDown` keeps the `LocalLimit` above the `Offset`, and `EliminateOffsets` keeps both `Offset` operators. Pushing a limit that large down would not reduce the rows read in practice. Values that are not integer literals are merged as before. ### Why are the changes needed? `CheckAnalysis` rejects an overflowing sum with `SUM_OF_LIMIT_AND_OFFSET_EXCEEDS_MAX_INT` only for a `LocalLimit` directly over an `Offset` in the analyzed plan. The optimizer builds other such pairs, e.g. when it pushes a limit into a `Union` branch or removes a `Project` between a limit and an offset, and it merges two `Offset` operators without any check. With a table `t` of 3 rows and no known row count, on master: | Query | Expected | ANSI on | ANSI off | |---|---|---|---| | `(SELECT * FROM t OFFSET 1) UNION ALL (SELECT * FROM t) LIMIT 2147483647` | 5 rows | `ARITHMETIC_OVERFLOW` | 3 rows | | `SELECT * FROM (SELECT * FROM t OFFSET 10) LIMIT 2147483647` | 0 rows | `ARITHMETIC_OVERFLOW` | `INTERNAL_ERROR` | | `SELECT * FROM (SELECT * FROM t OFFSET 2147483647) OFFSET 10` | 0 rows | `ARITHMETIC_OVERFLOW` | `INTERNAL_ERROR` | With ANSI off, the sum wraps to a negative value. A `LocalLimit` with a negative limit returns no rows, so the `UNION ALL` query loses the rows of the first branch, and the other two queries fail during physical planning. ### Does this PR introduce _any_ user-facing change? Yes. The queries above return the expected rows with ANSI on and off. Queries whose merged values fit in an `Int` are not changed. ### How was this patch tested? - `LimitPushdownSuite`: "SPARK-XXXXX: Do not push down limit through Offset if the merged limit overflows" checks that `LimitPushDown` leaves `LocalLimit(2147483647, Offset(10, ...))` unchanged. - `EliminateOffsetsSuite`: "SPARK-XXXXX: Do not combine Offset operators if the sum overflows" checks the same for `Offset(2147483647, Offset(10, ...))`. - `SQLQuerySuite`: "SPARK-XXXXX: LIMIT and OFFSET whose merged value overflows return the right rows" runs the three queries above with ANSI on and off, each with and without `ConstantFolding`. The two unit tests replace the ones that #59194 adds, which expect an unevaluated `Add` for an overflowing sum. Without the guards, all 12 query runs in the `SQLQuerySuite` test fail, and removing either guard alone makes its unit test fail. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5.5) -- 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]
