-
Notifications
You must be signed in to change notification settings - Fork 29.1k
[SPARK-34775][SQL] Push down limit through window when partitionSpec is not empty #32475
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
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #138283 has finished for PR 32475 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #138315 has finished for PR 32475 at commit
|
|
ping @wangyum |
...lyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/LimitPushDownThroughWindow.scala
Outdated
Show resolved
Hide resolved
...src/test/scala/org/apache/spark/sql/catalyst/optimizer/LimitPushdownThroughWindowSuite.scala
Outdated
Show resolved
Hide resolved
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #138616 has finished for PR 32475 at commit
|
| // Sort is needed here because we need global sort. | ||
| window.copy(child = Limit(limitExpr, Sort(orderSpec, true, child))) | ||
| window.copy(child = Limit(limitExpr, | ||
| Sort(partitionSpec.map(SortOrder(_, Ascending)) ++ orderSpec, true, child))) |
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.
@cloud-fan Do we need to changed the sort order from NULLS FIRST to NULLS LAST? Impala have changed it.
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.
is NULLS LAST faster?
| // Adding an extra Limit below WINDOW when the partitionSpec of all window functions is empty. | ||
| case LocalLimit(limitExpr @ IntegerLiteral(limit), | ||
| window @ Window(windowExpressions, Nil, orderSpec, child)) | ||
| window @ Window(windowExpressions, partitionSpec, orderSpec, child)) |
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.
can you provide a bit more context? why can we pushdown limit through window when there are partition specs?
|
Test build #142483 has finished for PR 32475 at commit
|
|
Test build #142489 has finished for PR 32475 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
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?
This is a followup from #31691. Push down limit through Window when the partitionSpec of all window functions is not empty and the same order is used.
Push down limit through Window when the partitionSpec of all window functions is not empty
And the origin author is @leoluan2009, since he didn't reply for long and did this follow up after invitation
Why are the changes needed?
Improve query performance.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Added UT