ulysses-you commented on code in PR #58279:
URL: https://github.com/apache/spark/pull/58279#discussion_r3932344324
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/EnsureRequirements.scala:
##########
@@ -634,11 +642,16 @@ case class EnsureRequirements(
|""".stripMargin)
leftLink.get.stats.sizeInBytes < rightLink.get.stats.sizeInBytes
} else {
- // As a simple heuristic, we pick the side with fewer number of
partitions
- // to apply the grouping & replication of partitions
+ // As a simple heuristic, we pick the side with fewer number of
partitions to
+ // apply the grouping & replication of partitions. The counts
read the
+ // pre-alignment plans, for the same reason the statistics do:
on a re-run both
+ // aligned reports hold the same number of keys, so comparing
them decides nothing.
logInfo("Using number of partitions to determine which side of
join " +
"to fully cluster partition values")
- leftPartKeys.size < rightPartKeys.size
+
PartitioningCollection.numKeyedPartitions(unwrappedLeft.outputPartitioning)
Review Comment:
Done -- the comment names the first-pass change verbatim, and the test
gained two bare first-pass arms: three splits under one distinct key against
two splits under two keys, where the pre-alignment count replicates the side
with fewer splits (revert-verified: the old distinct-key count picks the other
side), plus a control where both counts agree. 77c17e98254
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/physical/partitioning.scala:
##########
@@ -1133,6 +1133,14 @@ object PartitioningCollection {
case _ => None
}
+ /**
+ * The number of partitions of the [[KeyedPartitioning]] representing the
keyed members of
+ * `partitioning`, if any. Collections validate on construction that their
keyed members agree,
+ * so the representative's count stands for all of them.
+ */
+ def numKeyedPartitions(partitioning: Partitioning): Option[Int] =
Review Comment:
Done -- narrowed to `private[sql]`, and the scaladoc now states the stronger
fact: the collection's `require` already unifies the count across all members,
so the representative only decides whether a keyed member exists. 77c17e98254
--
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]
