You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "peter-toth (via GitHub)" <gi...@apache.org> on 2026/10/08 19:36:44 UTC
[PR] [SPARK-60084][SQL][4.3] Fix PlanMerger losing a reused filter attribute before the aggregate [spark]
peter-toth opened a new pull request, #59307:
URL: https://github.com/apache/spark/pull/59307
### Differences from #59294
This is the branch-4.3 backport of #59294. The description below is the original one.
- The cherry-pick applied cleanly, nothing was tailored.
- On branch-4.3 the 4 new tests fail without the fix. With it, `MergeSubplansSuite` and `PlanMergingSuite` pass (92 tests), 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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60084][SQL][4.3] Fix PlanMerger losing a reused filter attribute before the aggregate [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on PR #59307:
URL: https://github.com/apache/spark/pull/59307#issuecomment-6077190684
**Merge Summary:**
- merged into branch-4.3 https://github.com/apache/spark/commit/3789a804fc8ab814e714ca8c49e1743a35eac14b
*Posted by `merge_spark_pr.py`*
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60084][SQL][4.3] Fix PlanMerger losing a reused filter attribute before the aggregate [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on PR #59307:
URL: https://github.com/apache/spark/pull/59307#issuecomment-6072354773
(same comment as #59308)
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60084][SQL][4.3] Fix PlanMerger losing a reused filter attribute before the aggregate [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth closed pull request #59307: [SPARK-60084][SQL][4.3] Fix PlanMerger losing a reused filter attribute before the aggregate
URL: https://github.com/apache/spark/pull/59307
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60084][SQL][4.3] Fix PlanMerger losing a reused filter attribute before the aggregate [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on PR #59307:
URL: https://github.com/apache/spark/pull/59307#issuecomment-6077225195
Thanks @szehon-ho for the review.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org