joelrobin18 opened a new pull request, #59954: URL: https://github.com/apache/spark/pull/59954
### What changes were proposed in this pull request? `ReorderAssociativeOperator` flattens chains of integral `Add` / `Multiply` to fold their literals. Its `flattenAdd` / `flattenMultiply` helpers matched `Add(l, r, _)` / `Multiply(l, r, _)` without looking at the evaluation mode, and the rule then rebuilt the whole chain with the root operator's mode. Since SPARK-40222, numeric `try_add` and `try_multiply` are replaced by TRY-mode `Add` / `Multiply`, so combining them with regular arithmetic changed their semantics. This PR flattens only operators whose evaluation context equals the root's; an operator in a different mode is kept as a single operand. This also resolves the TODO in the rule about not reordering operators with different `failOnError` flags. ### Why are the changes needed? The rule changes query results when `try_add` / `try_multiply` over non-foldable inputs is combined with regular arithmetic: ```sql CREATE TABLE t1 (x INT, y INT) USING parquet; INSERT INTO t1 VALUES (2147483647, 1); SELECT try_add(x, y) + 1 FROM t1; -- ARITHMETIC_OVERFLOW, expected NULL SELECT try_multiply(x, 2) * 2 FROM t1; -- ARITHMETIC_OVERFLOW, expected NULL SELECT try_add(x + y, 1) FROM t1; -- NULL, expected ARITHMETIC_OVERFLOW from `x + y` (SPARK-40222) ``` For the first query the analyzed plan is `(try_add(x, y) + 1)`, but the optimized plan is `((x + y) + 1)` with the inner add evaluated in ANSI mode; excluding `ReorderAssociativeOperator` returns NULL. Literal inputs are not affected because `ConstantFolding` evaluates `try_add` before this rule runs. ### Does this PR introduce _any_ user-facing change? Yes, it fixes a bug. With non-foldable inputs, `try_add` / `try_multiply` nested in a regular `+` / `*` now return NULL on overflow instead of failing, and a regular `+` / `*` nested in `try_add` / `try_multiply` now raises its overflow instead of returning NULL. The incorrect behavior exists since Spark 3.4.0 (SPARK-40222). ### How was this patch tested? - Added a test to `ReorderAssociativeOperatorSuite` covering TRY-mode operators inside regular ones, regular operators inside TRY-mode ones, and literal folding around a TRY-mode operand. It fails without this change. - Ran all suites in `org.apache.spark.sql.catalyst.optimizer` (109 suites, 1592 tests), and `SQLQueryTestSuite` for `having.sql` (which has `avg(try_add(v, 1) + 1)`), `try_arithmetic.sql` and `nonansi/try_arithmetic.sql`. All pass and no golden files change (Pandas UDF variants were skipped locally because pandas/pyarrow are not installed). - Ran the queries above with `bin/spark-sql` before and after the change. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Isaac (Claude Opus 5.5) 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]
