-
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
Add OderBy output stage spill #7759
Conversation
✅ Deploy Preview for meta-velox canceled.
|
4a6013c
to
b96d687
Compare
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.
@duanmeng thanks for the change % minors on code structure.
velox/exec/Spiller.h
Outdated
/// Invoked to spill.Spill all rows pointed by the pointers in sortedRows.This | ||
/// is only used by 'kOrderByOutput' spiller type to spill during the order by | ||
/// output processing. Similarly, the spilled rows still stays in the row | ||
/// container.The caller needs to erase them from the row container. |
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.
s/container.The/container. The/
01583a8
to
a981427
Compare
@xiaoxmeng Hi Xuan, I've made a small refactor and resolved all your comments. Could you please help take another look? Thanks. |
6b0400a
to
551b3b5
Compare
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.
@duanmeng LGTM. Could you help to add to support hive sort writer in followup? Thanks!
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.
@duanmeng LGTM. Thanks!
@xiaoxmeng Hi Xuan, thanks for your review and guidance, I've resolved all the comments, could you please help to take a look? |
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.
@duanmeng thanks!
@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@xiaoxmeng merged this pull request in ddc3471. |
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
This reverts commit ddc3471.
This reverts commit ddc3471.
Add a new type of spiller named
kOrderByOutput
.It is used to spill data during the output processing
stage of
OrderBy
operator. The spiller uses a newlyadded API that supports spill data by row pointers.