[SPARK-39537][SQL] Ensure that the expressions order returned by InferFiltersFromConstraints
#36934
Conversation
…rFiltersFromConstraints
| private def inferFilters(plan: LogicalPlan): LogicalPlan = plan.transformWithPruning( | ||
| _.containsAnyPattern(FILTER, JOIN)) { | ||
| case filter @ Filter(condition, child) => | ||
| val newFilters = filter.constraints -- |
There was a problem hiding this comment.
is it possible to override -- in ExpressionSet?
There was a problem hiding this comment.
Yes, after SPARK-39520, this issue should no longer exists， and @HyukjinKwon already mark SPARK-39523 as RESOLVED
There was a problem hiding this comment.
It seems #36925 fixed this issue ? @LuciferYang
There was a problem hiding this comment.
SPARK-39520 was not merged to 3.3, do we have this issue in 3.3?
There was a problem hiding this comment.
It seems #36925 fixed this issue ? @LuciferYang
Yes
There was a problem hiding this comment.
SPARK-39520 was not merged to 3.3, do we have this issue in 3.3?
Let me double check this
There was a problem hiding this comment.
@cloud-fan @LuciferYang I have tested and the issue no longer exists. I will close this PR.
There was a problem hiding this comment.
SPARK-39520 was not merged to 3.3, do we have this issue in 3.3?
@cloud-fan 3.3 no this issue due to SPARK-38836 not in branch-3.3, the -- method hasn't been deleted, and I manually check the relevant suites(inlcude TPCDSV1_4_PlanStabilitySuite, AdaptiveQueryExecSuite, ExpressionSetSuite, JDBCV2Suite, GeneratorFunctionSuite), all passed.
|
ping @HyukjinKwon @huaxingao cc @cloud-fan |
3 participants
What changes were proposed in this pull request?
Currently,
InferFiltersFromConstraintsconstruct a newFilterwhich condition without the same order in scala 2.12 and 2.13It seems
--ofExpressionSetcannot guarantee the order of elements.The behavior lead to #35947 (comment)
Why are the changes needed?
Fix bug
Does this PR introduce any user-facing change?
'Yes'.
The behavior will be consistent between scala 2.12 and 2.13
How was this patch tested?
Exists tests.