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: [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]
