[SPARK-33036][SQL] Refactor RewriteCorrelatedScalarSubquery code to replace exprIds in a bottom-up manner #29913
Conversation
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
Test build #129274 has finished for PR 29913 at commit
|
|
cc: @cloud-fan |
|
Hi, @maropu . |
|
retest this please |
| val newExprs = exprs.map { _.transform { | ||
| case a: AttributeReference if attrMap.contains(a) => | ||
| val exprId = attrMap.getOrElse(a, a).exprId | ||
| a.withExprId(exprId) |
There was a problem hiding this comment.
shall we simply use attrMap(a) or only use the exprId?
|
Test build #129445 has finished for PR 29913 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
| @@ -587,6 +589,20 @@ object RewriteCorrelatedScalarSubquery extends Rule[LogicalPlan] { | |||
| } | |||
| } | |||
| } | |||
| (newChild, AttributeMap(subqueryAttrMapping)) | |||
There was a problem hiding this comment.
AttributeMap(subqueryAttrMapping.toSeq) to pass scala 2.13 compilation
There was a problem hiding this comment.
Ah, I missed that. Updated.
|
Test build #129480 has finished for PR 29913 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
Test build #129490 has finished for PR 29913 at commit
|
| attrMap: AttributeMap[Attribute]): Seq[E] = { | ||
| if (attrMap.nonEmpty) { | ||
| val newExprs = exprs.map { _.transform { | ||
| case a: AttributeReference if attrMap.contains(a) => attrMap(a) |
There was a problem hiding this comment.
not a big deal: it's more efficient to write case a: AttributeReference => attrMap.getOrElse(a, a)
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
GA passed, so merged to master. Thanks, @cloud-fan |
|
Test build #129496 has finished for PR 29913 at commit
|
What changes were proposed in this pull request?
This PR intends to refactor code in
RewriteCorrelatedScalarSubqueryfor replacingExprIds in a bottom-up manner instead of doing in a top-down one.This PR comes from the talk with @cloud-fan in #29585 (comment).
Why are the changes needed?
To improve code.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Existing tests.