Merge cockroachdb#85782

85782: sql: almost remove context.Context from eval.Context r=yuzefovich a=yuzefovich

This PR contains several commits that take steps towards the removal of the stored `context.Context` from `eval.Context`. The largest change is introducing an explicit `context.Context` argument to the `tree.TypedExpr.Eval` method (this is done in the first commit). The remaining commits remove accesses to the stored context in most cases (via either already available context in scope or small plumbing).

This PR doesn't completely remove the stored context because there were two methods `eval.Context.MustGetPlaceholderValue` and `eval.UnwrapDatum` (when unwrapping a placeholder) that still use it. Performing the plumbing for those turned out to be a daunting task, so it was abandoned.

In order to not introduce any new usages this PR unexports the stored context, provides the setter method, and marks both as deprecated.

Epic: None

Co-authored-by: Yahor Yuzefovich <[email protected]>

build/bazelutil/nogo_config.json

@@ -727,7 +727,16 @@
             "cockroach/pkg/.*\\.eg\\.go$": "generated code",
             ".*\\.pb\\.go$": "generated code",
             ".*\\.pb\\.gw\\.go$": "generated code",
-            "cockroach/pkg/.*_generated\\.go$": "generated code"
+            "cockroach/pkg/.*_generated\\.go$": "generated code",
+            "pkg/sql/conn_executor.go$": "temporary exclusion until deprecatedContext is removed from eval.Context",
+            "pkg/sql/instrumentation.go$": "temporary exclusion until deprecatedContext is removed from eval.Context",
+            "pkg/sql/planner.go$": "temporary exclusion until deprecatedContext is removed from eval.Context",
+            "pkg/sql/schema_changer.go$": "temporary exclusion until deprecatedContext is removed from eval.Context",
+            "pkg/sql/distsql/server.go$": "temporary exclusion until deprecatedContext is removed from eval.Context",
+            "pkg/sql/execinfra/processorsbase.go$": "temporary exclusion until deprecatedContext is removed from eval.Context",
+            "pkg/sql/importer/import_table_creation.go$": "temporary exclusion until deprecatedContext is removed from eval.Context",
+            "pkg/sql/row/expr_walker_test.go$": "temporary exclusion until deprecatedContext is removed from eval.Context",
+            "pkg/sql/schemachanger/scbuild/tree_context_builder.go$": "temporary exclusion until deprecatedContext is removed from eval.Context"
         },
         "only_files": {
             "cockroach/pkg/.*$": "first-party code"

docs/codelabs/00-sql-function.md

@@ -77,7 +77,7 @@ var builtins = map[string]builtinDefinition{
     tree.Overload{
       Types:      tree.VariadicType{VarType: types.String},
       ReturnType: tree.FixedReturnType(types.String),
-      Fn: func(ctx *eval.Context, args tree.Datums) (tree.Datum, error) {
+      Fn: func(ctx context.Context, evalCtx *eval.Context, args tree.Datums) (tree.Datum, error) {
         return tree.DNull, fmt.Errorf("nothing to see here")
       },
     },
@@ -349,7 +349,7 @@ check your solution against ours.
       tree.Overload{
         Types:      tree.VariadicType{VarType: types.String},
         ReturnType: tree.FixedReturnType(types.String),
-        Fn: func(ctx *eval.Context, args tree.Datums) (tree.Datum, error) {
+        Fn: func(ctx context.Context, evalCtx *eval.Context, args tree.Datums) (tree.Datum, error) {
           users := map[string]string{
             "bdarnell": "Ben Darnell",
             "pmattis":  "Peter Mattis",

pkg/ccl/changefeedccl/avro_test.go

@@ -119,7 +119,7 @@ func parseValues(tableDesc catalog.TableDescriptor, values string) ([]rowenc.Enc
 			if err != nil {
 				return nil, err
 			}
-			datum, err := eval.Expr(evalCtx, typedExpr)
+			datum, err := eval.Expr(ctx, evalCtx, typedExpr)
 			if err != nil {
 				return nil, errors.Wrapf(err, "evaluating %s", typedExpr)
 			}

pkg/ccl/changefeedccl/cdceval/expr_eval.go

@@ -293,7 +293,7 @@ func (e *exprEval) evalProjection(
 		if err != nil {
 			return cdcevent.Row{}, err
 		}
-		if err := e.projection.SetValueDatumAt(e.evalCtx, i, d); err != nil {
+		if err := e.projection.SetValueDatumAt(ctx, e.evalCtx, i, d); err != nil {
 			return cdcevent.Row{}, err
 		}
 	}
@@ -425,7 +425,7 @@ func (e *exprEval) typeCheck(
 		ctx, expr, targetType, "cdc", e.semaCtx,
 		volatility.Immutable, true)
 	if err == nil {
-		d, err := eval.Expr(e.evalCtx, typedExpr)
+		d, err := eval.Expr(ctx, e.evalCtx, typedExpr)
 		if err != nil {
 			return nil, err
 		}
@@ -448,7 +448,7 @@ func (e *exprEval) typeCheck(
 	if err != nil {
 		return nil, err
 	}
-	return normalize.Expr(e.evalCtx, typedExpr)
+	return normalize.Expr(ctx, e.evalCtx, typedExpr)
 }
 
 // evalExpr evaluates typed expression and returns resulting datum.
@@ -476,7 +476,7 @@ func (e *exprEval) evalExpr(
 		if err != nil {
 			return nil, err
 		}
-		d, err := eval.Expr(e.evalCtx, typedExpr)
+		d, err := eval.Expr(ctx, e.evalCtx, typedExpr)
 		if err != nil {
 			return nil, err
 		}

pkg/ccl/changefeedccl/cdceval/functions.go

@@ -9,6 +9,7 @@
 package cdceval
 
 import (
+	"context"
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/ccl/changefeedccl/cdcevent"
@@ -61,7 +62,7 @@ var cdcFunctions = map[string]*tree.ResolvedFunctionDefinition{
 		tree.Overload{
 			Types:      tree.ArgTypes{},
 			ReturnType: tree.FixedReturnType(types.Bool),
-			Fn: func(evalCtx *eval.Context, datums tree.Datums) (tree.Datum, error) {
+			Fn: func(ctx context.Context, evalCtx *eval.Context, datums tree.Datums) (tree.Datum, error) {
 				rowEvalCtx := rowEvalContextFromEvalContext(evalCtx)
 				if rowEvalCtx.updatedRow.IsDeleted() {
 					return tree.DBoolTrue, nil
@@ -130,7 +131,7 @@ func cdcTimestampBuiltin(
 			{
 				Types:      tree.ArgTypes{},
 				ReturnType: tree.FixedReturnType(types.Decimal),
-				Fn: func(evalCtx *eval.Context, datums tree.Datums) (tree.Datum, error) {
+				Fn: func(ctx context.Context, evalCtx *eval.Context, datums tree.Datums) (tree.Datum, error) {
 					rowEvalCtx := rowEvalContextFromEvalContext(evalCtx)
 					return eval.TimestampToDecimalDatum(tsFn(rowEvalCtx)), nil
 				},
@@ -145,7 +146,7 @@ func cdcTimestampBuiltin(
 	return tree.QualifyBuiltinFunctionDefinition(def, catconstants.PublicSchemaName)
 }
 
-func prevRowAsJSON(evalCtx *eval.Context, _ tree.Datums) (tree.Datum, error) {
+func prevRowAsJSON(ctx context.Context, evalCtx *eval.Context, _ tree.Datums) (tree.Datum, error) {
 	rec := rowEvalContextFromEvalContext(evalCtx)
 	if rec.memo.prevJSON != nil {
 		return rec.memo.prevJSON, nil

pkg/ccl/changefeedccl/cdcevent/event.go

@@ -557,7 +557,7 @@ func TestingGetFamilyIDFromKey(
 // MakeRowFromTuple converts a SQL datum produced by, for example, SELECT ROW(foo.*),
 // into the same kind of cdcevent.Row you'd get as a result of an insert, but without
 // the primary key.
-func MakeRowFromTuple(evalCtx *eval.Context, t *tree.DTuple) Row {
+func MakeRowFromTuple(ctx context.Context, evalCtx *eval.Context, t *tree.DTuple) Row {
 	r := Projection{EventDescriptor: &EventDescriptor{}}
 	names := t.ResolvedType().TupleLabels()
 	for i, d := range t.D {
@@ -568,10 +568,10 @@ func MakeRowFromTuple(evalCtx *eval.Context, t *tree.DTuple) Row {
 			name = names[i]
 		}
 		r.AddValueColumn(name, d.ResolvedType())
-		if err := r.SetValueDatumAt(evalCtx, i, d); err != nil {
+		if err := r.SetValueDatumAt(ctx, evalCtx, i, d); err != nil {
 			if build.IsRelease() {
-				log.Warningf(context.Background(), "failed to set row value from tuple due to error %v", err)
-				_ = r.SetValueDatumAt(evalCtx, i, tree.DNull)
+				log.Warningf(ctx, "failed to set row value from tuple due to error %v", err)
+				_ = r.SetValueDatumAt(ctx, evalCtx, i, tree.DNull)
 			} else {
 				panic(err)
 			}

pkg/ccl/changefeedccl/cdcevent/event_test.go

@@ -413,7 +413,7 @@ func TestMakeRowFromTuple(t *testing.T) {
 	st := cluster.MakeTestingClusterSettings()
 	evalCtx := eval.MakeTestingEvalContext(st)
 
-	rowFromUnlabeledTuple := MakeRowFromTuple(&evalCtx, unlabeledTuple)
+	rowFromUnlabeledTuple := MakeRowFromTuple(context.Background(), &evalCtx, unlabeledTuple)
 	expectedCols := []struct {
 		name        string
 		typ         *types.T
@@ -446,7 +446,7 @@ func TestMakeRowFromTuple(t *testing.T) {
 
 	remainingCols = expectedCols
 
-	rowFromLabeledTuple := MakeRowFromTuple(&evalCtx, labeledTuple)
+	rowFromLabeledTuple := MakeRowFromTuple(context.Background(), &evalCtx, labeledTuple)
 
 	require.NoError(t, rowFromLabeledTuple.ForEachColumn().Datum(func(d tree.Datum, col ResultColumn) error {
 		current := remainingCols[0]

pkg/ccl/changefeedccl/cdcevent/projection.go

@@ -9,6 +9,8 @@
 package cdcevent
 
 import (
+	"context"
+
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
@@ -66,7 +68,9 @@ func (p *Projection) AddValueColumn(name string, typ *types.T) {
 }
 
 // SetValueDatumAt sets value datum at specified position.
-func (p *Projection) SetValueDatumAt(evalCtx *eval.Context, pos int, d tree.Datum) error {
+func (p *Projection) SetValueDatumAt(
+	ctx context.Context, evalCtx *eval.Context, pos int, d tree.Datum,
+) error {
 	pos += len(p.keyCols)
 	if pos >= len(p.datums) {
 		return errors.AssertionFailedf("%d out of bounds", pos)
@@ -78,7 +82,7 @@ func (p *Projection) SetValueDatumAt(evalCtx *eval.Context, pos int, d tree.Datu
 			return pgerror.Newf(pgcode.DatatypeMismatch,
 				"expected type %s for column %s@%d, found %s", col.Typ, col.Name, pos, d.ResolvedType())
 		}
-		cd, err := eval.PerformCast(evalCtx, d, col.Typ)
+		cd, err := eval.PerformCast(ctx, evalCtx, d, col.Typ)
 		if err != nil {
 			return errors.Wrapf(err, "expected type %s for column %s@%d, found %s",
 				col.Typ, col.Name, pos, d.ResolvedType())

pkg/ccl/changefeedccl/cdcevent/projection_test.go

@@ -29,8 +29,9 @@ func TestProjection(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
+	ctx := context.Background()
 	s, db, _ := serverutils.StartServer(t, base.TestServerArgs{})
-	defer s.Stopper().Stop(context.Background())
+	defer s.Stopper().Stop(ctx)
 
 	sqlDB := sqlutils.MakeSQLRunner(db)
 	sqlDB.Exec(t, `CREATE TYPE status AS ENUM ('open', 'closed', 'inactive')`)
@@ -61,7 +62,7 @@ CREATE TABLE foo (
 		idx := 0
 		require.NoError(t, input.ForEachColumn().Datum(func(d tree.Datum, col ResultColumn) error {
 			p.AddValueColumn(col.Name, col.Typ)
-			err := p.SetValueDatumAt(&evalCtx, idx, d)
+			err := p.SetValueDatumAt(ctx, &evalCtx, idx, d)
 			idx++
 			return err
 		}))
@@ -76,9 +77,9 @@ CREATE TABLE foo (
 		input := TestingMakeEventRow(desc, 0, encDatums, false)
 		p := MakeProjection(input.EventDescriptor)
 		p.AddValueColumn("wrong_type", types.Int)
-		require.Regexp(t, "expected type int", p.SetValueDatumAt(&evalCtx, 0, tree.NewDString("fail")))
+		require.Regexp(t, "expected type int", p.SetValueDatumAt(ctx, &evalCtx, 0, tree.NewDString("fail")))
 		// But we allow NULL.
-		require.NoError(t, p.SetValueDatumAt(&evalCtx, 0, tree.DNull))
+		require.NoError(t, p.SetValueDatumAt(ctx, &evalCtx, 0, tree.DNull))
 	})
 
 	t.Run("project_extra_column", func(t *testing.T) {
@@ -87,12 +88,12 @@ CREATE TABLE foo (
 		idx := 0
 		require.NoError(t, input.ForEachColumn().Datum(func(d tree.Datum, col ResultColumn) error {
 			p.AddValueColumn(col.Name, col.Typ)
-			err := p.SetValueDatumAt(&evalCtx, idx, d)
+			err := p.SetValueDatumAt(ctx, &evalCtx, idx, d)
 			idx++
 			return err
 		}))
 		p.AddValueColumn("test", types.Int)
-		require.NoError(t, p.SetValueDatumAt(&evalCtx, idx, tree.NewDInt(5)))
+		require.NoError(t, p.SetValueDatumAt(ctx, &evalCtx, idx, tree.NewDInt(5)))
 
 		pr, err := p.Project(input)
 		require.NoError(t, err)

pkg/ccl/changefeedccl/encoder_json.go

@@ -414,8 +414,8 @@ func init() {
 	overload := tree.Overload{
 		Types:      tree.VariadicType{FixedTypes: []*types.T{types.AnyTuple}, VarType: types.String},
 		ReturnType: tree.FixedReturnType(types.Bytes),
-		Fn: func(evalCtx *eval.Context, args tree.Datums) (tree.Datum, error) {
-			row := cdcevent.MakeRowFromTuple(evalCtx, tree.MustBeDTuple(args[0]))
+		Fn: func(ctx context.Context, evalCtx *eval.Context, args tree.Datums) (tree.Datum, error) {
+			row := cdcevent.MakeRowFromTuple(ctx, evalCtx, tree.MustBeDTuple(args[0]))
 			flags := make([]string, len(args)-1)
 			for i, d := range args[1:] {
 				flags[i] = string(tree.MustBeDString(d))

pkg/ccl/changefeedccl/testfeed_test.go

@@ -1379,7 +1379,7 @@ func exprAsString(expr tree.Expr) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	datum, err := eval.Expr(evalCtx, te)
+	datum, err := eval.Expr(context.Background(), evalCtx, te)
 	if err != nil {
 		return "", err
 	}

pkg/ccl/kvccl/kvfollowerreadsccl/boundedstaleness.go

@@ -9,6 +9,7 @@
 package kvfollowerreadsccl
 
 import (
+	"context"
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/ccl/utilccl"
@@ -22,18 +23,20 @@ import (
 	"github.com/cockroachdb/errors"
 )
 
-func checkBoundedStalenessEnabled(ctx *eval.Context) error {
-	st := ctx.Settings
+func checkBoundedStalenessEnabled(evalCtx *eval.Context) error {
+	st := evalCtx.Settings
 	return utilccl.CheckEnterpriseEnabled(
 		st,
-		ctx.ClusterID,
+		evalCtx.ClusterID,
 		sql.ClusterOrganization.Get(&st.SV),
 		"bounded staleness",
 	)
 }
 
-func evalMaxStaleness(ctx *eval.Context, d duration.Duration) (time.Time, error) {
-	if err := checkBoundedStalenessEnabled(ctx); err != nil {
+func evalMaxStaleness(
+	ctx context.Context, evalCtx *eval.Context, d duration.Duration,
+) (time.Time, error) {
+	if err := checkBoundedStalenessEnabled(evalCtx); err != nil {
 		return time.Time{}, err
 	}
 	if d.Compare(duration.FromInt64(0)) < 0 {
@@ -43,15 +46,15 @@ func evalMaxStaleness(ctx *eval.Context, d duration.Duration) (time.Time, error)
 			asof.WithMaxStalenessFunctionName,
 		)
 	}
-	return duration.Add(ctx.GetStmtTimestamp(), d.Mul(-1)), nil
+	return duration.Add(evalCtx.GetStmtTimestamp(), d.Mul(-1)), nil
 }
 
-func evalMinTimestamp(ctx *eval.Context, t time.Time) (time.Time, error) {
-	if err := checkBoundedStalenessEnabled(ctx); err != nil {
+func evalMinTimestamp(ctx context.Context, evalCtx *eval.Context, t time.Time) (time.Time, error) {
+	if err := checkBoundedStalenessEnabled(evalCtx); err != nil {
 		return time.Time{}, err
 	}
 	t = t.Round(time.Microsecond)
-	if stmtTimestamp := ctx.GetStmtTimestamp().Round(time.Microsecond); t.After(stmtTimestamp) {
+	if stmtTimestamp := evalCtx.GetStmtTimestamp().Round(time.Microsecond); t.After(stmtTimestamp) {
 		return time.Time{}, errors.WithDetailf(
 			pgerror.Newf(
 				pgcode.InvalidParameterValue,

pkg/ccl/partitionccl/partition.go

@@ -112,7 +112,7 @@ func valueEncodePartitionTuple(
 			return nil, pgerror.Newf(pgcode.Syntax,
 				"%s: partition values must be constant", typedExpr)
 		}
-		datum, err := eval.Expr(evalCtx, typedExpr)
+		datum, err := eval.Expr(ctx, evalCtx, typedExpr)
 		if err != nil {
 			return nil, errors.Wrapf(err, "evaluating %s", typedExpr)
 		}

pkg/ccl/streamingccl/streamingest/stream_ingest_manager.go

@@ -9,6 +9,8 @@
 package streamingest
 
 import (
+	"context"
+
 	"github.com/cockroachdb/cockroach/pkg/ccl/streamingccl/streampb"
 	"github.com/cockroachdb/cockroach/pkg/ccl/utilccl"
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
@@ -25,22 +27,26 @@ type streamIngestManagerImpl struct{}
 
 // CompleteStreamIngestion implements streaming.StreamIngestManager interface.
 func (r *streamIngestManagerImpl) CompleteStreamIngestion(
-	evalCtx *eval.Context, txn *kv.Txn, ingestionJobID jobspb.JobID, cutoverTimestamp hlc.Timestamp,
+	ctx context.Context,
+	evalCtx *eval.Context,
+	txn *kv.Txn,
+	ingestionJobID jobspb.JobID,
+	cutoverTimestamp hlc.Timestamp,
 ) error {
-	return completeStreamIngestion(evalCtx, txn, ingestionJobID, cutoverTimestamp)
+	return completeStreamIngestion(ctx, evalCtx, txn, ingestionJobID, cutoverTimestamp)
 }
 
 // GetStreamIngestionStats implements streaming.StreamIngestManager interface.
 func (r *streamIngestManagerImpl) GetStreamIngestionStats(
-	evalCtx *eval.Context, txn *kv.Txn, ingestionJobID jobspb.JobID,
+	ctx context.Context, evalCtx *eval.Context, txn *kv.Txn, ingestionJobID jobspb.JobID,
 ) (*streampb.StreamIngestionStats, error) {
-	return getStreamIngestionStats(evalCtx, txn, ingestionJobID)
+	return getStreamIngestionStats(ctx, evalCtx, txn, ingestionJobID)
 }
 
 func newStreamIngestManagerWithPrivilegesCheck(
-	evalCtx *eval.Context,
+	ctx context.Context, evalCtx *eval.Context,
 ) (streaming.StreamIngestManager, error) {
-	isAdmin, err := evalCtx.SessionAccessor.HasAdminRole(evalCtx.Context)
+	isAdmin, err := evalCtx.SessionAccessor.HasAdminRole(ctx)
 	if err != nil {
 		return nil, err
 	}