-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
distsqlrun: fix lookup join with limit #30819
Conversation
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.
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 1 stale)
pkg/sql/distsqlrun/joinreader.go, line 536 at r1 (raw file):
// emitRow returns the next row from jr.toEmit, if present. Otherwise it // prepares for another input batch. func (jr *joinReader) emitRow() (joinReaderState, sqlbase.EncDatumRow, *ProducerMetadata) {
[nit] remove the last return which is always nil
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.
You could also consider applying ProcessRowHelper at the end of
Next()
just to be more consistent with other processors like hashjoiner.
Reviewable status:
complete! 1 of 0 LGTMs obtained (and 1 stale)
Previously, lookup join used its processRowHelper in an inappropriate place (not directly before returning a row) which caused the final rows in a limit query to get omitted in certain cases. Correct this problem. Release note (bug fix): lookup joins no longer omit rows in certain circumstances during limit queries.
54cd53c
to
7ba018c
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.
Good idea, done.
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 2 stale)
bors r+ |
30819: distsqlrun: fix lookup join with limit r=jordanlewis a=jordanlewis Previously, lookup join used its processRowHelper in an inappropriate place (not directly before returning a row) which caused the final rows in a limit query to get omitted in certain cases. Correct this problem. Fixes #30812. Release note (bug fix): lookup joins no longer omit rows in certain circumstances during limit queries. Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>
Build succeeded |
Previously, lookup join used its processRowHelper in an inappropriate
place (not directly before returning a row) which caused the final rows
in a limit query to get omitted in certain cases. Correct this problem.
Fixes #30812.
Release note (bug fix): lookup joins no longer omit rows in certain
circumstances during limit queries.