[GLUTEN-12566][CORE] Propagate keyGroupedPartitioning in BatchScanExecTransformer#12567
Open
wangyum wants to merge 3 commits into
Open
[GLUTEN-12566][CORE] Propagate keyGroupedPartitioning in BatchScanExecTransformer#12567wangyum wants to merge 3 commits into
wangyum wants to merge 3 commits into
Conversation
|
Run Gluten Clickhouse CI on x86 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes canonicalization and hashing behavior for BatchScanExecTransformer so that keyGroupedPartitioning is treated consistently with equality semantics, improving AQE plan/exchange reuse behavior when expression IDs differ.
Changes:
- Normalize
keyGroupedPartitioningduringdoCanonicalize()usingQueryPlan.normalizeExpressions. - Include
keyGroupedPartitioninginBatchScanExecTransformerBase.hashCode().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
69
to
73
| runtimeFilters.filterNot(_ == DynamicPruningExpression(Literal.TrueLiteral)), | ||
| output), | ||
| keyGroupedPartitioning = keyGroupedPartitioning.map( | ||
| QueryPlan.normalizeExpressions(_, output)), | ||
| pushDownFilters = pushDownFilters.map(QueryPlan.normalizePredicates(_, output)) |
wangyum
marked this pull request as draft
July 20, 2026 00:25
wangyum
force-pushed
the
fix-batchscan-canonicalize
branch
from
July 20, 2026 00:35
eaea5ff to
89029fb
Compare
wangyum
changed the title
wangyum
force-pushed
the
fix-batchscan-canonicalize
branch
from
July 20, 2026 01:09
89029fb to
f52a4da
Compare
wangyum
force-pushed
the
fix-batchscan-canonicalize
branch
from
July 20, 2026 01:47
f52a4da to
c3abbea
Compare
|
Run Gluten Clickhouse CI on x86 |
wangyum
force-pushed
the
fix-batchscan-canonicalize
branch
from
July 20, 2026 02:58
c3abbea to
621b65f
Compare
|
Run Gluten Clickhouse CI on x86 |
wangyum
force-pushed
the
fix-batchscan-canonicalize
branch
from
July 20, 2026 05:20
621b65f to
5371721
Compare
|
Run Gluten Clickhouse CI on x86 |
…upedPartitioning ScanTransformerFactory.createBatchScanTransformer() does not pass keyGroupedPartitioning from the original BatchScanExec to the BatchScanExecTransformer, leaving it as None even when the source BatchScanExec has SPJ (Storage Partitioned Join) partitioning. This causes Gluten's BatchScanExecTransformer to report UnknownPartitioning instead of KeyGroupedPartitioning, defeating SPJ and inserting redundant shuffles. IcebergScanTransformer and PaimonScanTransformer already pass keyGroupedPartitioning correctly; this fixes the same gap for regular file scans (Parquet, ORC, etc.) created via ScanTransformerFactory. Additionally fix two related issues in BatchScanExecTransformer that were masked by keyGroupedPartitioning always being None: 1. doCanonicalize() does not normalize keyGroupedPartitioning. When two semantically identical scans have different expression IDs in their keyGroupedPartitioning expressions, the canonicalized plans are not equal, preventing AQE exchange reuse. 2. hashCode() does not include keyGroupedPartitioning, violating the equals/hashCode contract (equals compares it via spjParams, but hashCode omits it). Closes apache#12566.
wangyum
force-pushed
the
fix-batchscan-canonicalize
branch
from
July 20, 2026 06:13
5371721 to
a667fbb
Compare
|
Run Gluten Clickhouse CI on x86 |
wangyum
marked this pull request as ready for review
July 20, 2026 08:43
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes are proposed in this pull request?
Fix three related bugs around
keyGroupedPartitioninginBatchScanExecTransformer:1.
ScanTransformerFactorydoes not propagatekeyGroupedPartitioning(main fix)ScanTransformerFactory.createBatchScanTransformer()creates aBatchScanExecTransformerwithout passingkeyGroupedPartitioningfrom the sourceBatchScanExec. This means Gluten's scan always reportsUnknownPartitioninginstead ofKeyGroupedPartitioning, defeating SPJ (Storage Partitioned Joins) and inserting redundant shuffles.IcebergScanTransformerandPaimonScanTransformeralready passkeyGroupedPartitioningcorrectly — this fixes the same gap for regular file scans (Parquet, ORC, etc.).2.
doCanonicalize()does not normalizekeyGroupedPartitioningWhen two semantically identical scans have different expression IDs in their
keyGroupedPartitioningexpressions, the canonicalized plans are not equal, preventing AQE exchange reuse.3.
hashCode()does not includekeyGroupedPartitioning— equals/hashCode contract violationequals()compareskeyGroupedPartitioning(viaspjParams) buthashCode()omits it.How was this patch tested?
Existing tests. The fix aligns
ScanTransformerFactorywithIcebergScanTransformer/PaimonScanTransformer(samegetKeyGroupedPartitioningshim call), and alignshashCode()/doCanonicalize()with the existingequals()behavior.Closes #12566.
Was this patch authored or co-authored using generative AI tooling?
No