opt: fix execbuilder output column count for lookup joins #40669
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
justinj
reviewed
Sep 11, 2019
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @justinj and @RaduBerinde)
|
Thanks for the quick fix! I had added this to the release blockers list, so please check the box marking it as done when this merges. |
|
Regarding merging justification during release stability period, I believe this falls under "Low risk, high benefit changes to existing functionality". |
This commit fixes a bug in the execbuilder, which was determining the number of output columns incorrectly for lookup joins. In particular, if the join was a semi- or anti-join, the execbuilder was incorrectly including the right side columns in the column count. This commit fixes the code so it only includes left side columns for semi- and anti-joins. This commit also fixes a bug in the DistSQL planner that was masked by the bug in the execbuilder. In particular, the output types slice for lookup semi- and anti-joins incorrectly included the types from the right side columns. This commit fixes it to only include types for the left side columns. Fixes cockroachdb#40562 Release note (bug fix): Fixed an error that could occur when the optimizer created a plan with a lookup semi- or anti-join nested inside of another join.
rytaft
force-pushed
the
lookup-join-err
branch
from
0ccf0ea
to
63b31df
Compare
|
TFTR! Turns out there was another bug lurking in the DistSQL planning side that was masked by this bug. @jordanlewis can you please take a look? |
RaduBerinde
reviewed
Sep 11, 2019
There was a problem hiding this comment.
DistSQL fix LGTM.
Reviewable status:
rytaft
commented
Sep 11, 2019
There was a problem hiding this comment.
Thanks!
bors r+
Reviewable status:
craig bot
pushed a commit
that referenced
this pull request
Sep 11, 2019
40583: sqlmigrations: remove ensureMaxPrivileges migration r=mberhault a=ajwerner This migration has baked. Furthermore it uses table descriptors in ways I'm working to change and I want to see it gone. Release note: None 40669: opt: fix execbuilder output column count for lookup joins r=rytaft a=rytaft This commit fixes a bug in the `execbuilder`, which was determining the number of output columns incorrectly for lookup joins. In particular, if the join was a semi- or anti-join, the `execbuilder` was incorrectly including the right side columns in the column count. This commit fixes the code so it only includes left side columns for semi- and anti-joins. This commit also fixes a bug in the DistSQL planner that was masked by the bug in the `execbuilder`. In particular, the output types slice for lookup semi- and anti-joins incorrectly included the types from the right side columns. This commit fixes it to only include types for the left side columns. Fixes #40562 Release note (bug fix): Fixed an error that could occur when the optimizer created a plan with a lookup semi- or anti-join nested inside of another join. Co-authored-by: Andrew Werner <ajwerner@cockroachlabs.com> Co-authored-by: Rebecca Taft <becca@cockroachlabs.com>
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit fixes a bug in the
execbuilder, which was determiningthe number of output columns incorrectly for lookup joins. In particular,
if the join was a semi- or anti-join, the
execbuilderwas incorrectlyincluding the right side columns in the column count. This commit
fixes the code so it only includes left side columns for semi- and
anti-joins.
This commit also fixes a bug in the DistSQL planner that was masked
by the bug in the
execbuilder. In particular, the output types slicefor lookup semi- and anti-joins incorrectly included the types from the
right side columns. This commit fixes it to only include types for the
left side columns.
Fixes #40562
Release note (bug fix): Fixed an error that could occur when the
optimizer created a plan with a lookup semi- or anti-join nested
inside of another join.