You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "hemanthboyina (via GitHub)" <gi...@apache.org> on 2026/09/02 10:01:30 UTC
[PR] [SPARK-59141][SQL] Columnar UnionExec ignores outputPartitioning and returns wrong results [spark]
hemanthboyina opened a new pull request, #58473:
URL: https://github.com/apache/spark/pull/58473
**What changes were proposed in this pull request?**
UnionExec.doExecuteColumnar now gets the same partitioning split that doExecute already has: a union whose outputPartitioning is index-co-locatable builds a SQLPartitioningAwareUnionRDD that interleaves same-index partitions across children, and only an UnknownPartitioning or KeyedPartitioning union concatenates. SQLPartitioningAwareUnionRDD is generic over its element type, so it works for `RDD[ColumnarBatch]` with no new RDD type.
**Why are the changes needed?**
Wrong results on the default configuration. `doExecuteColumnar` concatenated its children unconditionally while `outputPartitioning` kept reporting the partitioning the children agreed on, so a parent that dropped its exchange on the strength of that report read a concatenation instead of an interleaving.
A union of two columnar-only bucketed scans reports `HashPartitioning(k, 4)`, which lets a downstream `GROUP BY k` skip its shuffle; the concatenated columnar union then puts each key in two partitions, so the shuffle-free aggregate emits each group twice.
**Does this PR introduce _any_ user-facing change?**
Yes, it fixes wrong results. Any union of columnar children that reports an index-co-locatable partitioning was affected.
**How was this patch tested?**
A new case in `DataFrameSetOperationsSuite` builds a union of two bucketed (columnar-only) scans feeding a shuffle-free aggregate, asserts the union is columnar-only, reports a `HashPartitioning`, and has no exchange, then compares against the same query with `spark.sql.unionOutputPartitioning` off. It fails with ten rows where five are expected without the fix and passes with it. `DataFrameSetOperationsSuite`, `UnionCodegenSuite`, `BucketedReadWithoutHiveSupportSuite` and `KeyGroupedPartitioningSuite` pass.
**Was this patch authored or co-authored using generative AI tooling?**
Yes, used Claude code
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59141][SQL] Columnar UnionExec ignores outputPartitioning and returns wrong results [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #58473:
URL: https://github.com/apache/spark/pull/58473#issuecomment-5509710967
Thank you @hemanthboyina
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59141][SQL] Columnar UnionExec ignores outputPartitioning and returns wrong results [spark]
Posted by "hemanthboyina (via GitHub)" <gi...@apache.org>.
hemanthboyina closed pull request #58473: [SPARK-59141][SQL] Columnar UnionExec ignores outputPartitioning and returns wrong results
URL: https://github.com/apache/spark/pull/58473
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59141][SQL] Columnar UnionExec ignores outputPartitioning and returns wrong results [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #58473:
URL: https://github.com/apache/spark/pull/58473#discussion_r3914096409
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/basicPhysicalOperators.scala:
##########
@@ -1255,7 +1255,17 @@ 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()))
+ // Mirror `doExecute`: a union whose `outputPartitioning` is index-co-locatable must interleave
+ // same-index partitions so it actually delivers the partitioning it reports. Only an
+ // `UnknownPartitioning` or `KeyedPartitioning` union concatenates its children.
+ outputPartitioning match {
+ case _: UnknownPartitioning | _: KeyedPartitioning =>
+ sparkContext.union(children.map(_.executeColumnar()))
+ case _ =>
Review Comment:
outputPartitioning is called twice: once as the match scrutinee and once inside the arm for .numPartitions. The two calls recompute from children on every invocation, so if a child's reported partitioning changes between them (the precise scenario documented by PR #58419 for InMemoryTableScanExec under AQE - UnknownPartitioning(0) before isFinalPlan, then HashPartitioning after), the first call could route to case _ while the second call returns numPartitions = 0, constructing a SQLPartitioningAwareUnionRDD with zero partitions and producing an empty result. PR #58419 applies the same single-capture fix to doExecute (val partitioning = outputPartitioning; partitioning match { ... SQLPartitioningAwareUnionRDD(sc, rdds, partitioning.numPartitions) }). The new doExecuteColumnar should use the same pattern for consistency and to stay correct once #58419 lands.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59141][SQL] Columnar UnionExec ignores outputPartitioning and returns wrong results [spark]
Posted by "hemanthboyina (via GitHub)" <gi...@apache.org>.
hemanthboyina commented on PR #58473:
URL: https://github.com/apache/spark/pull/58473#issuecomment-5509704443
my mistake @LuciferYang haven't observed your PR, closing mine
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59141][SQL] Columnar UnionExec ignores outputPartitioning and returns wrong results [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #58473:
URL: https://github.com/apache/spark/pull/58473#issuecomment-5508798448
In fact, I already submitted the fix PR yesterday.
- https://github.com/apache/spark/pull/58445
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59141][SQL] Columnar UnionExec ignores outputPartitioning and returns wrong results [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on PR #58473:
URL: https://github.com/apache/spark/pull/58473#issuecomment-5510468444
oh sorry I haven't seen @LuciferYang's PR above!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org