szehon-ho commented on code in PR #53098:
URL: https://github.com/apache/spark/pull/53098#discussion_r2535284424
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/EnsureRequirements.scala:
##########
@@ -140,6 +140,13 @@ case class EnsureRequirements(
// Choose all the specs that can be used to shuffle other children
val candidateSpecs = specs
.filter(_._2.canCreatePartitioning)
+ .filter {
Review Comment:
just for reference , were both checks needed? ie this and the other check
in 'checkKeyGroupCompatible'
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/EnsureRequirements.scala:
##########
@@ -140,6 +140,13 @@ case class EnsureRequirements(
// Choose all the specs that can be used to shuffle other children
val candidateSpecs = specs
.filter(_._2.canCreatePartitioning)
+ .filter {
+ // To choose a KeyGroupedShuffleSpec, we must be able to push down
SPJ parameters into
+ // the scan (for join key positions). If these parameters can't be
pushed down, this
+ // spec can't be used to shuffle other children.
+ case (idx, _: KeyGroupedShuffleSpec) =>
canPushDownSPJParamsToScan(children(idx))
Review Comment:
nit: extra space after '=>'
--
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]
