diff --git a/pkg/sql/BUILD.bazel b/pkg/sql/BUILD.bazel index d459e13fa8e5..c73466d1149e 100644 --- a/pkg/sql/BUILD.bazel +++ b/pkg/sql/BUILD.bazel @@ -211,7 +211,6 @@ go_library( "scrub_fk.go", "scrub_index.go", "scrub_unique_constraint.go", - "select_name_resolution.go", "sequence.go", "sequence_select.go", "serial.go", diff --git a/pkg/sql/alter_role.go b/pkg/sql/alter_role.go index f0ee1ee49667..06be03f536c1 100644 --- a/pkg/sql/alter_role.go +++ b/pkg/sql/alter_role.go @@ -415,7 +415,7 @@ func (p *planner) processSetOrResetClause( expr = paramparse.UnresolvedNameToStrVal(expr) typedValue, err := p.analyzeExpr( - ctx, expr, nil, tree.IndexedVarHelper{}, types.String, false, "ALTER ROLE ... SET ", + ctx, expr, tree.IndexedVarHelper{}, types.String, false, "ALTER ROLE ... SET ", ) if err != nil { return unknown, "", sessionVar{}, nil, wrapSetVarError(err, varName, expr.String()) diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go index 6085b7798dd8..98897c86964d 100644 --- a/pkg/sql/alter_table.go +++ b/pkg/sql/alter_table.go @@ -122,7 +122,6 @@ func (p *planner) AlterTable(ctx context.Context, n *tree.AlterTable) (planNode, } typedExpr, err := p.analyzeExpr( ctx, injectStats.Stats, - nil, /* sources - no name resolution */ tree.IndexedVarHelper{}, types.Jsonb, true, /* requireType */ "INJECT STATISTICS" /* typingContext */) diff --git a/pkg/sql/analyze_expr.go b/pkg/sql/analyze_expr.go index 36a6fa392092..d658723d45a1 100644 --- a/pkg/sql/analyze_expr.go +++ b/pkg/sql/analyze_expr.go @@ -13,14 +13,11 @@ package sql import ( "context" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/types" ) // analyzeExpr performs semantic analysis of an expression, including: -// - replacing sub-queries by a sql.subquery node; -// - resolving names (optional); // - type checking (with optional type enforcement); // - normalization. // The parameters sources and IndexedVars, if both are non-nil, indicate @@ -28,31 +25,20 @@ import ( // as a result. func (p *planner) analyzeExpr( ctx context.Context, - raw tree.Expr, - source *colinfo.DataSourceInfo, + expr tree.Expr, iVarHelper tree.IndexedVarHelper, expectedType *types.T, requireType bool, typingContext string, ) (tree.TypedExpr, error) { - // Perform optional name resolution. - resolved := raw - if source != nil { - var err error - resolved, err = p.resolveNames(raw, source, iVarHelper) - if err != nil { - return nil, err - } - } - // Type check. var typedExpr tree.TypedExpr var err error p.semaCtx.IVarContainer = iVarHelper.Container() if requireType { - typedExpr, err = tree.TypeCheckAndRequire(ctx, resolved, &p.semaCtx, expectedType, typingContext) + typedExpr, err = tree.TypeCheckAndRequire(ctx, expr, &p.semaCtx, expectedType, typingContext) } else { - typedExpr, err = tree.TypeCheck(ctx, resolved, &p.semaCtx, expectedType) + typedExpr, err = tree.TypeCheck(ctx, expr, &p.semaCtx, expectedType) } p.semaCtx.IVarContainer = nil if err != nil { diff --git a/pkg/sql/catalog/schemaexpr/BUILD.bazel b/pkg/sql/catalog/schemaexpr/BUILD.bazel index af1f391d886a..9768fdafb967 100644 --- a/pkg/sql/catalog/schemaexpr/BUILD.bazel +++ b/pkg/sql/catalog/schemaexpr/BUILD.bazel @@ -12,8 +12,8 @@ go_library( "doc.go", "expr.go", "hash_sharded_compute_expr.go", + "name.go", "partial_index.go", - "select_name_resolution.go", "sequence_options.go", "unique_contraint.go", ], diff --git a/pkg/sql/catalog/schemaexpr/expr.go b/pkg/sql/catalog/schemaexpr/expr.go index 5b62ff9eb10e..acf46d9d006a 100644 --- a/pkg/sql/catalog/schemaexpr/expr.go +++ b/pkg/sql/catalog/schemaexpr/expr.go @@ -363,8 +363,14 @@ func newNameResolver( // resolveNames returns an expression equivalent to the input expression with // unresolved names replaced with IndexedVars. func (nr *nameResolver) resolveNames(expr tree.Expr) (tree.Expr, error) { - var v NameResolutionVisitor - return ResolveNamesUsingVisitor(&v, expr, nr.source, *nr.ivarHelper) + v := nameResolutionVisitor{ + iVarHelper: *nr.ivarHelper, + resolver: colinfo.ColumnResolver{ + Source: nr.source, + }, + } + expr, _ = tree.WalkExpr(&v, expr) + return expr, v.err } // addColumn adds a new column to the nameResolver so that it can be resolved in diff --git a/pkg/sql/catalog/schemaexpr/name.go b/pkg/sql/catalog/schemaexpr/name.go new file mode 100644 index 000000000000..e12787dfdc97 --- /dev/null +++ b/pkg/sql/catalog/schemaexpr/name.go @@ -0,0 +1,63 @@ +// Copyright 2016 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. +// +// This file implements the select code that deals with column references +// and resolving column names in expressions. + +package schemaexpr + +import ( + "context" + + "github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo" + "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" +) + +// nameResolutionVisitor is a tree.Visitor implementation used to resolve column +// names in an expression as indexed vars. +type nameResolutionVisitor struct { + err error + iVarHelper tree.IndexedVarHelper + resolver colinfo.ColumnResolver +} + +var _ tree.Visitor = &nameResolutionVisitor{} + +// VisitPre implements tree.Visitor. +func (v *nameResolutionVisitor) VisitPre(expr tree.Expr) (recurse bool, newNode tree.Expr) { + if v.err != nil { + return false, expr + } + + switch t := expr.(type) { + case *tree.UnresolvedName: + vn, err := t.NormalizeVarName() + if err != nil { + v.err = err + return false, expr + } + return v.VisitPre(vn) + + case *tree.ColumnItem: + _, err := colinfo.ResolveColumnItem(context.Background(), &v.resolver, t) + if err != nil { + v.err = err + return false, expr + } + + colIdx := v.resolver.ResolverState.ColIdx + ivar := v.iVarHelper.IndexedVar(colIdx) + return true, ivar + } + return true, expr +} + +// VisitPost implements tree.Visitor. +func (*nameResolutionVisitor) VisitPost(expr tree.Expr) tree.Expr { return expr } diff --git a/pkg/sql/catalog/schemaexpr/select_name_resolution.go b/pkg/sql/catalog/schemaexpr/select_name_resolution.go deleted file mode 100644 index 7f15322a0e4c..000000000000 --- a/pkg/sql/catalog/schemaexpr/select_name_resolution.go +++ /dev/null @@ -1,114 +0,0 @@ -// Copyright 2016 The Cockroach Authors. -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.txt. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. -// -// This file implements the select code that deals with column references -// and resolving column names in expressions. - -package schemaexpr - -import ( - "context" - - "github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo" - "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" -) - -// NameResolutionVisitor is a tree.Visitor implementation used to -// resolve the column names in an expression. -type NameResolutionVisitor struct { - err error - iVarHelper tree.IndexedVarHelper - resolver colinfo.ColumnResolver -} - -var _ tree.Visitor = &NameResolutionVisitor{} - -// VisitPre implements tree.Visitor. -func (v *NameResolutionVisitor) VisitPre(expr tree.Expr) (recurse bool, newNode tree.Expr) { - if v.err != nil { - return false, expr - } - - switch t := expr.(type) { - case *tree.IndexedVar: - // If the indexed var is a standalone ordinal reference, ensure it - // becomes a fully bound indexed var. - t, v.err = v.iVarHelper.BindIfUnbound(t) - if v.err != nil { - return false, expr - } - - return false, t - - case *tree.UnresolvedName: - vn, err := t.NormalizeVarName() - if err != nil { - v.err = err - return false, expr - } - return v.VisitPre(vn) - - case *tree.ColumnItem: - _, err := colinfo.ResolveColumnItem(context.TODO(), &v.resolver, t) - if err != nil { - v.err = err - return false, expr - } - - colIdx := v.resolver.ResolverState.ColIdx - ivar := v.iVarHelper.IndexedVar(colIdx) - return true, ivar - - case *tree.FuncExpr: - // Check for invalid use of *, which, if it is an argument, is the only argument. - if len(t.Exprs) != 1 { - break - } - vn, ok := t.Exprs[0].(tree.VarName) - if !ok { - break - } - vn, v.err = vn.NormalizeVarName() - if v.err != nil { - return false, expr - } - // Save back to avoid re-doing the work later. - t.Exprs[0] = vn - return true, t - } - - return true, expr -} - -// VisitPost implements tree.Visitor. -func (*NameResolutionVisitor) VisitPost(expr tree.Expr) tree.Expr { return expr } - -// ResolveNamesUsingVisitor resolves the names in the given expression. -func ResolveNamesUsingVisitor( - v *NameResolutionVisitor, - expr tree.Expr, - source *colinfo.DataSourceInfo, - ivarHelper tree.IndexedVarHelper, -) (tree.Expr, error) { - *v = NameResolutionVisitor{ - iVarHelper: ivarHelper, - resolver: colinfo.ColumnResolver{ - Source: source, - }, - } - - expr, _ = tree.WalkExpr(v, expr) - return expr, v.err -} - -// Err returns visitor error. -func (v *NameResolutionVisitor) Err() error { - return v.err -} diff --git a/pkg/sql/distsql_plan_stats.go b/pkg/sql/distsql_plan_stats.go index 2cfafe707b97..5db6fafd2ae7 100644 --- a/pkg/sql/distsql_plan_stats.go +++ b/pkg/sql/distsql_plan_stats.go @@ -505,7 +505,7 @@ func (dsp *DistSQLPlanner) createStatsPlan( var rb renderBuilder rb.init(exec.Node(planNode(&scan)), exec.OutputOrdering{}) for i, expr := range exprs { - exprs[i] = rb.r.ivarHelper.Rebind(expr) + exprs[i] = rb.r.ivarHelper.RebindTyped(expr) } rb.setOutput(exprs, resultCols) diff --git a/pkg/sql/execinfrapb/expr.go b/pkg/sql/execinfrapb/expr.go index d8daf92ac7f5..5b9e6925313c 100644 --- a/pkg/sql/execinfrapb/expr.go +++ b/pkg/sql/execinfrapb/expr.go @@ -26,30 +26,6 @@ import ( "github.com/cockroachdb/errors" ) -// ivarBinder is a tree.Visitor that binds ordinal references -// (IndexedVars represented by @1, @2, ...) to an IndexedVarContainer. -type ivarBinder struct { - h *tree.IndexedVarHelper - err error -} - -func (v *ivarBinder) VisitPre(expr tree.Expr) (recurse bool, newExpr tree.Expr) { - if v.err != nil { - return false, expr - } - if ivar, ok := expr.(*tree.IndexedVar); ok { - newVar, err := v.h.BindIfUnbound(ivar) - if err != nil { - v.err = err - return false, expr - } - return false, newVar - } - return true, expr -} - -func (*ivarBinder) VisitPost(expr tree.Expr) tree.Expr { return expr } - // DeserializeExpr deserializes expr, binds the indexed variables to the // provided IndexedVarHelper, and evaluates any constants in the expression. // @@ -130,12 +106,8 @@ func processExpression( return nil, err } - // Bind IndexedVars to our eh.Vars. - v := ivarBinder{h: h, err: nil} - expr, _ = tree.WalkExpr(&v, expr) - if v.err != nil { - return nil, v.err - } + // Bind IndexedVars to h. + expr = h.Rebind(expr) semaCtx.IVarContainer = h.Container() // Convert to a fully typed expression. @@ -355,7 +327,7 @@ func (eh *exprHelper) prepareExpr(ctx context.Context, expr Expression) (tree.Ty return nil, nil } if expr.LocalExpr != nil { - eh.vars.Rebind(expr.LocalExpr) + eh.vars.RebindTyped(expr.LocalExpr) return expr.LocalExpr, nil } return DeserializeExpr(ctx, expr.Expr, eh.semaCtx, eh.evalCtx, &eh.vars) diff --git a/pkg/sql/execinfrapb/expr_test.go b/pkg/sql/execinfrapb/expr_test.go index 6a50e335cb44..7c78f8bc0b12 100644 --- a/pkg/sql/execinfrapb/expr_test.go +++ b/pkg/sql/execinfrapb/expr_test.go @@ -23,25 +23,40 @@ import ( "github.com/cockroachdb/cockroach/pkg/util/leaktest" ) -type testVarContainer struct{} +type testVarContainer struct { + used []bool +} -var _ tree.IndexedVarContainer = testVarContainer{} +var _ tree.IndexedVarContainer = &testVarContainer{} -func (d testVarContainer) IndexedVarResolvedType(idx int) *types.T { +func (d *testVarContainer) IndexedVarResolvedType(idx int) *types.T { + // Track usage of the IndexedVar. + for idx > len(d.used)-1 { + d.used = append(d.used, false) + } + d.used[idx] = true return types.Int } -func (d testVarContainer) IndexedVarNodeFormatter(idx int) tree.NodeFormatter { +func (d *testVarContainer) IndexedVarNodeFormatter(idx int) tree.NodeFormatter { n := tree.Name(fmt.Sprintf("var%d", idx)) return &n } +func (d *testVarContainer) wasUsed(idx int) bool { + if idx < len(d.used) { + return d.used[idx] + } + return false +} + func TestProcessExpression(t *testing.T) { defer leaktest.AfterTest(t)() e := Expression{Expr: "@1 * (@2 + @3) + @1"} - h := tree.MakeIndexedVarHelper(testVarContainer{}, 4) + var c testVarContainer + h := tree.MakeIndexedVarHelper(&c, 4) st := cluster.MakeTestingClusterSettings() evalCtx := eval.MakeTestingEvalContext(st) semaCtx := tree.MakeSemaContext() @@ -50,9 +65,9 @@ func TestProcessExpression(t *testing.T) { t.Fatal(err) } - if !h.IndexedVarUsed(0) || !h.IndexedVarUsed(1) || !h.IndexedVarUsed(2) || h.IndexedVarUsed(3) { + if !c.wasUsed(0) || !c.wasUsed(1) || !c.wasUsed(2) || c.wasUsed(3) { t.Errorf("invalid IndexedVarUsed results %t %t %t %t (expected false false false true)", - h.IndexedVarUsed(0), h.IndexedVarUsed(1), h.IndexedVarUsed(2), h.IndexedVarUsed(3)) + c.wasUsed(0), c.wasUsed(1), c.wasUsed(2), c.wasUsed(3)) } str := expr.String() diff --git a/pkg/sql/opt_exec_factory.go b/pkg/sql/opt_exec_factory.go index aadd05ecffa1..74969d3393ef 100644 --- a/pkg/sql/opt_exec_factory.go +++ b/pkg/sql/opt_exec_factory.go @@ -226,7 +226,7 @@ func (ef *execFactory) ConstructFilter( source: src, } f.ivarHelper = tree.MakeIndexedVarHelper(f, len(src.columns)) - f.filter = f.ivarHelper.Rebind(filter) + f.filter = f.ivarHelper.RebindTyped(filter) if f.filter == nil { // Filter statically evaluates to true. Just return the input plan. return n, nil @@ -383,7 +383,7 @@ func (ef *execFactory) ConstructRender( var rb renderBuilder rb.init(n, reqOrdering) for i, expr := range exprs { - exprs[i] = rb.r.ivarHelper.Rebind(expr) + exprs[i] = rb.r.ivarHelper.RebindTyped(expr) } rb.setOutput(exprs, columns) return rb.res, nil @@ -416,7 +416,7 @@ func (ef *execFactory) ConstructHashJoin( pred.leftEqKey = leftEqColsAreKey pred.rightEqKey = rightEqColsAreKey - pred.onCond = pred.iVarHelper.Rebind(extraOnCond) + pred.onCond = pred.iVarHelper.RebindTyped(extraOnCond) return p.makeJoinNode(leftSrc, rightSrc, pred), nil } @@ -431,7 +431,7 @@ func (ef *execFactory) ConstructApplyJoin( ) (exec.Node, error) { leftSrc := asDataSource(left) pred := makePredicate(joinType, leftSrc.columns, rightColumns) - pred.onCond = pred.iVarHelper.Rebind(onCond) + pred.onCond = pred.iVarHelper.RebindTyped(onCond) return newApplyJoinNode(joinType, leftSrc, rightColumns, pred, planRightSideFn) } @@ -449,7 +449,7 @@ func (ef *execFactory) ConstructMergeJoin( leftSrc := asDataSource(left) rightSrc := asDataSource(right) pred := makePredicate(joinType, leftSrc.columns, rightSrc.columns) - pred.onCond = pred.iVarHelper.Rebind(onCond) + pred.onCond = pred.iVarHelper.RebindTyped(onCond) node := p.makeJoinNode(leftSrc, rightSrc, pred) pred.leftEqKey = leftEqColsAreKey pred.rightEqKey = rightEqColsAreKey @@ -759,13 +759,13 @@ func (ef *execFactory) ConstructLookupJoin( } pred := makePredicate(joinType, planColumns(input.(planNode)), planColumns(tableScan)) if lookupExpr != nil { - n.lookupExpr = pred.iVarHelper.Rebind(lookupExpr) + n.lookupExpr = pred.iVarHelper.RebindTyped(lookupExpr) } if remoteLookupExpr != nil { - n.remoteLookupExpr = pred.iVarHelper.Rebind(remoteLookupExpr) + n.remoteLookupExpr = pred.iVarHelper.RebindTyped(remoteLookupExpr) } if onCond != nil && onCond != tree.DBoolTrue { - n.onCond = pred.iVarHelper.Rebind(onCond) + n.onCond = pred.iVarHelper.RebindTyped(onCond) } n.columns = pred.cols if isFirstJoinInPairedJoiner { @@ -831,7 +831,7 @@ func (ef *execFactory) constructVirtualTableLookupJoin( return nil, errors.AssertionFailedf("unexpected join type for virtual lookup join: %s", joinType.String()) } pred := makePredicate(joinType, inputCols, projectedVtableCols) - pred.onCond = pred.iVarHelper.Rebind(onCond) + pred.onCond = pred.iVarHelper.RebindTyped(onCond) n := &vTableLookupJoinNode{ input: input.(planNode), joinType: joinType, diff --git a/pkg/sql/planner.go b/pkg/sql/planner.go index 1201214af81c..db5bbbedf3b3 100644 --- a/pkg/sql/planner.go +++ b/pkg/sql/planner.go @@ -34,7 +34,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" "github.com/cockroachdb/cockroach/pkg/sql/catalog/lease" "github.com/cockroachdb/cockroach/pkg/sql/catalog/resolver" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/schemaexpr" "github.com/cockroachdb/cockroach/pkg/sql/catalog/schematelemetry/schematelemetrycontroller" "github.com/cockroachdb/cockroach/pkg/sql/clusterunique" "github.com/cockroachdb/cockroach/pkg/sql/evalcatalog" @@ -253,9 +252,8 @@ type planner struct { curPlan planTop // Avoid allocations by embedding commonly used objects and visitors. - txCtx transform.ExprTransformContext - nameResolutionVisitor schemaexpr.NameResolutionVisitor - tableName tree.TableName + txCtx transform.ExprTransformContext + tableName tree.TableName // optPlanningCtx stores the optimizer planning context, which contains // data structures that can be reused between queries (for efficiency). diff --git a/pkg/sql/rename_tenant.go b/pkg/sql/rename_tenant.go index 817045b95662..4a09ef01696a 100644 --- a/pkg/sql/rename_tenant.go +++ b/pkg/sql/rename_tenant.go @@ -41,7 +41,7 @@ func (p *planner) alterRenameTenant( e = tree.NewStrVal(tree.AsStringWithFlags(s, tree.FmtBareIdentifiers)) } tname, err := p.analyzeExpr( - ctx, e, nil, tree.IndexedVarHelper{}, types.String, true, "ALTER VIRTUAL CLUSTER RENAME") + ctx, e, tree.IndexedVarHelper{}, types.String, true, "ALTER VIRTUAL CLUSTER RENAME") if err != nil { return nil, err } diff --git a/pkg/sql/scatter.go b/pkg/sql/scatter.go index baa8b267fcab..f5097180314d 100644 --- a/pkg/sql/scatter.go +++ b/pkg/sql/scatter.go @@ -77,7 +77,7 @@ func (p *planner) Scatter(ctx context.Context, n *tree.Scatter) (planNode, error fromVals := make([]tree.Datum, len(n.From)) for i, expr := range n.From { typedExpr, err := p.analyzeExpr( - ctx, expr, nil, tree.IndexedVarHelper{}, desiredTypes[i], true, "SCATTER", + ctx, expr, tree.IndexedVarHelper{}, desiredTypes[i], true, "SCATTER", ) if err != nil { return nil, err @@ -90,7 +90,7 @@ func (p *planner) Scatter(ctx context.Context, n *tree.Scatter) (planNode, error toVals := make([]tree.Datum, len(n.From)) for i, expr := range n.To { typedExpr, err := p.analyzeExpr( - ctx, expr, nil, tree.IndexedVarHelper{}, desiredTypes[i], true, "SCATTER", + ctx, expr, tree.IndexedVarHelper{}, desiredTypes[i], true, "SCATTER", ) if err != nil { return nil, err diff --git a/pkg/sql/select_name_resolution.go b/pkg/sql/select_name_resolution.go deleted file mode 100644 index a1c6d0b5c9dd..000000000000 --- a/pkg/sql/select_name_resolution.go +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2016 The Cockroach Authors. -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.txt. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. -// -// This file implements the select code that deals with column references -// and resolving column names in expressions. - -package sql - -import ( - "github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/schemaexpr" - "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" -) - -// resolveNames walks the provided expression and resolves all names -// using the tableInfo and iVarHelper. -func (p *planner) resolveNames( - expr tree.Expr, source *colinfo.DataSourceInfo, ivarHelper tree.IndexedVarHelper, -) (tree.Expr, error) { - if expr == nil { - return nil, nil - } - return schemaexpr.ResolveNamesUsingVisitor(&p.nameResolutionVisitor, expr, source, ivarHelper) -} diff --git a/pkg/sql/sem/tree/indexed_vars.go b/pkg/sql/sem/tree/indexed_vars.go index 265262ebf3bb..0079b84fc6c9 100644 --- a/pkg/sql/sem/tree/indexed_vars.go +++ b/pkg/sql/sem/tree/indexed_vars.go @@ -32,14 +32,11 @@ type IndexedVarContainer interface { } // IndexedVar is a VariableExpr that can be used as a leaf in expressions; it -// represents a dynamic value. It defers calls to TypeCheck, Eval, String to an +// represents a dynamic value. It defers calls to TypeCheck and Format to an // IndexedVarContainer. type IndexedVar struct { - Idx int - Used bool - + Idx int col NodeFormatter - typeAnnotation } @@ -119,26 +116,6 @@ func (h *IndexedVarHelper) Container() IndexedVarContainer { return h.container } -// BindIfUnbound ensures the IndexedVar is attached to this helper's container. -// - for freshly created IndexedVars (with a nil container) this will bind in-place. -// - for already bound IndexedVar, bound to this container, this will return the same ivar unchanged. -// - for ordinal references (with an explicit unboundContainer) this will return a new var. -// - for already bound IndexedVars, bound to another container, this will error out. -func (h *IndexedVarHelper) BindIfUnbound(ivar *IndexedVar) (*IndexedVar, error) { - // We perform the range check always, even if the ivar is already - // bound, as a form of safety assertion against misreuse of ivars - // across containers. - if ivar.Idx < 0 || ivar.Idx >= len(h.vars) { - return ivar, pgerror.Newf( - pgcode.UndefinedColumn, "invalid column ordinal: @%d", ivar.Idx+1) - } - - if !ivar.Used { - return h.IndexedVar(ivar.Idx), nil - } - return ivar, nil -} - // MakeIndexedVarHelper initializes an IndexedVarHelper structure. func MakeIndexedVarHelper(container IndexedVarContainer, numVars int) IndexedVarHelper { return IndexedVarHelper{ @@ -167,7 +144,6 @@ func (h *IndexedVarHelper) IndexedVar(idx int) *IndexedVar { h.checkIndex(idx) v := &h.vars[idx] v.Idx = idx - v.Used = true v.typ = h.container.IndexedVarResolvedType(idx) v.col = h.container.IndexedVarNodeFormatter(idx) return v @@ -181,28 +157,28 @@ func (h *IndexedVarHelper) IndexedVarWithType(idx int, typ *types.T) *IndexedVar h.checkIndex(idx) v := &h.vars[idx] v.Idx = idx - v.Used = true v.typ = typ return v } -// IndexedVarUsed returns true if IndexedVar() was called for the given index. -// The index must be valid. -func (h *IndexedVarHelper) IndexedVarUsed(idx int) bool { - h.checkIndex(idx) - return h.vars[idx].Used -} - // GetIndexedVars returns the indexed var array of this helper. -// IndexedVars to the caller; unused vars are guaranteed to have -// a false Used field. func (h *IndexedVarHelper) GetIndexedVars() []IndexedVar { return h.vars } // Rebind collects all the IndexedVars in the given expression and re-binds them // to this helper. -func (h *IndexedVarHelper) Rebind(expr TypedExpr) TypedExpr { +func (h *IndexedVarHelper) Rebind(expr Expr) Expr { + if expr == nil { + return nil + } + ret, _ := WalkExpr(h, expr) + return ret +} + +// RebindTyped collects all the IndexedVars in the given typed expression and +// re-binds them to this helper. +func (h *IndexedVarHelper) RebindTyped(expr TypedExpr) TypedExpr { if expr == nil { return nil } @@ -241,7 +217,7 @@ func (tc *typeContainer) IndexedVarNodeFormatter(idx int) NodeFormatter { // MakeTypesOnlyIndexedVarHelper creates an IndexedVarHelper which provides // the given types for indexed vars. It does not support evaluation, unless -// Rebind is used with another container which supports evaluation. +// RebindTyped is used with another container which supports evaluation. func MakeTypesOnlyIndexedVarHelper(types []*types.T) IndexedVarHelper { c := &typeContainer{types: types} return MakeIndexedVarHelper(c, len(types)) diff --git a/pkg/sql/sem/tree/indexed_vars_test.go b/pkg/sql/sem/tree/indexed_vars_test.go index 95e19d615752..1afa8ab24cae 100644 --- a/pkg/sql/sem/tree/indexed_vars_test.go +++ b/pkg/sql/sem/tree/indexed_vars_test.go @@ -24,18 +24,24 @@ import ( "github.com/cockroachdb/cockroach/pkg/util/log" ) -type testVarContainer []tree.Datum +type testVarContainerSlot struct { + d tree.Datum + used bool +} + +type testVarContainer []testVarContainerSlot var _ eval.IndexedVarContainer = testVarContainer{} func (d testVarContainer) IndexedVarEval( ctx context.Context, idx int, e tree.ExprEvaluator, ) (tree.Datum, error) { - return d[idx].Eval(ctx, e) + return d[idx].d.Eval(ctx, e) } func (d testVarContainer) IndexedVarResolvedType(idx int) *types.T { - return d[idx].ResolvedType() + d[idx].used = true + return d[idx].d.ResolvedType() } func (d testVarContainer) IndexedVarNodeFormatter(idx int) tree.NodeFormatter { @@ -47,10 +53,10 @@ func TestIndexedVars(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) c := make(testVarContainer, 4) - c[0] = tree.NewDInt(3) - c[1] = tree.NewDInt(5) - c[2] = tree.NewDInt(6) - c[3] = tree.NewDInt(0) + c[0].d = tree.NewDInt(3) + c[1].d = tree.NewDInt(5) + c[2].d = tree.NewDInt(6) + c[3].d = tree.NewDInt(0) h := tree.MakeIndexedVarHelper(c, 4) @@ -59,9 +65,9 @@ func TestIndexedVars(t *testing.T) { v1 := h.IndexedVar(1) v2 := h.IndexedVar(2) - if !h.IndexedVarUsed(0) || !h.IndexedVarUsed(1) || !h.IndexedVarUsed(2) || h.IndexedVarUsed(3) { + if !c[0].used || !c[1].used || !c[2].used || c[3].used { t.Errorf("invalid IndexedVarUsed results %t %t %t %t (expected false false false true)", - h.IndexedVarUsed(0), h.IndexedVarUsed(1), h.IndexedVarUsed(2), h.IndexedVarUsed(3)) + c[0].used, c[1].used, c[2].used, c[3].used) } binary := func(op treebin.BinaryOperator, left, right tree.Expr) tree.Expr { diff --git a/pkg/sql/set_cluster_setting.go b/pkg/sql/set_cluster_setting.go index 7847310f4ca2..85251d568276 100644 --- a/pkg/sql/set_cluster_setting.go +++ b/pkg/sql/set_cluster_setting.go @@ -277,7 +277,7 @@ func (p *planner) getAndValidateTypedClusterSetting( requiredType = types.Interval // Ensure that the expression contains a unit (i.e can't be a float) _, err := p.analyzeExpr( - ctx, expr, nil, dummyHelper, types.Float, false, "SET CLUSTER SETTING "+string(name), + ctx, expr, dummyHelper, types.Float, false, "SET CLUSTER SETTING "+string(name), ) // An interval with a unit (valid) will return an // "InvalidTextRepresentation" error when trying to parse it as a float. @@ -293,7 +293,7 @@ func (p *planner) getAndValidateTypedClusterSetting( } typed, err := p.analyzeExpr( - ctx, expr, nil, dummyHelper, requiredType, true, "SET CLUSTER SETTING "+string(name)) + ctx, expr, dummyHelper, requiredType, true, "SET CLUSTER SETTING "+string(name)) if err != nil { hasHint, hint := setting.ErrorHint() if hasHint { diff --git a/pkg/sql/set_var.go b/pkg/sql/set_var.go index f9e3a644a7dd..88d0cb499812 100644 --- a/pkg/sql/set_var.go +++ b/pkg/sql/set_var.go @@ -94,7 +94,7 @@ func (p *planner) SetVar(ctx context.Context, n *tree.SetVar) (planNode, error) var dummyHelper tree.IndexedVarHelper typedValue, err := p.analyzeExpr( - ctx, expr, nil, dummyHelper, types.String, false, "SET SESSION "+name) + ctx, expr, dummyHelper, types.String, false, "SET SESSION "+name) if err != nil { return nil, wrapSetVarError(err, name, expr.String()) } diff --git a/pkg/sql/set_zone_config.go b/pkg/sql/set_zone_config.go index baee896bc137..83eac60f3b9b 100644 --- a/pkg/sql/set_zone_config.go +++ b/pkg/sql/set_zone_config.go @@ -190,7 +190,7 @@ func (p *planner) getUpdatedZoneConfigYamlConfig( var err error yamlConfig, err = p.analyzeExpr( - ctx, n, nil, tree.IndexedVarHelper{}, types.String, false /*requireType*/, "configure zone") + ctx, n, tree.IndexedVarHelper{}, types.String, false /*requireType*/, "configure zone") if err != nil { return nil, err } @@ -240,7 +240,7 @@ func (p *planner) getUpdatedZoneConfigOptions( continue } valExpr, err := p.analyzeExpr( - ctx, opt.Value, nil, tree.IndexedVarHelper{}, req.requiredType, true /*requireType*/, string(opt.Key)) + ctx, opt.Value, tree.IndexedVarHelper{}, req.requiredType, true /*requireType*/, string(opt.Key)) if err != nil { return nil, err } diff --git a/pkg/sql/tenant_capability.go b/pkg/sql/tenant_capability.go index ede65e9ee822..1e9326750ff7 100644 --- a/pkg/sql/tenant_capability.go +++ b/pkg/sql/tenant_capability.go @@ -95,7 +95,6 @@ func (p *planner) AlterTenantCapability( typedValue, err = p.analyzeExpr( ctx, update.Value, - nil, /* source */ dummyHelper, desiredType, true, /* requireType */ diff --git a/pkg/sql/tenant_spec.go b/pkg/sql/tenant_spec.go index 1c36cf3cc477..f637c4dda9df 100644 --- a/pkg/sql/tenant_spec.go +++ b/pkg/sql/tenant_spec.go @@ -48,7 +48,7 @@ func (p *planner) planTenantSpec( if !ts.IsName { // By-ID reference. typedTenantID, err := p.analyzeExpr( - ctx, ts.Expr, nil, dummyHelper, types.Int, true, op) + ctx, ts.Expr, dummyHelper, types.Int, true, op) if err != nil { return nil, err } @@ -66,7 +66,7 @@ func (p *planner) planTenantSpec( } typedTenantName, err := p.analyzeExpr( - ctx, e, nil, dummyHelper, types.String, true, op) + ctx, e, dummyHelper, types.String, true, op) if err != nil { return nil, err }