ulysses-you commented on code in PR #57491:
URL: https://github.com/apache/spark/pull/57491#discussion_r3648282059
##########
sql/core/src/test/scala/org/apache/spark/sql/DataFrameSetOperationsSuite.scala:
##########
@@ -1659,6 +1659,187 @@ class DataFrameSetOperationsSuite extends
SharedSparkSession with AdaptiveSparkP
}
}
+ test("SPARK-58317: union partitioning - PartitioningCollection child
intersects to single") {
+ withSQLConf(
+ SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
+ SQLConf.PREFER_SORTMERGEJOIN.key -> "false") {
+ withTempView("t1", "t2", "t3", "t4") {
+ Seq((1, 2, 4), (1, 3, 5), (2, 2, 3)).toDF("c1", "c2",
"c3").createOrReplaceTempView("t1")
+ Seq((1, 9), (2, 9)).toDF("c1", "x").createOrReplaceTempView("t2")
+ Seq((1, 2, 4), (2, 4, 5), (3, 6, 7)).toDF("c1", "c2",
"c3").createOrReplaceTempView("t3")
+ Seq((1, 9), (3, 9)).toDF("c1", "y").createOrReplaceTempView("t4")
+
+ // The first branch is an inner shuffled-hash join and selects both
join keys (t1.c1 and
+ // t2.c1), so its output partitioning is a
PartitioningCollection(Hash(c1), Hash(c1#..))
+ // that a downstream ProjectExec cannot narrow to a single member. The
second branch is a
+ // left join, whose output partitioning is a single
HashPartitioning(c1). The union should
+ // intersect the two to a single HashPartitioning(c1) and let the
group-by skip a shuffle.
+ def unionDF: DataFrame = sql(
+ """SELECT c1, c2, c3, count(*) FROM (
+ | SELECT /*+ SHUFFLE_HASH(t2) */ t1.c1, t1.c2, t1.c3, t2.c1 AS k
+ | FROM t1 JOIN t2 ON t1.c1 = t2.c1
+ | UNION ALL
+ | SELECT /*+ SHUFFLE_HASH(t4) */ t3.c1, t3.c2, t3.c3, t3.c1 AS k
+ | FROM t3 LEFT JOIN t4 ON t3.c1 = t4.c1
+ |) GROUP BY c1, c2, c3
+ |""".stripMargin)
+
+ val correctResult = withSQLConf(SQLConf.UNION_OUTPUT_PARTITIONING.key
-> "false") {
+ unionDF.collect()
+ }
+
+ val shuffleNums = Seq(true, false).map { enabled =>
+ withSQLConf(
+ SQLConf.ADAPTIVE_EXECUTION_ENABLED.key -> "false",
+ SQLConf.UNION_OUTPUT_PARTITIONING.key -> enabled.toString) {
+ val union = unionDF
+ val unionExec = union.queryExecution.executedPlan.collect { case
u: UnionExec => u }
+ assert(unionExec.size == 1)
+
+ val partitioning = unionExec.head.outputPartitioning
+ if (enabled) {
+ assert(partitioning.isInstanceOf[HashPartitioning],
+ s"expected a HashPartitioning pass-through but got
$partitioning")
+ } else {
+ assert(partitioning.isInstanceOf[UnknownPartitioning])
+ }
+
+ checkAnswer(union, correctResult)
+ union.queryExecution.executedPlan.collect {
+ case s: ShuffleExchangeExec => s
+ }.size
+ }
+ }
+ // Enabling the pass-through removes the shuffle before the aggregate.
+ assert(shuffleNums.head + 1 == shuffleNums.last)
+ }
+ }
+ }
+
+ test("SPARK-58317: union partitioning - all PartitioningCollection children
pass through") {
+ withSQLConf(
+ SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
+ SQLConf.PREFER_SORTMERGEJOIN.key -> "false") {
+ withTempView("t1", "t2", "t3", "t4") {
+ Seq((1, 2), (2, 3)).toDF("c1", "c2").createOrReplaceTempView("t1")
+ Seq((1, 9), (2, 9)).toDF("c1", "x").createOrReplaceTempView("t2")
+ Seq((1, 2), (3, 4)).toDF("c1", "c2").createOrReplaceTempView("t3")
+ Seq((1, 9), (3, 9)).toDF("c1", "y").createOrReplaceTempView("t4")
+
+ // Both branches are inner shuffled-hash joins on a single key and
select both sides' join
+ // key (t1.c1 and t2.c1 AS k), so a downstream ProjectExec cannot
narrow either child's
+ // PartitioningCollection(Hash(c1), Hash(k)) to a single member. The
union should intersect
+ // to a PartitioningCollection carrying both members.
+ def unionDF: DataFrame = sql(
+ """SELECT /*+ SHUFFLE_HASH(t2) */ t1.c1, t2.c1 AS k
+ |FROM t1 JOIN t2 ON t1.c1 = t2.c1
+ |UNION ALL
+ |SELECT /*+ SHUFFLE_HASH(t4) */ t3.c1, t4.c1 AS k
+ |FROM t3 JOIN t4 ON t3.c1 = t4.c1
+ |""".stripMargin)
+
+ val correctResult = withSQLConf(SQLConf.UNION_OUTPUT_PARTITIONING.key
-> "false") {
+ unionDF.collect()
+ }
+
+ Seq(true, false).foreach { enabled =>
+ withSQLConf(
+ SQLConf.ADAPTIVE_EXECUTION_ENABLED.key -> "false",
+ SQLConf.UNION_OUTPUT_PARTITIONING.key -> enabled.toString) {
+ val union = unionDF
+ val unionExec = union.queryExecution.executedPlan.collect { case
u: UnionExec => u }
+ assert(unionExec.size == 1)
+
+ val partitioning = unionExec.head.outputPartitioning
+ if (enabled) {
+ assert(partitioning.isInstanceOf[PartitioningCollection],
+ s"expected a PartitioningCollection pass-through but got
$partitioning")
+ val members =
partitioning.asInstanceOf[PartitioningCollection].partitionings
+ assert(members.forall(_.isInstanceOf[HashPartitioning]))
+ assert(members.size == 2)
+ } else {
+ assert(partitioning.isInstanceOf[UnknownPartitioning])
+ }
+
+ checkAnswer(union, correctResult)
+ }
+ }
+ }
+ }
+ }
+
+ test("SPARK-58317: union partitioning - empty intersection falls back") {
+ withSQLConf(
+ SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
+ SQLConf.PREFER_SORTMERGEJOIN.key -> "false") {
+ withTempView("t1", "t2") {
+ Seq((1, 2, 4), (2, 3, 5)).toDF("c1", "c2",
"c3").createOrReplaceTempView("t1")
+ Seq((1, 9), (2, 9)).toDF("c1", "x").createOrReplaceTempView("t2")
+
+ // First branch reports PartitioningCollection(Hash(c1), Hash(c1#..));
the second branch
+ // is repartitioned on a disjoint column, so the intersection is empty.
+ def unionDF: DataFrame = sql(
+ """SELECT /*+ SHUFFLE_HASH(t2) */ t1.c1, t1.c2, t1.c3, t2.c1 AS k
+ |FROM t1 JOIN t2 ON t1.c1 = t2.c1
+ |UNION ALL
+ |SELECT c1, c2, c3, c2 AS k FROM t1 DISTRIBUTE BY c2
+ |""".stripMargin)
+
+ val correctResult = withSQLConf(SQLConf.UNION_OUTPUT_PARTITIONING.key
-> "false") {
+ unionDF.collect()
+ }
+
+ Seq(true, false).foreach { enabled =>
+ withSQLConf(
+ SQLConf.ADAPTIVE_EXECUTION_ENABLED.key -> "false",
+ SQLConf.UNION_OUTPUT_PARTITIONING.key -> enabled.toString) {
+ val union = unionDF
+ val unionExec = union.queryExecution.executedPlan.collect { case
u: UnionExec => u }
+ assert(unionExec.size == 1)
+
assert(unionExec.head.outputPartitioning.isInstanceOf[UnknownPartitioning])
+ checkAnswer(union, correctResult)
+ }
+ }
+ }
+ }
+ }
+
+ test("SPARK-58317: union partitioning - PartitioningCollection pass-through
under AQE") {
+ // AQE is enabled by default in production; the collection pass-through
must produce correct
+ // results there too, where the union output flows through the
coalesce-compatibility path.
+ withSQLConf(
+ SQLConf.ADAPTIVE_EXECUTION_ENABLED.key -> "true",
+ SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
+ SQLConf.PREFER_SORTMERGEJOIN.key -> "false") {
+ withTempView("t1", "t2", "t3", "t4") {
+ Seq((1, 2), (2, 3), (1, 4)).toDF("c1",
"c2").createOrReplaceTempView("t1")
+ Seq((1, 9), (2, 9)).toDF("c1", "x").createOrReplaceTempView("t2")
+ Seq((1, 2), (3, 4)).toDF("c1", "c2").createOrReplaceTempView("t3")
+ Seq((1, 9), (3, 9)).toDF("c1", "y").createOrReplaceTempView("t4")
+
+ def unionDF: DataFrame = sql(
+ """SELECT c1, count(*) FROM (
+ | SELECT /*+ SHUFFLE_HASH(t2) */ t1.c1, t2.c1 AS k
+ | FROM t1 JOIN t2 ON t1.c1 = t2.c1
+ | UNION ALL
+ | SELECT /*+ SHUFFLE_HASH(t4) */ t3.c1, t4.c1 AS k
+ | FROM t3 JOIN t4 ON t3.c1 = t4.c1
+ |) GROUP BY c1
Review Comment:
Applied in b79f1b2 -- `SELECT c1, k, ...` / `GROUP BY c1, k`, same reasoning
as above so the `PartitioningCollection` survives `ColumnPruning` under AQE.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/basicPhysicalOperators.scala:
##########
@@ -937,47 +940,76 @@ case class UnionExec(children: Seq[SparkPlan]) extends
SparkPlan with CodegenSup
}
override def outputPartitioning: Partitioning = {
- if (conf.getConf(SQLConf.UNION_OUTPUT_PARTITIONING)) {
- val partitionings = prepareOutputPartitioning()
- if (partitionings.forall(comparePartitioning(_, partitionings.head))) {
- val partitioner = partitionings.head
-
- // Take the output attributes of this union and map the partitioner to
them.
- val attributeMap = children.head.output.zip(output).toMap
- partitioner match {
- case headKp: KeyedPartitioning =>
- // A `UnionExec` concatenates its children's partitions in order
(one child's
- // partitions after another's), so the merged `KeyedPartitioning`
carries the
- // concatenation of the children's partition keys, one key per
physical output
- // partition. Children usually hold different key sets, so the
merged keys often
- // contain duplicates and `isGrouped` is false; a downstream
`GroupPartitionsExec`
- // regroups partitions that share a key. The children's
expressions have already
- // been remapped to the first child's attributes by
`prepareOutputPartitioning`;
- // here they are remapped to the union's output attributes.
- val mergedKeys = partitionings.flatMap {
- case k: KeyedPartitioning => k.partitionKeys
- case _ => return super.outputPartitioning
- }
- val mergedExpressions = headKp.expressions.map(_.transform {
- case a: Attribute if attributeMap.contains(a) => attributeMap(a)
- })
- val isGrouped = mergedKeys.distinct.size == mergedKeys.size
- val isNarrowed = partitionings.exists {
- case k: KeyedPartitioning => k.isNarrowed
- case _ => false
- }
- KeyedPartitioning(mergedExpressions, mergedKeys, isGrouped,
isNarrowed)
- case e: Expression =>
- e.transform {
- case a: Attribute if attributeMap.contains(a) => attributeMap(a)
- }.asInstanceOf[Partitioning]
- case _ => partitioner
- }
+ if (!conf.getConf(SQLConf.UNION_OUTPUT_PARTITIONING)) {
+ return super.outputPartitioning
+ }
+
+ // Children's partitionings with attributes remapped to the first child's
attributes.
+ val partitionings = prepareOutputPartitioning()
+ // Map from the first child's attributes to this union's own output
attributes.
+ val attributeMap = children.head.output.zip(output).toMap
+ def toUnionOutput(p: Partitioning): Partitioning = p match {
+ case e: Expression =>
+ e.transform {
+ case a: Attribute if attributeMap.contains(a) => attributeMap(a)
+ }.asInstanceOf[Partitioning]
+ case _ => p
+ }
+
+ // Case A: every child is a single `KeyedPartitioning`. A `UnionExec`
concatenates its
+ // children's partitions in order (one child's partitions after
another's), so the merged
+ // `KeyedPartitioning` carries the concatenation of the children's
partition keys, one key
+ // per physical output partition. Children usually hold different key
sets, so the merged
+ // keys often contain duplicates and `isGrouped` is false; a downstream
`GroupPartitionsExec`
+ // regroups partitions that share a key. This concatenation (numPartitions
= sum) is a
+ // distinct physical strategy from the co-located pass-through below
(numPartitions = N), so
+ // it is kept as a separate case and never folded into a
`PartitioningCollection`.
+ if (partitionings.forall(_.isInstanceOf[KeyedPartitioning])) {
+ val kps = partitionings.map(_.asInstanceOf[KeyedPartitioning])
+ val headKp = kps.head
+ // The `KeyedPartitioning`s must agree on the partition expressions to
merge.
+ val compatible = kps.forall(comparePartitioning(_, headKp))
+ if (compatible) {
+ val mergedKeys = kps.flatMap(_.partitionKeys)
+ val mergedExpressions = headKp.expressions.map(_.transform {
+ case a: Attribute if attributeMap.contains(a) => attributeMap(a)
+ })
+ val isGrouped = mergedKeys.distinct.size == mergedKeys.size
+ val isNarrowed = kps.exists(_.isNarrowed)
+ return KeyedPartitioning(mergedExpressions, mergedKeys, isGrouped,
isNarrowed)
} else {
- super.outputPartitioning
+ return super.outputPartitioning
}
- } else {
- super.outputPartitioning
+ }
+
+ // Case B: treat each child's partitioning as a set of candidate
partitionings (a
+ // `PartitioningCollection` flattens to its members; a single partitioning
is a one-element
+ // set) and pass through the intersection across all children. Only
index-co-locatable
+ // partitionings participate; `KeyedPartitioning` is excluded here because
its concatenation
+ // semantics (Case A) are incompatible with the co-located union RDD.
+ def flattenPartitioning(p: Partitioning): Seq[Partitioning] = p match {
Review Comment:
Done in b79f1b2. Rather than depend on the `sql/core` helper from catalyst,
I lifted a shared `PartitioningCollection.flatten` into the catalyst companion
object and reused it from both `UnionExec` and
`PartitioningPreservingUnaryExecNode`, removing the duplication.
--
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]
