You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "tokoko (via GitHub)" <gi...@apache.org> on 2026/09/03 20:52:46 UTC
[PR] [SPARK-53618][SQL] Turn a FULL JOIN with a false condition into a UNI… [spark]
tokoko opened a new pull request, #58506:
URL: https://github.com/apache/spark/pull/58506
### What changes were proposed in this pull request?
`PropagateEmptyRelation` already rewrites a join whose condition is `FalseLiteral` for most join types: inner/cross and left semi collapse to an empty relation for example. This PR adds `FullOuter` handling. With a false condition no row on either side can find a match, so the join is equivalent to a UNION ALL of both sides, each padded with nulls for the other side's columns.
### Why are the changes needed?
1 = 0 is not an equi-join condition, so today this plan runs as a BroadcastNestedLoopJoin which is inefficient. After the rewrite both sides are scanned once and fuse into a single whole-stage codegen stage, with no exchange.
Users can usually write the union themselves, but not always. The motivating case is a MERGE with an intentionally non-matching condition, where the join is generated by the MERGE rewrite rather than written by hand:
```
MERGE INTO target_table AS t
USING source AS s ON 1 = 0
WHEN NOT MATCHED THEN INSERT *
WHEN NOT MATCHED BY SOURCE AND [replaceWhereCondition] THEN DELETE
```
This is the shape for a replaceWhere-style operation made possible by #52185.
### Does this PR introduce _any_ user-facing change?
No. Query results are unchanged; only the optimized plan changes.
### How was this patch tested?
New and updated unit tests.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code 2.1.259 (Claude Opus 5)
--
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-53618][SQL] Turn a FULL JOIN with a false condition into a UNION ALL [spark]
Posted by "tokoko (via GitHub)" <gi...@apache.org>.
tokoko commented on PR #58506:
URL: https://github.com/apache/spark/pull/58506#issuecomment-6144415128
@cloud-fan I looked into it. The reuse loss can happen, but I think it's too unlikely to change the overall picture.
It needs several false full outer joins whose broadcast sides are the same subplan, and that subplan must have no exchange of its own (a shuffle inside it is still reused). Even then, the plan being replaced is not cheap: BroadcastNestedLoopJoinExec executes the streamed side twice per full outer join and evaluates the condition for every row pair, even when it is false. The broadcast side is likely small, so the extra read usually won't matter. I'd really have to engineer a scenario where the broadcast side depends on some exceptionally expensive computation for the lost reuse to outweigh the gain.
Lastly, the existing false-condition LeftOuter/RightOuter rewrites in this rule drop the broadcast exchange in the same way, so this follows the rule's current behavior.
I can benchmark a few scenarios if you think it's worthwhile, though I'm not sure how much it would add.
--
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-53618][SQL] Turn a FULL JOIN with a false condition into a UNION ALL [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #58506:
URL: https://github.com/apache/spark/pull/58506#issuecomment-6126263071
Thanks for the ping. I have reviewed the current head.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"conversation:5774780296","thread_id":"conversation:5774780296","verdict_sha256":"a0582cbf2d4df87c8b7457be2fa0508cbeb5db6fda0555323f2b323de0a637b7"} -->
--
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-53618][SQL] Turn a FULL JOIN with a false condition into a UNION ALL [spark]
Posted by "tokoko (via GitHub)" <gi...@apache.org>.
tokoko commented on code in PR #58506:
URL: https://github.com/apache/spark/pull/58506#discussion_r4256687470
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/PropagateEmptyRelation.scala:
##########
@@ -123,6 +130,13 @@ abstract class PropagateEmptyRelationBase extends Rule[LogicalPlan] with CastSup
Project(p.left.output ++ nullValueProjectList(p.right), p.left)
case RightOuter if isFalseCondition && canExecuteWithoutJoin(p.right) =>
Project(nullValueProjectList(p.left) ++ p.right.output, p.right)
+ // No row of either side can find a match, so the join degenerates into both sides
+ // padded with nulls and concatenated.
+ case FullOuter if isFalseCondition && canExecuteWithoutJoin(p.left) &&
Review Comment:
added the test with `PropagateEmptyRelation` excluded.
--
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-53618][SQL] Turn a FULL JOIN with a false condition into a UNION ALL [spark]
Posted by "tokoko (via GitHub)" <gi...@apache.org>.
tokoko commented on code in PR #58506:
URL: https://github.com/apache/spark/pull/58506#discussion_r3932774896
##########
sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala:
##########
@@ -1838,6 +1838,20 @@ class JoinSuite extends SharedSparkSession with AdaptiveSparkPlanHelper
cached.unpersist()
}
}
+
+ test("SPARK-53618: full outer join with a false condition is rewritten to a union") {
Review Comment:
Added another test for MERGE case
##########
sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala:
##########
@@ -1838,6 +1838,20 @@ class JoinSuite extends SharedSparkSession with AdaptiveSparkPlanHelper
cached.unpersist()
}
}
+
+ test("SPARK-53618: full outer join with a false condition is rewritten to a union") {
+ val df = sql(
+ """
+ |SELECT t1.id AS a, t2.id AS b
+ |FROM range(0, 2) t1 FULL OUTER JOIN range(10, 12) t2 ON 1 = 0
+ |""".stripMargin)
+
+ val optimized = df.queryExecution.optimizedPlan
+ assert(!optimized.exists(_.isInstanceOf[Join]))
Review Comment:
done
--
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-53618][SQL] Turn a FULL JOIN with a false condition into a UNION ALL [spark]
Posted by "tokoko (via GitHub)" <gi...@apache.org>.
tokoko commented on code in PR #58506:
URL: https://github.com/apache/spark/pull/58506#discussion_r3932777556
##########
sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala:
##########
@@ -1838,6 +1838,20 @@ class JoinSuite extends SharedSparkSession with AdaptiveSparkPlanHelper
cached.unpersist()
}
}
+
+ test("SPARK-53618: full outer join with a false condition is rewritten to a union") {
+ val df = sql(
+ """
+ |SELECT t1.id AS a, t2.id AS b
+ |FROM range(0, 2) t1 FULL OUTER JOIN range(10, 12) t2 ON 1 = 0
Review Comment:
done
--
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-53618][SQL] Turn a FULL JOIN with a false condition into a UNI… [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #58506:
URL: https://github.com/apache/spark/pull/58506#issuecomment-5533531031
seems like the PR title was truncated. Can you make it complete please?
--
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-53618][SQL] Turn a FULL JOIN with a false condition into a UNION ALL [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #58506:
URL: https://github.com/apache/spark/pull/58506#discussion_r3932246004
##########
sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala:
##########
@@ -1838,6 +1838,20 @@ class JoinSuite extends SharedSparkSession with AdaptiveSparkPlanHelper
cached.unpersist()
}
}
+
+ test("SPARK-53618: full outer join with a false condition is rewritten to a union") {
Review Comment:
The PR description is about MERGE ... ON 1 = 0 with WHEN NOT MATCHED / WHEN NOT MATCHED BY SOURCE. However, tests only cover a standalone FULL OUTER JOIN. A DSv2 MergeIntoTable test (or even a logical-plan assertion that MergeRows.child is a Union) would lock in the case users cannot rewrite by hand.
##########
sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala:
##########
@@ -1838,6 +1838,20 @@ class JoinSuite extends SharedSparkSession with AdaptiveSparkPlanHelper
cached.unpersist()
}
}
+
+ test("SPARK-53618: full outer join with a false condition is rewritten to a union") {
+ val df = sql(
+ """
+ |SELECT t1.id AS a, t2.id AS b
+ |FROM range(0, 2) t1 FULL OUTER JOIN range(10, 12) t2 ON 1 = 0
+ |""".stripMargin)
+
+ val optimized = df.queryExecution.optimizedPlan
+ assert(!optimized.exists(_.isInstanceOf[Join]))
Review Comment:
The test asserts the logical shape, not the physical win. A cheap extra check on executedPlan (UnionExec present, BroadcastNestedLoopJoinExec absent) would match that claim. Catalyst already covers the Union shape.
##########
sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala:
##########
@@ -1838,6 +1838,20 @@ class JoinSuite extends SharedSparkSession with AdaptiveSparkPlanHelper
cached.unpersist()
}
}
+
+ test("SPARK-53618: full outer join with a false condition is rewritten to a union") {
+ val df = sql(
+ """
+ |SELECT t1.id AS a, t2.id AS b
+ |FROM range(0, 2) t1 FULL OUTER JOIN range(10, 12) t2 ON 1 = 0
+ |""".stripMargin)
Review Comment:
Also, ON 1 = 0 is rewritten late. The early PropagateEmptyRelation batch runs before ConstantFolding, so ON false rewrites early and ON 1 = 0 waits until the later LocalRelation batch (after join reorder). Fine for a single MERGE join; only relevant if this full outer sits in a larger CBO join graph.
##########
sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala:
##########
@@ -1838,6 +1838,20 @@ class JoinSuite extends SharedSparkSession with AdaptiveSparkPlanHelper
cached.unpersist()
}
}
+
+ test("SPARK-53618: full outer join with a false condition is rewritten to a union") {
+ val df = sql(
+ """
+ |SELECT t1.id AS a, t2.id AS b
+ |FROM range(0, 2) t1 FULL OUTER JOIN range(10, 12) t2 ON 1 = 0
Review Comment:
range().id is non-nullable; a full outer join must produce nullable columns. checkAnswer does not verify schema. Something like assert(df.schema.fields.forall(_.nullable)) would catch a Union.mergeChildOutputs / padding regression.
--
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-53618][SQL] Turn a FULL JOIN with a false condition into a UNION ALL [spark]
Posted by "tokoko (via GitHub)" <gi...@apache.org>.
tokoko commented on code in PR #58506:
URL: https://github.com/apache/spark/pull/58506#discussion_r3932794911
##########
sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala:
##########
@@ -1838,6 +1838,20 @@ class JoinSuite extends SharedSparkSession with AdaptiveSparkPlanHelper
cached.unpersist()
}
}
+
+ test("SPARK-53618: full outer join with a false condition is rewritten to a union") {
+ val df = sql(
+ """
+ |SELECT t1.id AS a, t2.id AS b
+ |FROM range(0, 2) t1 FULL OUTER JOIN range(10, 12) t2 ON 1 = 0
+ |""".stripMargin)
Review Comment:
true, but I don't think we can change anything about that, right? Seems like it doesn't have a practical impact at the moment.
--
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-53618][SQL] Turn a FULL JOIN with a false condition into a UNION ALL [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #58506:
URL: https://github.com/apache/spark/pull/58506#discussion_r4255572292
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/PropagateEmptyRelation.scala:
##########
@@ -123,6 +130,13 @@ abstract class PropagateEmptyRelationBase extends Rule[LogicalPlan] with CastSup
Project(p.left.output ++ nullValueProjectList(p.right), p.left)
case RightOuter if isFalseCondition && canExecuteWithoutJoin(p.right) =>
Project(nullValueProjectList(p.left) ++ p.right.output, p.right)
+ // No row of either side can find a match, so the join degenerates into both sides
+ // padded with nulls and concatenated.
+ case FullOuter if isFalseCondition && canExecuteWithoutJoin(p.left) &&
Review Comment:
**Nit (P3):** No test exercises this new branch in AQE with a broadcast query stage on each possible build side. This matters when the normal `PropagateEmptyRelation` rule is excluded but the AQE rule remains enabled: dropping either corresponding guard would rewrite the join into a union that strands a `BroadcastQueryStageExec`, producing an invalid adaptive plan while the current tests stay green.
**Verification:**
- **Regression:** Exercise both left- and right-broadcast adaptive child arrangements and fail if either one is detached from the full outer join by the empty-relation propagation rule.
- **Behavior:** Exercise an otherwise equivalent non-broadcast adaptive arrangement and verify the eligible false-condition full outer join still rewrites to the null-padded union with unchanged rows.
--
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-53618][SQL] Turn a FULL JOIN with a false condition into a UNION ALL [spark]
Posted by "Visorgood (via GitHub)" <gi...@apache.org>.
Visorgood commented on PR #58506:
URL: https://github.com/apache/spark/pull/58506#issuecomment-5748370989
Hi @uros-b! Would you have some time to review this one more time?
--
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-53618][SQL] Turn a FULL JOIN with a false condition into a UNION ALL [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #58506:
URL: https://github.com/apache/spark/pull/58506#issuecomment-6144093767
> Confirm the exchange-reuse tradeoff for duplicate false joins
@tokoko can you take a look at this point in the previous review?
--
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-53618][SQL] Turn a FULL JOIN with a false condition into a UNION ALL [spark]
Posted by "tokoko (via GitHub)" <gi...@apache.org>.
tokoko commented on code in PR #58506:
URL: https://github.com/apache/spark/pull/58506#discussion_r4251874834
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/PropagateEmptyRelation.scala:
##########
@@ -123,6 +125,13 @@ abstract class PropagateEmptyRelationBase extends Rule[LogicalPlan] with CastSup
Project(p.left.output ++ nullValueProjectList(p.right), p.left)
case RightOuter if isFalseCondition && canExecuteWithoutJoin(p.right) =>
Project(nullValueProjectList(p.left) ++ p.right.output, p.right)
+ // No row of either side can find a match, so the join degenerates into both sides
+ // padded with nulls and concatenated.
+ case FullOuter if isFalseCondition && canExecuteWithoutJoin(p.left) &&
+ canExecuteWithoutJoin(p.right) =>
+ Union(
+ Project(p.left.output ++ nullValueProjectList(p.right), p.left),
Review Comment:
Fixed in `nullValueProjectList`, thanks. Just note this was a preexisting behavior for left/right rewrites as well. The change now fixes it for all join types.
--
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-53618][SQL] Turn a FULL JOIN with a false condition into a UNION ALL [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #58506:
URL: https://github.com/apache/spark/pull/58506#discussion_r4249117583
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/PropagateEmptyRelation.scala:
##########
@@ -123,6 +125,13 @@ abstract class PropagateEmptyRelationBase extends Rule[LogicalPlan] with CastSup
Project(p.left.output ++ nullValueProjectList(p.right), p.left)
case RightOuter if isFalseCondition && canExecuteWithoutJoin(p.right) =>
Project(nullValueProjectList(p.left) ++ p.right.output, p.right)
+ // No row of either side can find a match, so the join degenerates into both sides
+ // padded with nulls and concatenated.
+ case FullOuter if isFalseCondition && canExecuteWithoutJoin(p.left) &&
+ canExecuteWithoutJoin(p.right) =>
+ Union(
+ Project(p.left.output ++ nullValueProjectList(p.right), p.left),
Review Comment:
**Non-blocking (P2):** The first Union branch synthesizes the right-side columns with `nullValueProjectList`, whose aliases retain only the name and ExprId. Because `Union.mergeChildOutputs` takes metadata and qualifiers from the first branch, this rewrite drops those properties from the right-side optimized output even though the original `FullOuter` output preserves them. The new tests use metadata-free inputs or inspect the analyzed schema, so they do not catch this regression.
**Verification:**
- **Regression:** Optimize a literal-false full outer join whose left and right attributes carry distinct metadata and qualifiers, then assert that optimized output matches the analyzed Join output for names, data types, ExprIds, qualifiers, metadata, and widened nullability.
- **Behavior:** Execute representative standalone and generated MERGE false-condition full outer plans and verify unchanged padded rows together with Union execution and absence of the nested-loop join.
- **Compatibility:** Exercise the other outer-join simplifications that share nullValueProjectList and verify preserving attribute properties does not change their row results, output positions, or ExprIds.
--
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-53618][SQL] Turn a FULL JOIN with a false condition into a UNION ALL [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on PR #58506:
URL: https://github.com/apache/spark/pull/58506#issuecomment-5774780296
Thank you for addressing the comments! Let's also ping @cloud-fan here too
--
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