[SPARK-21091][SQL] Move constraint code into QueryPlanConstraints #18298
Conversation
| private lazy val aliasMap: AttributeMap[Expression] = AttributeMap( | ||
| expressions.collect { | ||
| case a: Alias => (a.toAttribute, a.child) | ||
| } ++ children.flatMap(_.asInstanceOf[QueryPlanConstraints[PlanType]].aliasMap)) |
There was a problem hiding this comment.
this is the only weird part; the rest are straightforward copy.
I couldn't figure out how to get the compiler to past without the explicit cast.
There was a problem hiding this comment.
what if we do trait QueryPlanConstraints[PlanType <: QueryPlan[PlanType]] extends QueryPlan[PlanType]?
There was a problem hiding this comment.
that would create a cyclic hierarchy ...
|
Test build #78034 has started for PR 18298 at commit |
|
Test build #78039 has finished for PR 18298 at commit
|
|
Test build #3797 has finished for PR 18298 at commit
|
|
retest this please |
|
LGTM |
|
Test build #78061 has finished for PR 18298 at commit
|
| lazy val constraints: ExpressionSet = | ||
| ExpressionSet( | ||
| validConstraints | ||
| .union(inferAdditionalConstraints(constraints)) |
| ExpressionSet( | ||
| validConstraints | ||
| .union(inferAdditionalConstraints(constraints)) | ||
| .union(constructIsNotNullConstraints(constraints)) |
|
Thanks for the cleanup! LGTM modulo Xiao's comment. |
This reverts commit a212b14.
|
LGTM |
|
Merging in master. |
|
Test build #78067 has finished for PR 18298 at commit
|
## What changes were proposed in this pull request? This patch moves constraint related code into a separate trait QueryPlanConstraints, so we don't litter QueryPlan with a lot of constraint private functions. ## How was this patch tested? This is a simple move refactoring and should be covered by existing tests. Author: Reynold Xin <[email protected]> Closes apache#18298 from rxin/SPARK-21091.
5 participants
What changes were proposed in this pull request?
This patch moves constraint related code into a separate trait QueryPlanConstraints, so we don't litter QueryPlan with a lot of constraint private functions.
How was this patch tested?
This is a simple move refactoring and should be covered by existing tests.