dongjoon-hyun commented on code in PR #58445:
URL: https://github.com/apache/spark/pull/58445#discussion_r3914843635
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/basicPhysicalOperators.scala:
##########
@@ -1259,7 +1259,16 @@ case class UnionExec(children: Seq[SparkPlan]) extends
SparkPlan with CodegenSup
override def supportsRowBased: Boolean = children.forall(_.supportsRowBased)
protected override def doExecuteColumnar(): RDD[ColumnarBatch] = {
- sparkContext.union(children.map(_.executeColumnar()))
+ // Same split as `doExecute`: a union that reports an index-co-locatable
partitioning has to
+ // interleave same-index partitions, or a parent that skipped an exchange
on that report reads
+ // a concatenation instead.
+ outputPartitioning match {
Review Comment:
This is a copy of `doExecute`'s dispatch with `execute()` swapped for
`executeColumnar()`, and the bug this PR fixes is exactly the two copies
drifting: SPARK-52921 added the split only to `doExecute` while
`doExecuteColumnar` already existed, and SPARK-57881 added the
`KeyedPartitioning` arm only there. Since both `SQLPartitioningAwareUnionRDD`
and `SparkContext.union` are generic over `T: ClassTag`, how about one helper
shared by both paths so they cannot drift again?
```scala
private def unionRDDs[T: ClassTag](rdds: Seq[RDD[T]]): RDD[T] =
outputPartitioning match {
case _: UnknownPartitioning | _: KeyedPartitioning =>
// (existing comment from doExecute)
sparkContext.union(rdds)
case partitioning =>
// (existing comment from doExecute)
new SQLPartitioningAwareUnionRDD(
sparkContext, rdds.filter(!_.partitions.isEmpty),
partitioning.numPartitions)
}
protected override def doExecute(): RDD[InternalRow] =
unionRDDs(children.map(_.execute()))
protected override def doExecuteColumnar(): RDD[ColumnarBatch] =
unionRDDs(children.map(_.executeColumnar()))
```
It needs `import scala.reflect.ClassTag`, and the "Same split as
`doExecute`" comment can go away since there is only one split left.
--
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]
