weiting-chen commented on code in PR #13243:
URL: https://github.com/apache/gluten/pull/13243#discussion_r4271688056
##########
gluten-ut/spark42/src/test/scala/org/apache/spark/sql/connector/GlutenKeyGroupedPartitioningSuite.scala:
##########
@@ -359,8 +370,11 @@ class GlutenKeyGroupedPartitioningSuite
val shuffles =
collectColumnarShuffleExchangeExec(df.queryExecution.executedPlan)
Review Comment:
**Optional: check both shuffle implementations below the join**
**Target Location:**
`gluten-ut/spark42/src/test/scala/org/apache/spark/sql/connector/GlutenKeyGroupedPartitioningSuite.scala:370-379`,
and the analogous restored SPJ copies.
**Problem:** The assertion says there is no shuffle, but this collector sees
only columnar exchanges. In the `pushDownValues=false` iteration, the grouping
check is skipped entirely: a future fallback could introduce a vanilla
`ShuffleExchangeExec` below the join, preserve the answer, and still pass this
absence check. This is a **nonblocking, inherited test-oracle gap**, not
evidence of a current extra shuffle or wrong result. Confidence: high.
**Evidence:**
```scala
val shuffles =
collectColumnarShuffleExchangeExec(df.queryExecution.executedPlan)
assert(shuffles.isEmpty, "should not contain any shuffle")
```
The updated grouping assertions run only under `if (pushDownValues)`. The
existing `collectShuffles` override at lines 92-101 already handles both Spark
and Gluten shuffle exchanges beneath either join representation.
**Suggested Fix:**
```scala
val shuffles = collectShuffles(df.queryExecution.executedPlan)
assert(shuffles.isEmpty, "should not contain any shuffle")
```
Use the **join-scoped** helper, rather than global `collectAllShuffles`: the
`ORDER BY` outside the join can legitimately need its own exchange. Keep
explicitly native-only checks elsewhere unchanged. Apply the same substitution
to the analogous restored no-shuffle assertions and rerun those SPJ variants.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
