Skip to content

Commit

Permalink
colfetcher: find needed columns upfront
Browse files Browse the repository at this point in the history
Previously, the two users of the `cFetcher` (`ColBatchScan` and
`ColIndexJoin`) had different ways of figuring out which columns are
actually needed to be fetched (that information is needed to not decode
parts of the KV pairs unnecessarily). In one case we explicitly
propagated the information through the spec, and in another we did some
pre-processing to find the information.

This overall was more complicated than necessary (because we had complex
code path for pruning "unaccessible" columns from the secondary index
while keeping track of the ordinal mapping) as well as suboptimal (in
some cases `TableReaderSpec.NeededColumns` contained ordinals of the
columns that weren't actually needed by the post-processing stage), so
this commit unifies both ways into one addressing both of these
shortcomings.

Now, in order to find the set of needed columns, we examine the
post-processing spec all the time, but we do it in an efficient manner:
- we make sure to deserialize render expressions early
- we use a `tree.Visitor` rather than a heavy-weight `ExprHelper`.

As a result, we can remove `TableReaderSpec.NeededColumns` field as well
as not decode some columns in a handful of cases.

Release note: None
  • Loading branch information
yuzefovich committed Jan 7, 2022
1 parent 7c72806 commit 2c23b80
Show file tree
Hide file tree
Showing 18 changed files with 144 additions and 306 deletions.
5 changes: 2 additions & 3 deletions pkg/sql/colexec/colbuilder/execplan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,8 @@ func TestNewColOperatorExpectedTypeSchema(t *testing.T) {

desc := catalogkv.TestingGetTableDescriptor(kvDB, keys.SystemSQLCodec, "test", "t")
tr := execinfrapb.TableReaderSpec{
Table: *desc.TableDesc(),
Spans: make([]roachpb.Span, 1),
NeededColumns: []uint32{0},
Table: *desc.TableDesc(),
Spans: make([]roachpb.Span, 1),
}
var err error
tr.Spans[0].Key, err = randgen.TestingMakePrimaryIndexKey(desc, 0)
Expand Down
1 change: 0 additions & 1 deletion pkg/sql/colexec/colexecargs/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ go_library(
"//pkg/sql/colexecop",
"//pkg/sql/execinfra",
"//pkg/sql/execinfrapb",
"//pkg/sql/parser",
"//pkg/sql/sem/tree",
"//pkg/sql/types",
"//pkg/util/mon",
Expand Down
53 changes: 0 additions & 53 deletions pkg/sql/colexec/colexecargs/expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"sync"

"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
"github.com/cockroachdb/cockroach/pkg/sql/parser"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/types"
)
Expand Down Expand Up @@ -51,55 +50,3 @@ func (h *ExprHelper) ProcessExpr(
tempVars := tree.MakeIndexedVarHelper(&h.helper, len(typs))
return execinfrapb.DeserializeExpr(expr.Expr, h.SemaCtx, evalCtx, &tempVars)
}

// Remove unused warning.
var _ = findIVarsInRange

// findIVarsInRange searches Expr for presence of tree.IndexedVars with indices
// in range [start, end). It returns a slice containing all such indices.
func findIVarsInRange(expr execinfrapb.Expression, start int, end int) ([]uint32, error) {
res := make([]uint32, 0)
if start >= end {
return res, nil
}
var exprToWalk tree.Expr
if expr.LocalExpr != nil {
exprToWalk = expr.LocalExpr
} else {
e, err := parser.ParseExpr(expr.Expr)
if err != nil {
return nil, err
}
exprToWalk = e
}
visitor := ivarExpressionVisitor{ivarSeen: make([]bool, end)}
_, _ = tree.WalkExpr(visitor, exprToWalk)
for i := start; i < end; i++ {
if visitor.ivarSeen[i] {
res = append(res, uint32(i))
}
}
return res, nil
}

type ivarExpressionVisitor struct {
ivarSeen []bool
}

var _ tree.Visitor = &ivarExpressionVisitor{}

// VisitPre is a part of tree.Visitor interface.
func (i ivarExpressionVisitor) VisitPre(expr tree.Expr) (bool, tree.Expr) {
switch e := expr.(type) {
case *tree.IndexedVar:
if e.Idx < len(i.ivarSeen) {
i.ivarSeen[e.Idx] = true
}
return false, expr
default:
return true, expr
}
}

// VisitPost is a part of tree.Visitor interface.
func (i ivarExpressionVisitor) VisitPost(expr tree.Expr) tree.Expr { return expr }
1 change: 1 addition & 0 deletions pkg/sql/colfetcher/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ go_library(
"//pkg/sql/colencoding",
"//pkg/sql/colexec/colexecargs",
"//pkg/sql/colexec/colexecspan",
"//pkg/sql/colexec/colexecutils",
"//pkg/sql/colexecerror",
"//pkg/sql/colexecop",
"//pkg/sql/colmem",
Expand Down
Loading

0 comments on commit 2c23b80

Please sign in to comment.