ulysses-you commented on code in PR #58339:
URL: https://github.com/apache/spark/pull/58339#discussion_r3932351297


##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/GroupPartitionsExec.scala:
##########
@@ -197,7 +214,9 @@ case class GroupPartitionsExec(
 
     // Both cheap terms come first, so the scan below runs only where a merge 
is possible. A
     // grouping that left the keys as they are groups the child's own key 
values, and one of those
-    // groups can only ever cover the one key it was built from.
+    // groups can only ever cover the one key it was built from. `keysChanged` 
also feeds
+    // `identityGrouping`: a projection or reduction re-labels the groups into 
a different key
+    // space, which no index alignment can undo.
     val keysChanged =
       joinKeyPositions.exists(_.length < childKp.expressions.length) || 
reducers.isDefined

Review Comment:
   Fixed in 96bb712a30a: `identityGrouping` now asks the stronger question via 
`keysRewritten` -- any projection that differs from the identity positions, or 
any reducer slot. A reordering projection merges no key, so `keysChanged` stays 
local to `grouping` where `isCollapsed` wants the narrower reading. Pinned by a 
`Seq(1, 0)` projection over keys `[(1, 8), (2, 9)]` that keeps every group at 
its index; the test fails without the clause.



-- 
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]

Reply via email to