-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-28220][SQL] Improve PropagateEmptyRelation to support join with false condition #31857
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/FilterPushdownSuite.scala
Outdated
Show resolved
Hide resolved
|
cc @cloud-fan |
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
Outdated
Show resolved
Hide resolved
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/FilterPushdownSuite.scala
Outdated
Show resolved
Hide resolved
...catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/PropagateEmptyRelation.scala
Show resolved
Hide resolved
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #136250 has finished for PR 31857 at commit
|
|
|
||
| private def pushDownJoinConditions(conditions: Seq[Expression], plan: LogicalPlan) = { | ||
| conditions | ||
| .filterNot(_.semanticEquals(TrueLiteral)) // Push down true condition is useless. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be optimized by BooleanSimplification already? true And cond -> cond
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is another issue. Will be fixed by another PR.
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
# Conflicts: # sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PropagateEmptyRelationSuite.scala
|
Test build #136259 has finished for PR 31857 at commit
|
|
Test build #136263 has finished for PR 31857 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
retest this please. |
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #136273 has finished for PR 31857 at commit
|
|
Merged to master. |
dongjoon-hyun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, late LGTM.
Thank you, @wangyum and @cloud-fan .
What changes were proposed in this pull request?
Improve
PropagateEmptyRelationto support join with false condition. For example:Before this pr:
After this pr:
Why are the changes needed?
Avoid
BroadcastNestedLoopJointo improve query performance.Does this PR introduce any user-facing change?
No.
How was this patch tested?
Unit test.