peter-toth opened a new pull request, #59308:
URL: https://github.com/apache/spark/pull/59308
### Differences from #59294
This is the branch-4.2 backport of #59294. The description below is the
original one.
- On branch-4.2 `PlanMerger` and `MergeSubplansSuite` are in `sql/catalyst`
(`org.apache.spark.sql.catalyst.optimizer`), and the end-to-end suite is
`org.apache.spark.sql.PlanMergeSuite`. The tests went in unchanged.
- The `PlanMerger` change was ported by hand, because git couldn't map it to
the catalyst file. 4.2's `PlanMerger` has no DSv2 scan merge, so the code
around the tagged `(Filter, Filter)` branch looks different. The same lines
change: the lookup restriction, the dropped `isNew` flag and the append check
in `mergeNamedExpressions`.
- On branch-4.2 the 4 new tests fail without the fix. With it,
`MergeSubplansSuite` (58 tests) and `PlanMergeSuite` (14 tests) pass, and
scalastyle is clean.
### What changes were proposed in this pull request?
`PlanMerger.mergeNamedExpressions` appended the new plan's propagated filter
attribute only when it was a fresh alias. A reused one (`isNew = false`) was
assumed to be in the merged expression list already.
This PR appends the new plan's filter attribute whenever the list doesn't
output it yet. It also drops the `isNew` flag from
`TryMergeResult.newPlanFilter`, because nothing reads it any more.
The reuse lookup in the tagged `(Filter, Filter)` branch now only matches
the filter aliases that the merged `Filter` references. Before, a computed
column with the same expression as the new plan's filter could be reused, and
the merged `Filter` would drop that plan's rows.
### Why are the changes needed?
The assumption doesn't hold. A merge round can fold a filter attribute into
another filter alias, and then only the new alias is carried up to the
aggregate. A later round that reuses the original attribute ends up with an
aggregate `FILTER` on an attribute its child doesn't output:
```sql
SET
spark.sql.optimizer.mergeSubplans.filterPropagation.symmetricFilterPropagation.enabled=true;
SET
spark.sql.optimizer.mergeSubplans.filterPropagation.throughJoin.enabled=true;
CREATE TABLE t USING parquet AS SELECT * FROM VALUES
(1, 1, 10), (1, 2, 3), (2, 1, 7), (2, 2, 9), (3, 1, 9), (3, 2, 6) AS v(a,
b, c);
SELECT
(SELECT sum(l.a) FROM t l LEFT JOIN t r ON l.a = r.a WHERE l.b = 1),
(SELECT max(l.a) FROM t l LEFT JOIN t r ON l.a = r.a WHERE l.b = 2 AND
coalesce(r.c, 10) > 5),
(SELECT count(*) FROM t l LEFT JOIN t r ON l.a = r.a WHERE l.b = 2);
```
```
[INTERNAL_ERROR_ATTRIBUTE_NOT_FOUND] Could not find propagatedFilter_3#179
in
[sum#141L,max#182,count#145L,a#6,propagatedFilter_5#181,propagatedFilter_4#180].
SQLSTATE: XX000
```
What happens there:
1. Merging the 2nd subquery into the 1st aliases `l.b = 2` on the left side
of the join.
2. The 2nd subquery's post-join `coalesce(r.c, 10) > 5` folds that alias
into a new `AND` alias.
3. The 3rd subquery reuses the `l.b = 2` alias, but the `Project` under the
aggregate only outputs the new one.
This was found while reviewing #59161, which runs into the same problem
through its new join `AND`.
### Does this PR introduce _any_ user-facing change?
Yes. The query above now returns `12, 3, 6` instead of failing. It needs
both configs above, which are off by default. The bug is in 4.2.0.
### How was this patch tested?
New tests in `MergeSubplansSuite` (merged plans, including the computed
column case) and `PlanMergingSuite` (end to end). They all fail without the
fix. `MergeSubplansSuite` and `PlanMergingSuite` pass.
### 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]
