Skip to content

Commit

Permalink
opt: compute join output col map properly
Browse files Browse the repository at this point in the history
Fixes cockroachdb#39010.

This manifested with the advent of WITH, but the problem was actually
that we were computing a join output map somewhat incorrectly. This
never actually caused problems before because we would prune any columns
that could cause there to be a disparity between the two ways of
computing the size of the ColMap, but since WITH doesn't support
pruning, it caused issues.

I also replaced some other uses of Len that were potentially sketchy.

Release note: None
  • Loading branch information
Justin Jaffray committed Jul 24, 2019
1 parent bb7c804 commit 1feb3b0
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 4 deletions.
16 changes: 12 additions & 4 deletions pkg/sql/opt/exec/execbuilder/relational.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,13 @@ type execPlan struct {
// if the node outputs the same optimizer ColumnID multiple times.
// TODO(justin): we should keep track of this instead of computing it each time.
func (ep *execPlan) numOutputCols() int {
max, ok := ep.outputCols.MaxValue()
return numOutputColsInMap(ep.outputCols)
}

// numOutputColsInMap returns the number of slots required to fill in all of
// the columns referred to by this ColMap.
func numOutputColsInMap(m opt.ColMap) int {
max, ok := m.MaxValue()
if !ok {
return 0
}
Expand Down Expand Up @@ -512,6 +518,7 @@ func (b *Builder) buildProject(prj *memo.ProjectExpr) (execPlan, error) {
if err != nil {
return execPlan{}, err
}

projections := prj.Projections
if len(projections) == 0 {
// We have only pass-through columns.
Expand Down Expand Up @@ -633,7 +640,7 @@ func (b *Builder) buildApplyJoin(join memo.RelExpr) (execPlan, error) {
allCols := joinOutputMap(left.outputCols, fakeRight.outputCols)

ctx := buildScalarCtx{
ivh: tree.MakeIndexedVarHelper(nil /* container */, allCols.Len()),
ivh: tree.MakeIndexedVarHelper(nil /* container */, numOutputColsInMap(allCols)),
ivarMap: allCols,
}

Expand Down Expand Up @@ -771,7 +778,7 @@ func (b *Builder) initJoinBuild(
allCols := joinOutputMap(leftPlan.outputCols, rightPlan.outputCols)

ctx := buildScalarCtx{
ivh: tree.MakeIndexedVarHelper(nil /* container */, allCols.Len()),
ivh: tree.MakeIndexedVarHelper(nil /* container */, numOutputColsInMap(allCols)),
ivarMap: allCols,
}

Expand All @@ -792,7 +799,8 @@ func (b *Builder) initJoinBuild(
// joinOutputMap determines the outputCols map for a (non-semi/anti) join, given
// the outputCols maps for its inputs.
func joinOutputMap(left, right opt.ColMap) opt.ColMap {
numLeftCols := left.Len()
numLeftCols := numOutputColsInMap(left)

res := left.Copy()
right.ForEach(func(colIdx, rightIdx int) {
res.Set(colIdx, rightIdx+numLeftCols)
Expand Down
39 changes: 39 additions & 0 deletions pkg/sql/opt/exec/execbuilder/testdata/with
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,42 @@ root · · (a
· size 2 columns, 1 row · ·
· row 0, expr 0 1 · ·
· row 0, expr 1 unique_rowid() · ·

# Regression test for #39010.

statement ok
CREATE TABLE table39010 (col NAME)

query TTTTT
EXPLAIN (VERBOSE)
WITH
w AS (SELECT NULL, NULL FROM table39010)
SELECT
col
FROM
w, table39010
----
root · · (col) ·
├── render · · (col) ·
│ │ render 0 col · ·
│ └── hash-join · · ("?column?", "?column?", col) ·
│ │ type cross · ·
│ ├── render · · ("?column?", "?column?") ·
│ │ │ render 0 "?column?" · ·
│ │ │ render 1 "?column?" · ·
│ │ └── scan buffer node · · ("?column?") ·
│ │ label buffer 1 (w) · ·
│ └── scan · · (col) ·
│ table table39010@primary · ·
│ spans ALL · ·
└── subquery · · (col) ·
│ id @S1 · ·
│ original sql SELECT NULL, NULL FROM table39010 · ·
│ exec mode all rows · ·
└── buffer node · · ("?column?") ·
│ label buffer 1 (w) · ·
└── render · · ("?column?") ·
│ render 0 NULL · ·
└── scan · · () ·
· table table39010@primary · ·
· spans ALL · ·

0 comments on commit 1feb3b0

Please sign in to comment.