dongjoon-hyun opened a new pull request, #60190:
URL: https://github.com/apache/spark/pull/60190
### What changes were proposed in this pull request?
`ConstantPropagation` replaces an attribute with the literal of an equality
conjunct, such as `y = 5`, and rewrites each operand of `AND`, `OR`, `COALESCE`
and `IN` on its own. These operators short-circuit, so an operand decides
whether the operands after it run. The literal is the value of the attribute
only on the rows that pass the whole condition. On the other rows, a rewritten
operand can take another value and run an operand after it that can fail:
- `(y < 3 OR 10 / x > 1) AND y = 5`: `y < 3` becomes `5 < 3` and is folded
away, so `10 / x > 1` runs on a row with `y = 1` and `x = 0`.
- `coalesce(m[k], 10 / x) > 0 AND k = 'a'`: `m[k]` becomes `m['a']`, which
is NULL on a row with `k = 'b'` and `m = map('b', 1.0)`.
- `y > 0 AND 10 / x > 0 AND y = 1`: the conjunct `y > 0` becomes TRUE and is
removed, so `10 / x > 0` runs on a row with `y = -1` and `x = 0`.
When the rewrite removes the last reference to the propagated column,
predicate pushdown can also move the conjunct below an operator. In `HAVING s =
5 AND (s < 3 OR 10 / g > 1)`, `10 / g > 1` runs below the `Aggregate` on every
input row.
This PR changes `ConstantPropagation` as follows:
- An operand of `AND`, `OR`, `COALESCE` or `IN` in a conjunct is rewritten
only if none of the operands after it, as rewritten, can fail
(`ExprUtils.canEvaluateUnconditionally`). The conjunct then keeps the
propagated attribute, so it is not pushed below the operator that produces the
attribute.
- The conjuncts of the condition are still rewritten, but the equalities
move ahead of the other conjuncts when a conjunct that can fail comes after a
rewritten conjunct, or still refers to a propagated attribute. Then such a
conjunct only runs on rows where the equalities hold. For example, `(y > 3 OR
10 / x > 1) AND y = 5` keeps its `OR`, which used to become TRUE, and runs it
after `y = 5`. An equality between an attribute and a literal cannot fail, so
running it first raises no error.
### Why are the changes needed?
The queries below return their results when they are evaluated as written,
but fail with `DIVIDE_BY_ZERO` under ANSI mode.
```sql
CREATE TABLE t1(y BIGINT, x BIGINT) USING parquet;
INSERT INTO t1 VALUES (1, 0), (5, 20);
-- It should return no rows, but fails with DIVIDE_BY_ZERO.
SELECT * FROM t1 WHERE (y < 3 OR 10 / x > 1) AND y = 5;
CREATE TABLE t2(y BIGINT, x BIGINT) USING parquet;
INSERT INTO t2 VALUES (-1, 0), (1, 5);
-- It should return (1, 5), but fails with DIVIDE_BY_ZERO.
SELECT * FROM t2 WHERE y > 0 AND 10 / x > 0 AND y = 1;
CREATE TABLE t3(g BIGINT, x BIGINT) USING parquet;
INSERT INTO t3 VALUES (0, 1);
-- It should return no rows, but fails with DIVIDE_BY_ZERO.
SELECT g, sum(x) s FROM t3 GROUP BY g HAVING s = 5 AND (s < 3 OR 10 / g > 1);
```
The last query also needs #60162. Without it, `InferFiltersFromConstraints`
infers `10 / g > 1` from the conjunct that this PR keeps and pushes it below
the `Aggregate`.
`ConstantPropagation` has rewritten the operands of `OR` since SPARK-20758
(#17993, [v2.3.0](https://github.com/apache/spark/tree/v2.3.0) (2018-02-22)),
first only in `=` and `<=>`. SPARK-42500 (#42038) extended it to all
comparisons, and SPARK-44444 (#46013) turned on ANSI mode by default. Both are
in [v4.0.0](https://github.com/apache/spark/tree/v4.0.0) (2025-05-19).
This PR is independent of #60163, which keeps the attributes of an
expression that can fail. Both change `ConstantPropagation.replaceConstants`
and add the same reordering of the equalities, so the one merged later needs a
rebase.
### Does this PR introduce _any_ user-facing change?
Yes. The queries above, which fail with an error, now return their results.
Some plans keep an operand that was rewritten before. For example, `y = 5
AND (y > 3 OR 10 / x > 1)` keeps the `OR` with `y = 5` ahead of it, instead of
becoming `y = 5`, and `y = 5 AND coalesce(y, 10 / x) > 3` keeps the `COALESCE`.
The results are the same. When a conjunct that can fail comes after a rewritten
conjunct, or still refers to a propagated attribute, the equalities move ahead
of the other conjuncts in the plan.
### How was this patch tested?
- Added `SPARK-61033: Do not propagate a constant into an operand that
guards an expression that can fail` to `ConstantPropagationSuite`. It covers an
`OR` with a nested `AND` in a condition, a `COALESCE` in a comparison, and a
conjunct before a conjunct that can fail, and checks that the equality goes
first. Each part fails if its change is reverted.
- Ran all suites in `org.apache.spark.sql.catalyst.optimizer`,
`SubquerySuite`, and the TPC-DS and TPC-H `PlanStabilitySuite`s with ANSI mode
on and off. All pass, and no golden files change.
- Ran the queries above and the variants with a nested `AND`, an `IN` in a
comparison and a `COALESCE` over a map lookup, with and without #60162. They no
longer fail, except the `HAVING` query without #60162.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: 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]
