You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "sam-1112 (via GitHub)" <gi...@apache.org> on 2026/09/23 10:37:06 UTC
[PR] fix: preserve DPP filters during transition revert [datafusion-comet]
sam-1112 opened a new pull request, #6137:
URL: https://github.com/apache/datafusion-comet/pull/6137
## Which issue does this PR close?
Closes #5486.
## Rationale for this change
With AQE and dynamic partition pruning enabled, the query-stage optimizer rewrites the DPP placeholder in the live `CometNativeScanExec` into an executable broadcast subquery.
`RevertNativeForTransitionHeavyStages` runs later as a post-columnar rule. When reverting a native V1 scan, it restored the frozen `FileSourceScanExec` from `originalPlan`. That plan still contained the original `SubqueryAdaptiveBroadcastExec`, which cannot be executed directly and caused:
```text
SubqueryAdaptiveBroadcastExec does not support the execute() code path
```
## What changes are included in this PR?
- Preserve the live partitionFilters and dataFilters when reverting a CometNativeScanExec to its original FileSourceScanExec.
- Preserve the original scan's logical link.
- Add a V1 Parquet regression test covering:
- AQE enabled and disabled.
- Transition reversion enabled and disabled.
- Native project execution enabled and disabled.
- Verify that all eight configurations return 400 rows.
- When AQE and transition reversion are enabled, verify that:
- The scan is reverted to FileSourceScanExec.
- DPP retains an executable SubqueryBroadcastExec or CometSubqueryBroadcastExec.
- No unexecutable SubqueryAdaptiveBroadcastExec remains.
## How are these changes tested?
Built the native library:
```shell
make core
```
Ran the complete transition-reversion suite on the default Spark 4.1.3 profile:
```
./mvnw test -Dtest=none -Dsuites="org.apache.comet.rules.RevertNativeForTransitionHeavyStagesSuite"
```
Result: 16 tests passed.
Ran the new regression test on Spark 3.5:
```
./mvnw test -Pspark-3.5 -Dtest=none \
-Dsuites="org.apache.comet.rules.RevertNativeForTransitionHeavyStagesSuite AQE DPP remains
executable when transition reversion restores a V1 scan" \
-Dscalastyle.skip=true
```
Result: 1 test passed.
Ran the existing V1 DPP broadcast-join tests:
```
./mvnw test -Dtest=none \
-Dsuites="org.apache.comet.exec.CometExecSuite AQE DPP: BHJ works with CometNativeScanExec" \
-Dscalastyle.skip=true
```
Result: 2 tests passed.
The full Spark 4.1.3 suite invocation also completed the configured Spotless and Scalastyle checks successfully.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: preserve DPP filters during transition revert [datafusion-comet]
Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao merged PR #6137:
URL: https://github.com/apache/datafusion-comet/pull/6137
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: preserve DPP filters during transition revert [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #6137:
URL: https://github.com/apache/datafusion-comet/pull/6137#issuecomment-5876194635
This is a light fully automated review since there are so many PRs open.
The new `CometNativeScanExec` case in `revertToSpark` fixes the V1 Parquet path, but `CometIcebergNativeScanExec` still falls into the generic `case _ => cometExec.originalPlan` branch right below it (`spark/src/main/scala/org/apache/comet/rules/RevertNativeForTransitionHeavyStages.scala:202-214`), and it looks like it hits the same bug this PR closes. `CometIcebergNativeScanExec.originalPlan` is `@transient` and, per the comment in `spark/src/main/scala/org/apache/spark/sql/comet/CometIcebergNativeScanExec.scala:96-105`, is never touched when `CometPlanAdaptiveDynamicPruningFilters` rewrites the live scan's top-level `runtimeFilters`, so it keeps the pre-DPP `SubqueryAdaptiveBroadcastExec` placeholder. `serializedPartitionData` works around this by rebuilding `effectiveOriginalPlan = originalPlan.copy(runtimeFilters = runtimeFilters)` before reading it, but `revertToSpark` restores the stale `originalPlan` as-is. With AQE, dynamic partition pruning, `spark.comet.exec.transitionRe
vert.enabled`, and a native Iceberg scan (`spark.comet.scan.icebergNative.enabled` defaults to `true`) in the reverted stage, wouldn't this reproduce the same `SubqueryAdaptiveBroadcastExec does not support the execute()` failure, just for the wrapped `BatchScanExec` instead of `FileSourceScanExec`? Would it make sense to extend this case, or generalize it, to cover `CometIcebergNativeScanExec` too?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org