-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
planner, executor: inject proj below TopN and Sort if byItem contains scalarFunc #9197
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9197 +/- ##
==========================================
- Coverage 67.19% 67.18% -0.01%
==========================================
Files 371 371
Lines 77531 77511 -20
==========================================
- Hits 52096 52077 -19
+ Misses 20784 20780 -4
- Partials 4651 4654 +3
Continue to review full report at Codecov.
|
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.
LGTM
…proj_below_topn
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.
LGTM. Defer the approve to @zz-jason since he has open comments left.
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.
LGTM
What problem does this PR solve?
Fix #9047
What is changed and how it works?
The issue is caused by the re-usage of chunk between SortExec/TopNExec and HashAggExec. Different chunks have different capacity thus the growth of SortExec.rowChunks and SortExec.keyChunks is not exactly the same( SortExec.rowChunks.NumChunks() != SortExec.keyChunks.NumChunks()), so
index outof range
error may be raised when we use a singe rowPtr to record the position of every rows in SortExec.rowChunks and SortExec.keyChunks.Inject a Projection below TopN/ Sort when Orderby-item contains scalar function(s). Thus we can remove the keyChunks from SortExec and fix the issue.
Check List
Tests
Code changes
Side effects
N/A
Related changes