Fix: limit is missing after removing SPM #14569
Conversation
| ]; | ||
| let expected_optimized = [ | ||
| "SortPreservingMergeExec: [non_nullable_col@1 ASC], fetch=100", | ||
| " SortExec: expr=[non_nullable_col@1 ASC], preserve_partitioning=[false]", |
There was a problem hiding this comment.
I am wandering do we expect to optimize only sortExec with Topk(fetch) for the only one partition case?
xudong963
force-pushed
the
fix_limit_smp
branch
from
dc76129 to
12cd613
Compare
| "LocalLimitExec: fetch=100", | ||
| " SortExec: expr=[non_nullable_col@1 ASC], preserve_partitioning=[false]", |
There was a problem hiding this comment.
we should optimize such pattern to topk later
There was a problem hiding this comment.
This makes sense to me -- thank you @xudong963
In general it seems like we have a class of bugs related to removing fetch -- maybe we should re-evaluate enforce_sorting for a more holistic fix if we find any more issues
yes, I also have the thought, |
|
Thanks all, let's go! |
* Fix: limit is missing after removing SPM * fix test * use limitexec
Rationale for this change
It's clear that fetch will miss after removing SPM
What changes are included in this PR?
If SPM is with fetch, we won't remove it.
Are these changes tested?
Yes, ut
Are there any user-facing changes?
No