Skip to content

Commit

Permalink
Merge #40583 #40669
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
3 people committed Sep 11, 2019
3 parents 3593a11 + 71f14f3 + 63b31df commit bce673e
Show file tree
Hide file tree
Showing 4 changed files with 648 additions and 199 deletions.
1 change: 1 addition & 0 deletions pkg/sql/distsql_physical_planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -1989,6 +1989,7 @@ func (dsp *DistSQLPlanner) createPlanForLookupJoin(
// For anti/semi join, we only produce the input columns.
planToStreamColMap = planToStreamColMap[:numInputNodeCols]
post.OutputColumns = post.OutputColumns[:numInputNodeCols]
types = types[:numInputNodeCols]
}

// Instantiate one join reader for every stream.
Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/opt/exec/execbuilder/relational.go
Original file line number Diff line number Diff line change
Expand Up @@ -1238,6 +1238,10 @@ func (b *Builder) buildLookupJoin(join *memo.LookupJoinExpr) (execPlan, error) {
allCols := joinOutputMap(input.outputCols, lookupColMap)

res := execPlan{outputCols: allCols}
if join.JoinType == opt.SemiJoinOp || join.JoinType == opt.AntiJoinOp {
// For semi and anti join, only the left columns are output.
res.outputCols = input.outputCols
}

ctx := buildScalarCtx{
ivh: tree.MakeIndexedVarHelper(nil /* container */, allCols.Len()),
Expand Down
Loading

0 comments on commit bce673e

Please sign in to comment.