-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Update explain plan to show when topk operator is used #7750
Comments
I think this is an excellent task for new users as the code is quite straightforward and most of this work will be to learn how to run the various tests and update them for the new explain plans. |
solves: apache#7750 Replaced `SortExec: fetch={fetch}, expr=[{}]` with 'SortExec: TopK(fetch={fetch}), expr=[{}]' in [sort.rs](https://github.com/apache/arrow-datafusion/blob/main/datafusion/physical-plan/src/sorts/sort.rs) file
Let me try it. 😄 |
Note there is already a partial version in #7751 With a good comment here: #7751 (comment) Maybe you can base your efforts on that PR |
hello !! can i try working on this issue? |
hello !! can i try working on this issue? Absolutely -- I think we are just waiting on someone to make a PR that passes CI :) |
* Updated sort.rs solves: #7750 Replaced `SortExec: fetch={fetch}, expr=[{}]` with 'SortExec: TopK(fetch={fetch}), expr=[{}]' in [sort.rs](https://github.com/apache/arrow-datafusion/blob/main/datafusion/physical-plan/src/sorts/sort.rs) file * fix: ci --------- Co-authored-by: Pratibhanu Jarngal <[email protected]>
Is your feature request related to a problem or challenge?
After #7721 a
SortExec
with a limit will use a specialTopK
operatorHowever, I don't think it is clear from the EXPLAIN plan that this will be used (you have to know that sort with a limit is specially optimized to not actually sort).
Using DataFusion CLI and the dataset described here: #7721 (review), the explain plan looks like
Describe the solution you'd like
I would like to make it clearer in the explain plan that this new operator is used. Perhaps something like
See comment in https://github.com/apache/arrow-datafusion/pull/7721/files#r1343969678 for exactly where this code that controls the output is
Describe alternatives you've considered
We can leave the explain plans alone, but I think that is confusing
Additional context
No response
The text was updated successfully, but these errors were encountered: