[SPARK-33272][SQL] prune the attributes mapping in QueryPlan.transformUpWithNewOutput #30173
Conversation
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
| * @param canGetOutput a boolean condition to indicate if we can get the output of a plan node | ||
| * to prune the attributes mapping to be propagated. The default value is true | ||
| * as only unresolved logical plan can't get output. |
There was a problem hiding this comment.
nit: Is this needed to be parameterized? Seems we only need check unresolved logical plan.
There was a problem hiding this comment.
Do you mean something like
val canGetOutput = plan match {
case l: LogicalPlan => l.resolved
case _ => true
}
There was a problem hiding this comment.
One advantage of the current way is we can skip the check for optimizer rules (use the default value true).
|
Test build #130375 has finished for PR 30173 at commit
|
|
Thanks! Merged to master. |
|
hi @cloud-fan, can you help to open backport PRs for 3.0.2? thanks! I found that prunes the attributes mapping is not only performance optimized, it also circumvents the bug of rewriting attributes for some complex queries, e.g. https://issues.apache.org/jira/browse/SPARK-36815 |
|
@gaoyajun02 can you cherry-pick this commit and open the backport PR? I'm quite busy this week and I can do the backport next week if you are not able to do it. |
…mUpWithNewOutput ### What changes were proposed in this pull request? For complex query plans, `QueryPlan.transformUpWithNewOutput` will keep accumulating the attributes mapping to be propagated, which may hurt performance. This PR prunes the attributes mapping before propagating. ### Why are the changes needed? A simple perf improvement. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? existing tests Closes apache#30173 from cloud-fan/bug. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org> (cherry picked from commit 2639ad4)
…QueryPlan.transformUpWithNewOutput ### What changes were proposed in this pull request? This is a backport PR of #30173. For complex query plans, `QueryPlan.transformUpWithNewOutput` will keep accumulating the attributes mapping to be propagated, which may hurt performance. This PR prunes the attributes mapping before propagating. ### Why are the changes needed? A simple perf improvement. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? existing tests Closes #34068 from gaoyajun02/SPARK-33272. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: Liang-Chi Hsieh <viirya@gmail.com>
What changes were proposed in this pull request?
For complex query plans,
QueryPlan.transformUpWithNewOutputwill keep accumulating the attributes mapping to be propagated, which may hurt performance. This PR prunes the attributes mapping before propagating.Why are the changes needed?
A simple perf improvement.
Does this PR introduce any user-facing change?
No
How was this patch tested?
existing tests