You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "andygrove (via GitHub)" <gi...@apache.org> on 2026/01/21 19:29:47 UTC
[PR] fix: CometBroadcastExchangeExec should respect AQE shuffle partition coalescing [datafusion-comet]
andygrove opened a new pull request, #3235:
URL: https://github.com/apache/datafusion-comet/pull/3235
## Summary
- Fix `CometBroadcastExchangeExec` to properly use AQE's coalesced shuffle partition specs
- When `AQEShuffleReadExec` wraps a `CometShuffleExchangeExec`, use `getShuffleRDD(partitionSpecs)` to get the coalesced RDD instead of bypassing AQE
## Problem
When AQE (Adaptive Query Execution) coalesces shuffle partitions based on runtime statistics, `CometBroadcastExchangeExec` was bypassing this optimization by reading directly from the inner shuffle plan. This caused broadcast exchanges to spawn many tasks (e.g., 200) even when AQE determined that only 1 task was needed due to small data volume after filtering.
For example, in TPC-H Q18, after the filter `sum(l_quantity) > 313`, there are only ~900 rows to broadcast. AQE correctly identifies this and coalesces 200 shuffle partitions into 1. However, Comet's broadcast exchange was ignoring this and still reading from all 200 original partitions.
## Solution
Added proper handling for `AQEShuffleReadExec` wrapping `CometShuffleExchangeExec`:
1. Extract partition specs from `AQEShuffleReadExec`
2. Use `CometShuffleExchangeExec.getShuffleRDD(partitionSpecs)` to get the coalesced RDD
3. Serialize batches from the coalesced RDD for broadcasting
## Test plan
- [x] Existing unit tests pass (`CometExecSuite`, `CometShuffleSuite`)
- [x] Verified with TPC-H Q18: broadcast stages now use 1 task instead of 200 when AQE coalesces partitions
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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: CometBroadcastExchangeExec should respect AQE shuffle partition coalescing [datafusion-comet]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #3235: fix: CometBroadcastExchangeExec should respect AQE shuffle partition coalescing
URL: https://github.com/apache/datafusion-comet/pull/3235
--
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: CometBroadcastExchangeExec should respect AQE shuffle partition coalescing [datafusion-comet]
Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich commented on PR #3235:
URL: https://github.com/apache/datafusion-comet/pull/3235#issuecomment-3785498306
Should we run TPC-H benchmarks on this one?
--
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: CometBroadcastExchangeExec should respect AQE shuffle partition coalescing [datafusion-comet]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3235:
URL: https://github.com/apache/datafusion-comet/pull/3235#issuecomment-3781065367
## [Codecov](https://app.codecov.io/gh/apache/datafusion-comet/pull/3235?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `13 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 9.82%. Comparing base ([`f09f8af`](https://app.codecov.io/gh/apache/datafusion-comet/commit/f09f8af64c6599255e116a376f4f008f2fd63b43?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`ed7b8e0`](https://app.codecov.io/gh/apache/datafusion-comet/commit/ed7b8e0ea30112b2f8a25798533d3b50c1752838?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 864 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion-comet/pull/3235?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...e/spark/sql/comet/CometBroadcastExchangeExec.scala](https://app.codecov.io/gh/apache/datafusion-comet/pull/3235?src=pr&el=tree&filepath=spark%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fspark%2Fsql%2Fcomet%2FCometBroadcastExchangeExec.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3Bhcmsvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9zcGFyay9zcWwvY29tZXQvQ29tZXRCcm9hZGNhc3RFeGNoYW5nZUV4ZWMuc2NhbGE=) | 0.00% | [13 Missing :warning: ](https://app.codecov.io/gh/apache/datafusion-comet/pull/3235?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #3235 +/- ##
============================================
- Coverage 56.12% 9.82% -46.31%
+ Complexity 976 234 -742
============================================
Files 119 170 +51
Lines 11743 15773 +4030
Branches 2251 2585 +334
============================================
- Hits 6591 1550 -5041
- Misses 4012 14046 +10034
+ Partials 1140 177 -963
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/datafusion-comet/pull/3235?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: CometBroadcastExchangeExec should respect AQE shuffle partition coalescing [datafusion-comet]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on PR #3235:
URL: https://github.com/apache/datafusion-comet/pull/3235#issuecomment-3802695772
In what scenarios will the CoalesceShufflePartitions rule be applied to BroadcastExchange? Could you share the spark plan?
--
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: CometBroadcastExchangeExec should respect AQE shuffle partition coalescing [datafusion-comet]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3235:
URL: https://github.com/apache/datafusion-comet/pull/3235#issuecomment-4146396590
Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.
--
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