You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "wangyum (via GitHub)" <gi...@apache.org> on 2026/07/19 16:46:11 UTC

[PR] [GLUTEN-12566][CORE] Fix BatchScanExecTransformer hashCode/canonicalize to include keyGroupedPartitioning [gluten]

wangyum opened a new pull request, #12567:
URL: https://github.com/apache/gluten/pull/12567

   ## What changes are proposed in this pull request?
   
   Fix `BatchScanExecTransformer.doCanonicalize()` and `BatchScanExecTransformerBase.hashCode()` to include `keyGroupedPartitioning`.
   
   Currently `doCanonicalize()` normalizes `output`, `runtimeFilters`, and `pushDownFilters` but skips `keyGroupedPartitioning`. And `hashCode()` computes `Objects.hashCode(batch, runtimeFilters, pushDownFilters)` — also missing `keyGroupedPartitioning`. This is inconsistent with `equals()`, which (via `super.equals()` → `AbstractBatchScanExec.equals()`) compares `spjParams` that contains `keyGroupedPartitioning`.
   
   **Impact:**
   1. **Broken exchange reuse (AQE):** Two semantically identical `BatchScanExecTransformer` instances with different expression IDs in `keyGroupedPartitioning` are not equalized after `doCanonicalize()`, preventing AQE from reusing the exchange — resulting in a redundant shuffle.
   2. **equals/hashCode contract violation:** `equals()` compares `keyGroupedPartitioning` (via `spjParams`) but `hashCode()` does not include it.
   
   ## How was this patch tested?
   
   Existing tests. The fix aligns `hashCode()` and `doCanonicalize()` with the existing `equals()` behavior — no new test cases needed.
   
   Closes #12566.
   
   ---
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   No


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-12566][CORE] Propagate keyGroupedPartitioning in BatchScanExecTransformer [gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12567:
URL: https://github.com/apache/gluten/pull/12567#issuecomment-5019036585

   Run Gluten Clickhouse CI on x86


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-12566][CORE] Propagate keyGroupedPartitioning in BatchScanExecTransformer [gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12567:
URL: https://github.com/apache/gluten/pull/12567#issuecomment-5021068310

   Run Gluten Clickhouse CI on x86


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-12566][CORE] Propagate keyGroupedPartitioning in BatchScanExecTransformer [gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12567:
URL: https://github.com/apache/gluten/pull/12567#issuecomment-5018348089

   Run Gluten Clickhouse CI on x86


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-12566][CORE] Propagate keyGroupedPartitioning in BatchScanExecTransformer [gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12567:
URL: https://github.com/apache/gluten/pull/12567#issuecomment-5018090282

   Run Gluten Clickhouse CI on x86


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-12566][CORE] Fix BatchScanExecTransformer hashCode/canonicalize to include keyGroupedPartitioning [gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12567:
URL: https://github.com/apache/gluten/pull/12567#issuecomment-5016545215

   Run Gluten Clickhouse CI on x86


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-12566][CORE] Propagate keyGroupedPartitioning in BatchScanExecTransformer [gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12567:
URL: https://github.com/apache/gluten/pull/12567#issuecomment-5020785008

   Run Gluten Clickhouse CI on x86


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-12566][CORE] Fix BatchScanExecTransformer hashCode/canonicalize to include keyGroupedPartitioning [gluten]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #12567:
URL: https://github.com/apache/gluten/pull/12567#discussion_r3610958648


##########
gluten-substrait/src/main/scala/org/apache/gluten/execution/BatchScanExecTransformer.scala:
##########
@@ -68,6 +68,8 @@ case class BatchScanExecTransformer(
       runtimeFilters = QueryPlan.normalizePredicates(
         runtimeFilters.filterNot(_ == DynamicPruningExpression(Literal.TrueLiteral)),
         output),
+      keyGroupedPartitioning = keyGroupedPartitioning.map(
+        QueryPlan.normalizeExpressions(_, output)),
       pushDownFilters = pushDownFilters.map(QueryPlan.normalizePredicates(_, output))

Review Comment:
   Per Gluten's contribution guidance, a regression test should be added for this bug fix if it isn't already covered. I couldn't find any existing unit test that exercises BatchScanExecTransformer canonicalization/equality specifically for keyGroupedPartitioning (no matches for keyGroupedPartitioning / KeyGroupedPartitioning-related canonicalization in gluten-ut), so this change may be unguarded against regressions in AQE exchange reuse.



-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-12566][CORE] Propagate keyGroupedPartitioning in BatchScanExecTransformer [gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12567:
URL: https://github.com/apache/gluten/pull/12567#issuecomment-5019300734

   Run Gluten Clickhouse CI on x86


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-12566][CORE] Propagate keyGroupedPartitioning in BatchScanExecTransformer [gluten]

Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on PR #12567:
URL: https://github.com/apache/gluten/pull/12567#issuecomment-5059682264

   cc @jackylee-ch 


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-12566][CORE] Propagate keyGroupedPartitioning in BatchScanExecTransformer [gluten]

Posted by "zhouyuan (via GitHub)" <gi...@apache.org>.
zhouyuan merged PR #12567:
URL: https://github.com/apache/gluten/pull/12567


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org