-
Notifications
You must be signed in to change notification settings - Fork 28.6k
[SPARK-32801][SQL] Make InferFiltersFromConstraints take into account EqualNullSafe #29650
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
# Conflicts: # sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala
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.
Are you sure we can infer from EqualNullSafe
? Some EqualNullSafe
from Alias
:
spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/LogicalPlan.scala
Lines 175 to 183 in 0a6043f
case a @ Alias(l: Literal, _) => | |
allConstraints += EqualNullSafe(a.toAttribute, l) | |
case a @ Alias(e, _) => | |
// For every alias in `projectList`, replace the reference in constraints by its attribute. | |
allConstraints ++= allConstraints.map(_ transform { | |
case expr: Expression if expr.semanticEquals(e) => | |
a.toAttribute | |
}) | |
allConstraints += EqualNullSafe(e, a.toAttribute) |
I do not follow, why should this make |
Kubernetes integration test starting |
Kubernetes integration test status failure |
@cloud-fan, it seems you worked on this |
Test build #133059 has finished for PR 29650 at commit
|
Test build #134102 has finished for PR 29650 at commit
|
cc @maryannxue too |
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
What changes were proposed in this pull request?
InferFiltersFromConstraints
only infers new filters usingEqualTo
, generalized it to also includeEqualNullSafe
.This introduced an infinite loop in
InferFiltersFromConstraintsSuite
. It has been fixed in theOptimizer
by restructuring the batches (#19149) - I did the same in the test suite.Also it revealed, that the
InferFiltersFromConstraints
is not idempotent, for two cases:"x.a".attr === "x.b".attr && "x.a".attr === "y.a".attr && "x.b".attr === "y.b".attr
, that should infer"y.a".attr === "y.b".attr
These were also fixed
Why are the changes needed?
Possible performance improvement from new inferred constraints
Does this PR introduce any user-facing change?
No
How was this patch tested?
UT