Skip to content

Commit

Permalink
Merge #101486
Browse files Browse the repository at this point in the history
101486: sql: use global defaults for internal session data rather than zero-values r=chengxiong-ruan a=rafiss

fixes #70888

Release note: None

Co-authored-by: Rafi Shamim <[email protected]>
  • Loading branch information
craig[bot] and rafiss committed Apr 29, 2023
2 parents 2c9bdbb + 7409dab commit 1ee30a1
Show file tree
Hide file tree
Showing 126 changed files with 579 additions and 548 deletions.
1 change: 0 additions & 1 deletion pkg/bench/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ go_test(
"//pkg/sql/parser",
"//pkg/sql/sem/tree",
"//pkg/sql/sessiondata",
"//pkg/sql/sessiondatapb",
"//pkg/sql/types",
"//pkg/testutils/serverutils",
"//pkg/testutils/skip",
Expand Down
10 changes: 5 additions & 5 deletions pkg/bench/bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/parser"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
Expand Down Expand Up @@ -1475,14 +1474,15 @@ func BenchmarkFuncExprTypeCheck(b *testing.B) {

ctx := context.Background()
execCfg := s.ExecutorConfig().(sql.ExecutorConfig)
p, cleanup := sql.NewInternalPlanner("type-check-benchmark",
sd := sql.NewInternalSessionData(ctx, execCfg.Settings, "type-check-benchmark")
sd.Database = "defaultdb"
p, cleanup := sql.NewInternalPlanner(
"type-check-benchmark",
kvDB.NewTxn(ctx, "type-check-benchmark-planner"),
username.RootUserName(),
&sql.MemoryMetrics{},
&execCfg,
sessiondatapb.SessionData{
Database: "defaultdb",
},
sd,
)

defer cleanup()
Expand Down
1 change: 1 addition & 0 deletions pkg/ccl/backupccl/create_scheduled_backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1487,6 +1487,7 @@ WITH SCHEDULE OPTIONS on_execution_failure = 'pause', ignore_existing_backups, f
CommonEventDetails: logpb.CommonEventDetails{
EventType: "recovery_event",
},
ApplicationName: "$ internal-exec-backup",
RecoveryType: scheduledBackupEventType,
TargetScope: clusterScope.String(),
TargetCount: 1,
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/schedule_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (e *scheduledBackupExecutor) executeBackup(
log.Infof(ctx, "Starting scheduled backup %d", sj.ScheduleID())

// Invoke backup plan hook.
hook, cleanup := cfg.PlanHookMaker("exec-backup", txn.KV(), sj.Owner())
hook, cleanup := cfg.PlanHookMaker(ctx, "exec-backup", txn.KV(), sj.Owner())
defer cleanup()

if knobs, ok := cfg.TestingKnobs.(*jobs.TestingKnobs); ok {
Expand Down
1 change: 1 addition & 0 deletions pkg/ccl/changefeedccl/cdceval/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ go_library(
"//pkg/sql/sem/eval",
"//pkg/sql/sem/tree",
"//pkg/sql/sem/volatility",
"//pkg/sql/sessiondata",
"//pkg/sql/sessiondatapb",
"//pkg/sql/types",
"//pkg/util/ctxgroup",
Expand Down
10 changes: 5 additions & 5 deletions pkg/ccl/changefeedccl/cdceval/expr_eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/rowenc"
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
Expand All @@ -40,7 +40,7 @@ type Evaluator struct {
// Execution context.
execCfg *sql.ExecutorConfig
user username.SQLUsername
sessionData sessiondatapb.SessionData
sessionData *sessiondata.SessionData
withDiff bool
familyEval map[descpb.FamilyID]*familyEvaluator
}
Expand All @@ -64,7 +64,7 @@ type familyEvaluator struct {
// Execution context.
execCfg *sql.ExecutorConfig
user username.SQLUsername
sessionData sessiondatapb.SessionData
sessionData *sessiondata.SessionData

// rowCh receives projection datums.
rowCh chan tree.Datums
Expand All @@ -80,7 +80,7 @@ func NewEvaluator(
sc *tree.SelectClause,
execCfg *sql.ExecutorConfig,
user username.SQLUsername,
sd sessiondatapb.SessionData,
sd *sessiondata.SessionData,
statementTS hlc.Timestamp,
withDiff bool,
) *Evaluator {
Expand All @@ -101,7 +101,7 @@ func newFamilyEvaluator(
targetFamilyID descpb.FamilyID,
execCfg *sql.ExecutorConfig,
user username.SQLUsername,
sd sessiondatapb.SessionData,
sd *sessiondata.SessionData,
statementTS hlc.Timestamp,
withDiff bool,
) *familyEvaluator {
Expand Down
12 changes: 8 additions & 4 deletions pkg/ccl/changefeedccl/cdceval/expr_eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -784,8 +784,12 @@ func newEvaluatorWithNormCheck(
defaultDBSessionData, hlc.Timestamp{}, withDiff), nil
}

var defaultDBSessionData = sessiondatapb.SessionData{
Database: "defaultdb",
SearchPath: sessiondata.DefaultSearchPath.GetPathArray(),
TrigramSimilarityThreshold: 0.3,
var defaultDBSessionData = &sessiondata.SessionData{
SessionData: sessiondatapb.SessionData{
Database: "defaultdb",
TrigramSimilarityThreshold: 0.3,
UserProto: username.RootUserName().EncodeProto(),
},
SequenceState: sessiondata.NewSequenceState(),
SearchPath: sessiondata.DefaultSearchPathForUser(username.RootUserName()),
}
7 changes: 4 additions & 3 deletions pkg/ccl/changefeedccl/cdceval/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/errors"
Expand All @@ -45,7 +46,7 @@ func NormalizeExpression(
// Even though we have a job exec context, we shouldn't muck with it.
// Make our own copy of the planner instead.
if err := withPlanner(
ctx, execCtx.ExecCfg(), execCtx.User(), schemaTS, execCtx.SessionData().SessionData,
ctx, execCtx.ExecCfg(), execCtx.User(), schemaTS, execCtx.SessionData(),
func(ctx context.Context, execCtx sql.JobExecContext, cleanup func()) (err error) {
defer cleanup()
norm, withDiff, err = normalizeExpression(ctx, execCtx, descr, schemaTS, target, sc, splitFams)
Expand Down Expand Up @@ -106,7 +107,7 @@ func SpansForExpression(
ctx context.Context,
execCfg *sql.ExecutorConfig,
user username.SQLUsername,
sd sessiondatapb.SessionData,
sd *sessiondata.SessionData,
descr catalog.TableDescriptor,
schemaTS hlc.Timestamp,
target jobspb.ChangefeedTargetSpecification,
Expand Down Expand Up @@ -165,7 +166,7 @@ func withPlanner(
execCfg *sql.ExecutorConfig,
user username.SQLUsername,
schemaTS hlc.Timestamp,
sd sessiondatapb.SessionData,
sd *sessiondata.SessionData,
fn func(ctx context.Context, execCtx sql.JobExecContext, cleanup func()) error,
) error {
return sql.DescsTxn(ctx, execCfg, func(ctx context.Context, txn isql.Txn, col *descs.Collection) error {
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/changefeedccl/cdceval/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/rowenc/keyside"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
Expand Down Expand Up @@ -312,7 +312,7 @@ func normalizeAndPlan(
ctx context.Context,
execCfg *sql.ExecutorConfig,
user username.SQLUsername,
sd sessiondatapb.SessionData,
sd *sessiondata.SessionData,
descr catalog.TableDescriptor,
schemaTS hlc.Timestamp,
target jobspb.ChangefeedTargetSpecification,
Expand Down
5 changes: 2 additions & 3 deletions pkg/ccl/changefeedccl/changefeed_dist.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/rowexec"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catid"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
Expand Down Expand Up @@ -207,9 +206,9 @@ func fetchSpansForTables(

// SessionData is nil if the changefeed was created prior to
// clusterversion.V23_1_ChangefeedExpressionProductionReady
var sd sessiondatapb.SessionData
sd := sql.NewInternalSessionData(ctx, execCtx.ExecCfg().Settings, "changefeed-fetchSpansForTables")
if details.SessionData != nil {
sd = *details.SessionData
sd.SessionData = *details.SessionData
}
return cdceval.SpansForExpression(ctx, execCtx.ExecCfg(), execCtx.User(),
sd, tableDescs[0], initialHighwater, target, sc)
Expand Down
5 changes: 2 additions & 3 deletions pkg/ccl/changefeedccl/event_processing.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb"
"github.com/cockroachdb/cockroach/pkg/util/admission"
"github.com/cockroachdb/cockroach/pkg/util/admission/admissionpb"
"github.com/cockroachdb/cockroach/pkg/util/bufalloc"
Expand Down Expand Up @@ -274,7 +273,7 @@ func newEvaluator(
return nil, err
}

var sd sessiondatapb.SessionData
sd := sql.NewInternalSessionData(ctx, cfg.Settings, "changefeed-evaluator")
if spec.Feed.SessionData == nil {
// This changefeed was created prior to
// clusterversion.V23_1_ChangefeedExpressionProductionReady; thus we must
Expand All @@ -294,7 +293,7 @@ func newEvaluator(
sc = newExpr
}
} else {
sd = *spec.Feed.SessionData
sd.SessionData = *spec.Feed.SessionData
}

return cdceval.NewEvaluator(sc, cfg, spec.User(), sd, spec.Feed.StatementTime, withDiff), nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/changefeedccl/scheduled_changefeed.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func (s *scheduledChangefeedExecutor) executeChangefeed(
sj.ScheduleID(), tree.AsString(changefeedStmt))

// Invoke changefeed plan hook.
hook, cleanup := cfg.PlanHookMaker("exec-changefeed", txn.KV(), sj.Owner())
hook, cleanup := cfg.PlanHookMaker(ctx, "exec-changefeed", txn.KV(), sj.Owner())
defer cleanup()
changefeedFn, err := planCreateChangefeed(ctx, hook.(sql.PlanHookState), changefeedStmt)
if err != nil {
Expand Down
17 changes: 8 additions & 9 deletions pkg/ccl/changefeedccl/scheduled_changefeed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/parser"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
Expand Down Expand Up @@ -708,13 +707,13 @@ func TestCheckScheduleAlreadyExists(t *testing.T) {

ctx := context.Background()

sd := sql.NewInternalSessionData(ctx, execCfg.Settings, "test")
sd.Database = "d"
p, cleanup := sql.NewInternalPlanner("test",
execCfg.DB.NewTxn(ctx, "test-planner"),
username.RootUserName(), &sql.MemoryMetrics{}, &execCfg,
sessiondatapb.SessionData{
Database: "d",
SearchPath: sessiondata.DefaultSearchPath.GetPathArray(),
})
sd,
)
defer cleanup()

present, err := schedulebase.CheckScheduleAlreadyExists(ctx, p.(sql.PlanHookState), "simple")
Expand Down Expand Up @@ -743,13 +742,13 @@ func TestFullyQualifyTables(t *testing.T) {
require.NoError(t, err)
createChangeFeedStmt := stmt.AST.(*tree.CreateChangefeed)

sd := sql.NewInternalSessionData(ctx, execCfg.Settings, "test")
sd.Database = "ocean"
p, cleanupPlanHook := sql.NewInternalPlanner("test",
execCfg.DB.NewTxn(ctx, "test-planner"),
username.RootUserName(), &sql.MemoryMetrics{}, &execCfg,
sessiondatapb.SessionData{
Database: "ocean",
SearchPath: sessiondata.DefaultSearchPath.GetPathArray(),
})
sd,
)
defer cleanupPlanHook()

tablePatterns := make([]tree.TablePattern, 0)
Expand Down
1 change: 0 additions & 1 deletion pkg/ccl/logictestccl/testdata/logic_test/redact_descriptor
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ SELECT descriptor from redacted_descriptors where id = 'collate_partition'::REGC
"id": 2,
"name": "b",
"nullable": true,
"pgAttributeNum": 2,
"type": {
"family": "StringFamily",
"oid": 25
Expand Down
26 changes: 13 additions & 13 deletions pkg/ccl/schemachangerccl/testdata/decomp/multiregion
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ ElementState:
isHidden: false
isInaccessible: false
isSystemColumn: false
pgAttributeNum: 1
pgAttributeNum: 0
tableId: 110
Status: PUBLIC
- Column:
Expand All @@ -98,7 +98,7 @@ ElementState:
isHidden: true
isInaccessible: false
isSystemColumn: false
pgAttributeNum: 2
pgAttributeNum: 0
tableId: 110
Status: PUBLIC
- Column:
Expand All @@ -108,7 +108,7 @@ ElementState:
isHidden: true
isInaccessible: false
isSystemColumn: true
pgAttributeNum: 4.294967294e+09
pgAttributeNum: 0
tableId: 110
Status: PUBLIC
- Column:
Expand All @@ -118,7 +118,7 @@ ElementState:
isHidden: true
isInaccessible: false
isSystemColumn: true
pgAttributeNum: 4.294967295e+09
pgAttributeNum: 0
tableId: 110
Status: PUBLIC
- PrimaryIndex:
Expand Down Expand Up @@ -365,7 +365,7 @@ ElementState:
isHidden: false
isInaccessible: false
isSystemColumn: false
pgAttributeNum: 1
pgAttributeNum: 0
tableId: 109
Status: PUBLIC
- Column:
Expand All @@ -375,7 +375,7 @@ ElementState:
isHidden: true
isInaccessible: false
isSystemColumn: false
pgAttributeNum: 2
pgAttributeNum: 0
tableId: 109
Status: PUBLIC
- Column:
Expand All @@ -385,7 +385,7 @@ ElementState:
isHidden: true
isInaccessible: false
isSystemColumn: true
pgAttributeNum: 4.294967294e+09
pgAttributeNum: 0
tableId: 109
Status: PUBLIC
- Column:
Expand All @@ -395,7 +395,7 @@ ElementState:
isHidden: true
isInaccessible: false
isSystemColumn: true
pgAttributeNum: 4.294967295e+09
pgAttributeNum: 0
tableId: 109
Status: PUBLIC
- PrimaryIndex:
Expand Down Expand Up @@ -644,7 +644,7 @@ ElementState:
isHidden: false
isInaccessible: false
isSystemColumn: false
pgAttributeNum: 1
pgAttributeNum: 0
tableId: 108
Status: PUBLIC
- Column:
Expand All @@ -654,7 +654,7 @@ ElementState:
isHidden: false
isInaccessible: false
isSystemColumn: false
pgAttributeNum: 2
pgAttributeNum: 0
tableId: 108
Status: PUBLIC
- Column:
Expand All @@ -664,7 +664,7 @@ ElementState:
isHidden: true
isInaccessible: false
isSystemColumn: false
pgAttributeNum: 3
pgAttributeNum: 0
tableId: 108
Status: PUBLIC
- Column:
Expand All @@ -674,7 +674,7 @@ ElementState:
isHidden: true
isInaccessible: false
isSystemColumn: true
pgAttributeNum: 4.294967294e+09
pgAttributeNum: 0
tableId: 108
Status: PUBLIC
- Column:
Expand All @@ -684,7 +684,7 @@ ElementState:
isHidden: true
isInaccessible: false
isSystemColumn: true
pgAttributeNum: 4.294967295e+09
pgAttributeNum: 0
tableId: 108
Status: PUBLIC
- PrimaryIndex:
Expand Down
Loading

0 comments on commit 1ee30a1

Please sign in to comment.