diff --git a/pkg/bench/rttanalysis/internal_executor_bench_test.go b/pkg/bench/rttanalysis/internal_executor_bench_test.go new file mode 100644 index 000000000000..71724a66e248 --- /dev/null +++ b/pkg/bench/rttanalysis/internal_executor_bench_test.go @@ -0,0 +1,45 @@ +// Copyright 2021 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. + +package rttanalysis + +import "testing" + +func BenchmarkInternalExecutor(b *testing.B) { + tests := []RoundTripBenchTestCase{ + { + Name: "has_schema_privilege_1_col", + Setup: `CREATE SCHEMA s;`, + Stmt: `SELECT has_schema_privilege('s', 'CREATE')`, + }, + { + Name: "has_schema_privilege_2_col", + Setup: `CREATE SCHEMA s;`, + Stmt: `SELECT has_schema_privilege('s', 'CREATE'), has_schema_privilege('s', 'CREATE')`, + }, + { + Name: "has_schema_privilege_3_col", + Setup: `CREATE SCHEMA s;`, + Stmt: `SELECT has_schema_privilege('s', 'CREATE'), has_schema_privilege('s', 'CREATE'), has_schema_privilege('s', 'CREATE')`, + }, + { + Name: "has_schema_privilege_3_schemas_3_col", + Setup: `CREATE SCHEMA s; CREATE SCHEMA s2; CREATE SCHEMA s3;`, + Stmt: `SELECT has_schema_privilege('s', 'CREATE'), has_schema_privilege('s', 'CREATE'), has_schema_privilege('s', 'CREATE')`, + }, + { + Name: "has_schema_privilege_5_schemas_3_col", + Setup: `CREATE SCHEMA s; CREATE SCHEMA s2; CREATE SCHEMA s3; CREATE SCHEMA s4; CREATE SCHEMA s5;`, + Stmt: `SELECT has_schema_privilege('s', 'CREATE'), has_schema_privilege('s', 'CREATE'), has_schema_privilege('s', 'CREATE')`, + }, + } + + RunRoundTripBenchmark(b, tests) +} diff --git a/pkg/bench/rttanalysis/testdata/benchmark_expectations b/pkg/bench/rttanalysis/testdata/benchmark_expectations index ccbc3ef164a1..22b41dd3fdc9 100644 --- a/pkg/bench/rttanalysis/testdata/benchmark_expectations +++ b/pkg/bench/rttanalysis/testdata/benchmark_expectations @@ -52,6 +52,11 @@ exp,benchmark 20,Grant/grant_all_on_3_tables 17,GrantRole/grant_1_role 20,GrantRole/grant_2_roles +10,InternalExecutor/has_schema_privilege_1_col +14,InternalExecutor/has_schema_privilege_2_col +18,InternalExecutor/has_schema_privilege_3_col +48,InternalExecutor/has_schema_privilege_3_schemas_3_col +72,InternalExecutor/has_schema_privilege_5_schemas_3_col 2,ORMQueries/activerecord_type_introspection_query 2,ORMQueries/django_table_introspection_1_table 2,ORMQueries/django_table_introspection_4_tables @@ -60,8 +65,13 @@ exp,benchmark 2,ORMQueries/has_column_privilege_using_column_name 2,ORMQueries/has_table_privilege_real_table 2,ORMQueries/has_table_privilege_virtual_table +3,ORMQueries/information_schema._pg_index_position 2,ORMQueries/pg_attribute 2,ORMQueries/pg_class +4,ORMQueries/pg_is_other_temp_schema +4,ORMQueries/pg_is_other_temp_schema_multiple_times +2,ORMQueries/pg_my_temp_schema +2,ORMQueries/pg_my_temp_schema_multiple_times 2,ORMQueries/pg_namespace 2,ORMQueries/pg_type 16,Revoke/revoke_all_on_1_table diff --git a/pkg/bench/rttanalysis/validate_benchmark_data.go b/pkg/bench/rttanalysis/validate_benchmark_data.go index b56ec0cb507d..1c3b8c409125 100644 --- a/pkg/bench/rttanalysis/validate_benchmark_data.go +++ b/pkg/bench/rttanalysis/validate_benchmark_data.go @@ -238,7 +238,6 @@ func rewriteBenchmarkExpectations(t *testing.T, benchmarks []string) { t.Fatalf("duplicate expecatations for Name %s", expectations[i].name) } } - writeExpectationsFile(t, expectations) } diff --git a/pkg/ccl/streamingccl/streamingutils/BUILD.bazel b/pkg/ccl/streamingccl/streamingutils/BUILD.bazel index 2e2664e179dc..7b42eff32262 100644 --- a/pkg/ccl/streamingccl/streamingutils/BUILD.bazel +++ b/pkg/ccl/streamingccl/streamingutils/BUILD.bazel @@ -9,7 +9,9 @@ go_library( "//pkg/jobs", "//pkg/jobs/jobspb", "//pkg/kv", + "//pkg/security", "//pkg/sql/sem/tree", + "//pkg/sql/sessiondata", "//pkg/streaming", "//pkg/util/hlc", "//pkg/util/protoutil", diff --git a/pkg/ccl/streamingccl/streamingutils/utils.go b/pkg/ccl/streamingccl/streamingutils/utils.go index 9ec5d088e552..b1710010273b 100644 --- a/pkg/ccl/streamingccl/streamingutils/utils.go +++ b/pkg/ccl/streamingccl/streamingutils/utils.go @@ -12,9 +12,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/jobs" "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" "github.com/cockroachdb/cockroach/pkg/kv" - "github.com/cockroachdb/cockroach/pkg/security" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" - "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" "github.com/cockroachdb/cockroach/pkg/streaming" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/protoutil" @@ -33,9 +31,7 @@ func doCompleteIngestion( const jobsQuery = `SELECT progress FROM system.jobs WHERE id=$1 FOR UPDATE` row, err := evalCtx.Planner.QueryRowEx(evalCtx.Context, "get-stream-ingestion-job-metadata", - txn, sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - }, jobsQuery, jobID) + txn, evalCtx.SessionData(), jobsQuery, jobID) if err != nil { return err } @@ -89,8 +85,6 @@ func doCompleteIngestion( updateJobQuery := `UPDATE system.jobs SET progress=$1 WHERE id=$2` _, err = evalCtx.Planner.QueryRowEx(evalCtx.Context, "set-stream-ingestion-job-metadata", txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - }, updateJobQuery, progressBytes, jobID) + evalCtx.SessionData(), updateJobQuery, progressBytes, jobID) return err } diff --git a/pkg/server/server_sql.go b/pkg/server/server_sql.go index 9e7202529132..215767bbf4cf 100644 --- a/pkg/server/server_sql.go +++ b/pkg/server/server_sql.go @@ -763,7 +763,7 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) { // Now that we have a pgwire.Server (which has a sql.Server), we can close a // circular dependency between the rowexec.Server and sql.Server and set - // SessionBoundInternalExecutorFactory. The same applies for setting a + // InternalExecutorFactory. The same applies for setting a // SessionBoundInternalExecutor on the job registry. ieFactory := func( ctx context.Context, sessionData *sessiondata.SessionData, @@ -777,9 +777,24 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) { ie.SetSessionData(sessionData) return &ie } + + ieFactoryForBuiltins := func( + ctx context.Context, sessionData *sessiondata.SessionData, extraTxnState sql.ExtraTxnState, + ) sql.InternalExecutor { + ie := sql.MakeInternalExecutor( + ctx, + pgServer.SQLServer, + internalMemMetrics, + cfg.Settings, + ) + ie.SetExtraTxnState(extraTxnState) + ie.SetSessionData(sessionData) + return ie + } distSQLServer.ServerConfig.SessionBoundInternalExecutorFactory = ieFactory jobRegistry.SetSessionBoundInternalExecutorFactory(ieFactory) execCfg.IndexBackfiller = sql.NewIndexBackfiller(execCfg, ieFactory) + execCfg.InternalExecutorFactory = ieFactoryForBuiltins distSQLServer.ServerConfig.ProtectedTimestampProvider = execCfg.ProtectedTimestampProvider diff --git a/pkg/sql/conn_executor.go b/pkg/sql/conn_executor.go index ba7913b75760..395eab1d4de0 100644 --- a/pkg/sql/conn_executor.go +++ b/pkg/sql/conn_executor.go @@ -615,7 +615,7 @@ func (s *Server) SetupConn( ex := s.newConnExecutor( ctx, sdMutIterator, stmtBuf, clientComm, memMetrics, &s.Metrics, - s.sqlStats.GetApplicationStats(sd.ApplicationName), + s.sqlStats.GetApplicationStats(sd.ApplicationName), ExtraTxnState{}, ) return ConnectionHandler{ex}, nil } @@ -724,6 +724,7 @@ func (s *Server) newConnExecutor( memMetrics MemoryMetrics, srvMetrics *Metrics, applicationStats sqlstats.ApplicationStats, + extraTxnState ExtraTxnState, ) *connExecutor { // Create the various monitors. // The session monitors are started in activate(). @@ -809,6 +810,12 @@ func (s *Server) newConnExecutor( } ex.phaseTimes.SetSessionPhaseTime(sessionphase.SessionInit, timeutil.Now()) + ex.mu.ActiveQueries = make(map[ClusterWideID]*queryMeta) + ex.machine = fsm.MakeMachine(TxnStateTransitions, stateNoTxn{}, &ex.state) + + ex.sessionTracing.ex = ex + ex.transitionCtx.sessionTracing = &ex.sessionTracing + ex.extraTxnState.prepStmtsNamespace = prepStmtNamespace{ prepStmts: make(map[string]*PreparedStatement), portals: make(map[string]PreparedPortal), @@ -818,19 +825,20 @@ func (s *Server) newConnExecutor( portals: make(map[string]PreparedPortal), } ex.extraTxnState.prepStmtsNamespaceMemAcc = ex.sessionMon.MakeBoundAccount() - ex.extraTxnState.descCollection = s.cfg.CollectionFactory.MakeCollection( - descs.NewTemporarySchemaProvider(sdMutIterator.sds), - ) ex.extraTxnState.txnRewindPos = -1 ex.extraTxnState.schemaChangeJobRecords = make(map[descpb.ID]*jobs.Record) - ex.mu.ActiveQueries = make(map[ClusterWideID]*queryMeta) - ex.machine = fsm.MakeMachine(TxnStateTransitions, stateNoTxn{}, &ex.state) - - ex.sessionTracing.ex = ex - ex.transitionCtx.sessionTracing = &ex.sessionTracing - ex.extraTxnState.hasAdminRoleCache = HasAdminRoleCache{} + if extraTxnState.descs != nil { + ex.extraTxnState.descCollection = extraTxnState.descs + ex.extraTxnState.keepDescCollectionOnClose = true + } else { + descCol := s.cfg.CollectionFactory.MakeCollection( + descs.NewTemporarySchemaProvider(sdMutIterator.sds), + ) + ex.extraTxnState.descCollection = &descCol + } + ex.initPlanner(ctx, &ex.planner) return ex @@ -857,6 +865,7 @@ func (s *Server) newConnExecutorWithTxn( txn *kv.Txn, syntheticDescs []catalog.Descriptor, applicationStats sqlstats.ApplicationStats, + extraTxnState ExtraTxnState, ) *connExecutor { ex := s.newConnExecutor( ctx, @@ -866,6 +875,7 @@ func (s *Server) newConnExecutorWithTxn( memMetrics, srvMetrics, applicationStats, + extraTxnState, ) if txn.Type() == kv.LeafTxn { // If the txn is a leaf txn it is not allowed to perform mutations. For @@ -1114,7 +1124,12 @@ type connExecutor struct { // transaction finishes or gets retried. extraTxnState struct { // descCollection collects descriptors used by the current transaction. - descCollection descs.Collection + descCollection *descs.Collection + + // keepDescsCollectionOnClose determines if descCollection should be + // flushed on connExec's close. If the descCollection is passed in + // from the parent, we do not want to flush it. + keepDescCollectionOnClose bool // jobs accumulates jobs staged for execution inside the transaction. // Staging happens when executing statements that are implemented with a @@ -1482,7 +1497,9 @@ func (ex *connExecutor) resetExtraTxnState(ctx context.Context, ev txnEvent) err delete(ex.extraTxnState.schemaChangeJobRecords, k) } - ex.extraTxnState.descCollection.ReleaseAll(ctx) + if !ex.extraTxnState.keepDescCollectionOnClose { + ex.extraTxnState.descCollection.ReleaseAll(ctx) + } // Close all portals. for name, p := range ex.extraTxnState.prepStmtsNamespace.portals { @@ -2445,7 +2462,7 @@ func (ex *connExecutor) initEvalCtx(ctx context.Context, evalCtx *extendedEvalCo RegionsServer: ex.server.cfg.RegionsServer, SQLStatusServer: ex.server.cfg.SQLStatusServer, MemMetrics: &ex.memMetrics, - Descs: &ex.extraTxnState.descCollection, + Descs: ex.extraTxnState.descCollection, ExecCfg: ex.server.cfg, DistSQLPlanner: ex.server.cfg.DistSQLPlanner, TxnModesSetter: ex, @@ -2897,7 +2914,7 @@ func (ex *connExecutor) runPreCommitStages(ctx context.Context) error { return nil } executor := scexec.NewExecutor( - ex.planner.txn, &ex.extraTxnState.descCollection, ex.server.cfg.Codec, + ex.planner.txn, ex.extraTxnState.descCollection, ex.server.cfg.Codec, nil /* backfiller */, nil /* jobTracker */, ex.server.cfg.NewSchemaChangerTestingKnobs, ex.server.cfg.JobRegistry, ex.planner.execCfg.InternalExecutor, ) @@ -2948,7 +2965,7 @@ func (ex *connExecutor) runPreCommitStages(ctx context.Context) error { } // Write the job ID to the affected descriptors. if err := scexec.UpdateDescriptorJobIDs( - ctx, ex.planner.Txn(), &ex.extraTxnState.descCollection, descIDs, jobspb.InvalidJobID, job.ID(), + ctx, ex.planner.Txn(), ex.extraTxnState.descCollection, descIDs, jobspb.InvalidJobID, job.ID(), ); err != nil { return err } diff --git a/pkg/sql/conn_executor_exec.go b/pkg/sql/conn_executor_exec.go index e73a10c038f1..803500993a47 100644 --- a/pkg/sql/conn_executor_exec.go +++ b/pkg/sql/conn_executor_exec.go @@ -794,7 +794,7 @@ func (ex *connExecutor) checkDescriptorTwoVersionInvariant(ctx context.Context) ctx, ex.server.cfg.Clock, ex.server.cfg.InternalExecutor, - &ex.extraTxnState.descCollection, + ex.extraTxnState.descCollection, ex.state.mu.txn, inRetryBackoff, ) diff --git a/pkg/sql/exec_util.go b/pkg/sql/exec_util.go index c1936f169ca1..cbaa49d94655 100644 --- a/pkg/sql/exec_util.go +++ b/pkg/sql/exec_util.go @@ -1165,6 +1165,22 @@ type ExecutorConfig struct { // SpanConfigReconciliationJobDeps are used to drive the span config // reconciliation job. SpanConfigReconciliationJobDeps spanconfig.ReconciliationDependencies + + // InternalExecutorFactory is used to create an InternalExecutor binded with + // SessionData and other ExtraTxnState. + // This is currently only for builtin functions where we need to execute sql. + InternalExecutorFactory InternalExecutorFactory +} + +// InternalExecutorFactory is a function that produces a "session +// bound" internal executor. +type InternalExecutorFactory func( + context.Context, *sessiondata.SessionData, ExtraTxnState, +) InternalExecutor + +// ExtraTxnState holds state to initialize an InternalExecutor with. +type ExtraTxnState struct { + descs *descs.Collection } // UpdateVersionSystemSettingHook provides a callback that allows us diff --git a/pkg/sql/explain_bundle.go b/pkg/sql/explain_bundle.go index 092e435243e8..ed2378eee10e 100644 --- a/pkg/sql/explain_bundle.go +++ b/pkg/sql/explain_bundle.go @@ -320,7 +320,7 @@ The UI can then be accessed at http://localhost:16686/search`, stmt) } func (b *stmtBundleBuilder) addEnv(ctx context.Context) { - c := makeStmtEnvCollector(ctx, b.ie, nil) + c := makeStmtEnvCollector(ctx, b.ie) var buf bytes.Buffer if err := c.PrintVersion(&buf); err != nil { @@ -416,10 +416,8 @@ type stmtEnvCollector struct { sessionDataOverride *sessiondata.SessionData } -func makeStmtEnvCollector( - ctx context.Context, ie *InternalExecutor, sessionDataOverride *sessiondata.SessionData, -) stmtEnvCollector { - return stmtEnvCollector{ctx: ctx, ie: ie, sessionDataOverride: sessionDataOverride} +func makeStmtEnvCollector(ctx context.Context, ie *InternalExecutor) stmtEnvCollector { + return stmtEnvCollector{ctx: ctx, ie: ie} } // environmentQuery is a helper to run a query that returns a single string @@ -429,7 +427,7 @@ func (c *stmtEnvCollector) query(query string) (string, error) { c.ctx, "stmtEnvCollector", nil, /* txn */ - sessiondata.InternalExecutorOverride{SessionData: c.sessionDataOverride}, + sessiondata.InternalExecutorOverride{}, query, ) if err != nil { @@ -550,7 +548,7 @@ func (c *stmtEnvCollector) PrintClusterSettings(w io.Writer) error { c.ctx, "stmtEnvCollector", nil, /* txn */ - sessiondata.InternalExecutorOverride{SessionData: c.sessionDataOverride}, + sessiondata.InternalExecutorOverride{}, "SELECT variable, value, description FROM [ SHOW ALL CLUSTER SETTINGS ]", ) if err != nil { diff --git a/pkg/sql/faketreeeval/BUILD.bazel b/pkg/sql/faketreeeval/BUILD.bazel index 01e11885b123..223aad0a25c4 100644 --- a/pkg/sql/faketreeeval/BUILD.bazel +++ b/pkg/sql/faketreeeval/BUILD.bazel @@ -6,6 +6,7 @@ go_library( importpath = "github.com/cockroachdb/cockroach/pkg/sql/faketreeeval", visibility = ["//visibility:public"], deps = [ + "//pkg/kv", "//pkg/security", "//pkg/sql/parser", "//pkg/sql/pgwire/pgcode", diff --git a/pkg/sql/faketreeeval/evalctx.go b/pkg/sql/faketreeeval/evalctx.go index e5173b5d7bb4..772c21e69e5f 100644 --- a/pkg/sql/faketreeeval/evalctx.go +++ b/pkg/sql/faketreeeval/evalctx.go @@ -334,7 +334,7 @@ func (ep *DummyEvalPlanner) QueryRowEx( ctx context.Context, opName string, txn *kv.Txn, - session sessiondata.InternalExecutorOverride, + session *sessiondata.SessionData, stmt string, qargs ...interface{}, ) (tree.Datums, error) { @@ -346,7 +346,7 @@ func (ep *DummyEvalPlanner) QueryIteratorEx( ctx context.Context, opName string, txn *kv.Txn, - session sessiondata.InternalExecutorOverride, + session *sessiondata.SessionData, stmt string, qargs ...interface{}, ) (tree.InternalRows, error) { diff --git a/pkg/sql/internal.go b/pkg/sql/internal.go index 60d72614d364..50036113a58b 100644 --- a/pkg/sql/internal.go +++ b/pkg/sql/internal.go @@ -71,6 +71,8 @@ type InternalExecutor struct { // // Warning: Not safe for concurrent use from multiple goroutines. syntheticDescriptors []catalog.Descriptor + + extraTxnState ExtraTxnState } // WithSyntheticDescriptors sets the synthetic descriptors before running the @@ -125,6 +127,10 @@ func (ie *InternalExecutor) SetSessionData(sessionData *sessiondata.SessionData) ie.sessionDataStack = sessiondata.NewStack(sessionData) } +func (ie *InternalExecutor) SetExtraTxnState(extraTxnState ExtraTxnState) { + ie.extraTxnState = extraTxnState +} + // initConnEx creates a connExecutor and runs it on a separate goroutine. It // takes in a StmtBuf into which commands can be pushed and a WaitGroup that // will be signaled when connEx.run() returns. @@ -143,6 +149,7 @@ func (ie *InternalExecutor) initConnEx( sd *sessiondata.SessionData, stmtBuf *StmtBuf, wg *sync.WaitGroup, + extraTxnState ExtraTxnState, syncCallback func([]resWithPos), errCallback func(error), ) { @@ -182,6 +189,7 @@ func (ie *InternalExecutor) initConnEx( ie.memMetrics, &ie.s.InternalMetrics, applicationStats, + extraTxnState, ) } else { ex = ie.s.newConnExecutorWithTxn( @@ -195,6 +203,7 @@ func (ie *InternalExecutor) initConnEx( txn, ie.syntheticDescriptors, applicationStats, + extraTxnState, ) } @@ -570,9 +579,6 @@ func (ie *InternalExecutor) QueryIteratorEx( // applyOverrides overrides the respective fields from sd for all the fields set on o. func applyOverrides(o sessiondata.InternalExecutorOverride, sd *sessiondata.SessionData) { - if o.SessionData != nil { - *sd = *o.SessionData - } if !o.User.Undefined() { sd.UserProto = o.User.EncodeProto() } @@ -588,7 +594,6 @@ func applyOverrides(o sessiondata.InternalExecutorOverride, sd *sessiondata.Sess if o.DatabaseIDToTempSchemaID != nil { sd.DatabaseIDToTempSchemaID = o.DatabaseIDToTempSchemaID } - sd.StubCatalogTablesEnabled = o.StubCatalogTables } func (ie *InternalExecutor) maybeRootSessionDataOverride( @@ -732,7 +737,7 @@ func (ie *InternalExecutor) execInternal( stmtBuf.Close() _ = rw.addResult(ctx, ieIteratorResult{err: err}) } - ie.initConnEx(ctx, txn, rw, sd, stmtBuf, &wg, syncCallback, errCallback) + ie.initConnEx(ctx, txn, rw, sd, stmtBuf, &wg, ie.extraTxnState, syncCallback, errCallback) typeHints := make(tree.PlaceholderTypes, len(datums)) for i, d := range datums { diff --git a/pkg/sql/opt_exec_factory.go b/pkg/sql/opt_exec_factory.go index c0f3e9b9c025..0485050209b1 100644 --- a/pkg/sql/opt_exec_factory.go +++ b/pkg/sql/opt_exec_factory.go @@ -1153,11 +1153,12 @@ func (e *urlOutputter) finish() (url.URL, error) { func (ef *execFactory) showEnv(plan string, envOpts exec.ExplainEnvData) (exec.Node, error) { var out urlOutputter - c := makeStmtEnvCollector( + ie := ef.planner.extendedEvalCtx.ExecCfg.InternalExecutorFactory( ef.planner.EvalContext().Context, - ef.planner.extendedEvalCtx.ExecCfg.InternalExecutor, ef.planner.SessionData(), + ExtraTxnState{ef.planner.Descriptors()}, ) + c := makeStmtEnvCollector(ef.planner.EvalContext().Context, &ie) // Show the version of Cockroach running. if err := c.PrintVersion(&out.buf); err != nil { diff --git a/pkg/sql/planner.go b/pkg/sql/planner.go index 67eb9cb31a1f..20921bc76129 100644 --- a/pkg/sql/planner.go +++ b/pkg/sql/planner.go @@ -825,21 +825,23 @@ func (p *planner) QueryRowEx( ctx context.Context, opName string, txn *kv.Txn, - session sessiondata.InternalExecutorOverride, + session *sessiondata.SessionData, stmt string, qargs ...interface{}, ) (tree.Datums, error) { - return p.ExecCfg().InternalExecutor.QueryRowEx(ctx, opName, txn, session, stmt, qargs...) + ie := p.ExecCfg().InternalExecutorFactory(ctx, session, ExtraTxnState{descs: p.Descriptors()}) + return ie.QueryRowEx(ctx, opName, txn, sessiondata.InternalExecutorOverride{}, stmt, qargs...) } func (p *planner) QueryIteratorEx( ctx context.Context, opName string, txn *kv.Txn, - session sessiondata.InternalExecutorOverride, + session *sessiondata.SessionData, stmt string, qargs ...interface{}, ) (tree.InternalRows, error) { - rows, err := p.ExecCfg().InternalExecutor.QueryIteratorEx(ctx, opName, txn, session, stmt, qargs...) + ie := p.ExecCfg().InternalExecutorFactory(ctx, session, ExtraTxnState{descs: p.Descriptors()}) + rows, err := ie.QueryIteratorEx(ctx, opName, txn, sessiondata.InternalExecutorOverride{}, stmt, qargs...) return rows.(tree.InternalRows), err } diff --git a/pkg/sql/row/BUILD.bazel b/pkg/sql/row/BUILD.bazel index 6a8836d6309a..a1ef66284c3f 100644 --- a/pkg/sql/row/BUILD.bazel +++ b/pkg/sql/row/BUILD.bazel @@ -97,7 +97,6 @@ go_test( "//pkg/sql/rowenc", "//pkg/sql/rowinfra", "//pkg/sql/sem/tree", - "//pkg/sql/sqlutil", "//pkg/storage", "//pkg/testutils", "//pkg/testutils/serverutils", diff --git a/pkg/sql/sem/builtins/BUILD.bazel b/pkg/sql/sem/builtins/BUILD.bazel index 978eb9e002b8..cd7685c2fcee 100644 --- a/pkg/sql/sem/builtins/BUILD.bazel +++ b/pkg/sql/sem/builtins/BUILD.bazel @@ -61,7 +61,6 @@ go_library( "//pkg/sql/sqlerrors", "//pkg/sql/sqlliveness", "//pkg/sql/sqltelemetry", - "//pkg/sql/sqlutil", "//pkg/sql/types", "//pkg/streaming", "//pkg/util", diff --git a/pkg/sql/sem/builtins/generator_builtins.go b/pkg/sql/sem/builtins/generator_builtins.go index 55a3d47c5b36..f72f5c562fb6 100644 --- a/pkg/sql/sem/builtins/generator_builtins.go +++ b/pkg/sql/sem/builtins/generator_builtins.go @@ -20,7 +20,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/kv/kvclient" "github.com/cockroachdb/cockroach/pkg/roachpb" - "github.com/cockroachdb/cockroach/pkg/security" "github.com/cockroachdb/cockroach/pkg/sql/lexbase" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" @@ -1837,9 +1836,8 @@ func makePayloadsForTraceGenerator( ctx.Ctx(), "crdb_internal.payloads_for_trace", ctx.Txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - }, query, + ctx.SessionData(), + query, traceID, ) if err != nil { @@ -1898,7 +1896,7 @@ type showCreateAllTablesGenerator struct { ids []int64 dbName string acc mon.BoundAccount - user security.SQLUsername + sessionData *sessiondata.SessionData // The following variables are updated during // calls to Next() and change throughout the lifecycle of @@ -1930,7 +1928,7 @@ func (s *showCreateAllTablesGenerator) Start(ctx context.Context, txn *kv.Txn) e // We also account for the memory in the BoundAccount memory monitor in // showCreateAllTablesGenerator. ids, err := getTopologicallySortedTableIDs( - ctx, s.evalPlanner, txn, s.dbName, s.user, &s.acc, + ctx, s.evalPlanner, txn, s.dbName, s.sessionData, &s.acc, ) if err != nil { return err @@ -1956,7 +1954,7 @@ func (s *showCreateAllTablesGenerator) Next(ctx context.Context) (bool, error) { } createStmt, err := getCreateStatement( - ctx, s.evalPlanner, s.txn, s.ids[s.idx], s.dbName, s.user, + ctx, s.evalPlanner, s.txn, s.ids[s.idx], s.dbName, s.sessionData, ) if err != nil { return false, err @@ -2002,7 +2000,7 @@ func (s *showCreateAllTablesGenerator) Next(ctx context.Context) (bool, error) { statementReturnType = alterValidateFKStatements } alterStmt, err := getAlterStatements( - ctx, s.evalPlanner, s.txn, s.ids[s.idx], s.dbName, s.user, statementReturnType, + ctx, s.evalPlanner, s.txn, s.ids[s.idx], s.dbName, s.sessionData, statementReturnType, ) if err != nil { return false, err @@ -2043,6 +2041,6 @@ func makeShowCreateAllTablesGenerator( evalPlanner: ctx.Planner, dbName: dbName, acc: ctx.Mon.MakeBoundAccount(), - user: ctx.SessionData().User(), + sessionData: ctx.SessionData(), }, nil } diff --git a/pkg/sql/sem/builtins/pg_builtins.go b/pkg/sql/sem/builtins/pg_builtins.go index cfd07c61493d..cf3be7adca69 100644 --- a/pkg/sql/sem/builtins/pg_builtins.go +++ b/pkg/sql/sem/builtins/pg_builtins.go @@ -26,7 +26,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/privilege" "github.com/cockroachdb/cockroach/pkg/sql/rowenc" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" - "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" "github.com/cockroachdb/cockroach/pkg/sql/sqlerrors" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/encoding" @@ -229,10 +228,7 @@ func makePGGetIndexDef(argTypes tree.ArgTypes) tree.Overload { r, err := ctx.Planner.QueryRowEx( ctx.Ctx(), "pg_get_indexdef", ctx.Txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - Database: ctx.SessionData().Database, - }, + ctx.SessionData(), "SELECT indexdef FROM pg_catalog.pg_indexes WHERE crdb_oid = $1", args[0]) if err != nil { return nil, err @@ -249,10 +245,7 @@ func makePGGetIndexDef(argTypes tree.ArgTypes) tree.Overload { r, err = ctx.Planner.QueryRowEx( ctx.Ctx(), "pg_get_indexdef", ctx.Txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - Database: ctx.SessionData().Database, - }, + ctx.SessionData(), `SELECT ischema.column_name as pg_get_indexdef FROM information_schema.statistics AS ischema INNER JOIN pg_catalog.pg_indexes AS pgindex @@ -287,10 +280,7 @@ func makePGGetViewDef(argTypes tree.ArgTypes) tree.Overload { r, err := ctx.Planner.QueryRowEx( ctx.Ctx(), "pg_get_viewdef", ctx.Txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - Database: ctx.SessionData().Database, - }, + ctx.SessionData(), "SELECT definition FROM pg_catalog.pg_views v JOIN pg_catalog.pg_class c ON "+ "c.relname=v.viewname WHERE oid=$1", args[0]) if err != nil { @@ -315,10 +305,7 @@ func makePGGetConstraintDef(argTypes tree.ArgTypes) tree.Overload { r, err := ctx.Planner.QueryRowEx( ctx.Ctx(), "pg_get_constraintdef", ctx.Txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - Database: ctx.SessionData().Database, - }, + ctx.SessionData(), "SELECT condef FROM pg_catalog.pg_constraint WHERE oid=$1", args[0]) if err != nil { return nil, err @@ -457,11 +444,7 @@ func getNameForArg(ctx *tree.EvalContext, arg tree.Datum, pgTable, pgCol string) return "", errors.AssertionFailedf("unexpected arg type %T", t) } r, err := ctx.Planner.QueryRowEx(ctx.Ctx(), "get-name-for-arg", - ctx.Txn, sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - Database: ctx.SessionData().Database, - StubCatalogTables: ctx.SessionData().StubCatalogTablesEnabled, - }, query, arg) + ctx.Txn, ctx.SessionData(), query, arg) if err != nil || r == nil { return "", err } @@ -491,10 +474,7 @@ func getTableNameForArg(ctx *tree.EvalContext, arg tree.Datum) (*tree.TableName, case *tree.DOid: r, err := ctx.Planner.QueryRowEx(ctx.Ctx(), "get-table-name-for-arg", ctx.Txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - Database: ctx.SessionData().Database, - }, + ctx.SessionData(), `SELECT n.nspname, c.relname FROM pg_class c JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace WHERE c.oid = $1`, t) @@ -664,10 +644,7 @@ func evalPrivilegeCheck( privilege.ALL, priv, schema, infoTable, pred) r, err := ctx.Planner.QueryRowEx( ctx.Ctx(), "eval-privilege-check", ctx.Txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - Database: ctx.SessionData().Database, - }, + ctx.SessionData(), query, allRoles, ) if err != nil { @@ -816,10 +793,7 @@ var pgBuiltins = map[string]builtinDefinition{ t, err := ctx.Planner.QueryRowEx( ctx.Ctx(), "pg_get_function_result", ctx.Txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - Database: ctx.SessionData().Database, - }, + ctx.SessionData(), `SELECT prorettype::REGTYPE::TEXT FROM pg_proc WHERE oid=$1`, int(funcOid.DInt)) if err != nil { return nil, err @@ -847,10 +821,8 @@ var pgBuiltins = map[string]builtinDefinition{ t, err := ctx.Planner.QueryRowEx( ctx.Ctx(), "pg_get_function_identity_arguments", ctx.Txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - Database: ctx.SessionData().Database, - }, `SELECT array_agg(unnest(proargtypes)::REGTYPE::TEXT) FROM pg_proc WHERE oid=$1`, int(funcOid.DInt)) + ctx.SessionData(), + `SELECT array_agg(unnest(proargtypes)::REGTYPE::TEXT) FROM pg_proc WHERE oid=$1`, int(funcOid.DInt)) if err != nil { return nil, err } @@ -1040,10 +1012,8 @@ var pgBuiltins = map[string]builtinDefinition{ t, err := ctx.Planner.QueryRowEx( ctx.Ctx(), "pg_get_userbyid", ctx.Txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - Database: ctx.SessionData().Database, - }, "SELECT rolname FROM pg_catalog.pg_roles WHERE oid=$1", oid) + ctx.SessionData(), + "SELECT rolname FROM pg_catalog.pg_roles WHERE oid=$1", oid) if err != nil { return nil, err } @@ -1071,10 +1041,8 @@ var pgBuiltins = map[string]builtinDefinition{ r, err := ctx.Planner.QueryRowEx( ctx.Ctx(), "pg_sequence_parameters", ctx.Txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - Database: ctx.SessionData().Database, - }, `SELECT seqstart, seqmin, seqmax, seqincrement, seqcycle, seqcache, seqtypid `+ + ctx.SessionData(), + `SELECT seqstart, seqmin, seqmax, seqincrement, seqcycle, seqcache, seqtypid `+ `FROM pg_catalog.pg_sequence WHERE seqrelid=$1`, args[0]) if err != nil { return nil, err @@ -1159,12 +1127,8 @@ var pgBuiltins = map[string]builtinDefinition{ // on pg_description and let predicate push-down do its job. r, err := ctx.Planner.QueryRowEx( ctx.Ctx(), "pg_get_coldesc", - ctx.Txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - Database: ctx.SessionData().Database, - }, + ctx.SessionData(), ` SELECT COALESCE(c.comment, pc.comment) FROM system.comments c FULL OUTER JOIN crdb_internal.predefined_comments pc @@ -1236,10 +1200,8 @@ WHERE c.type=$1::int AND c.object_id=$2::int AND c.sub_id=$3::int LIMIT 1 r, err := ctx.Planner.QueryRowEx( ctx.Ctx(), "pg_get_shobjdesc", ctx.Txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - Database: ctx.SessionData().Database, - }, fmt.Sprintf(` + ctx.SessionData(), + fmt.Sprintf(` SELECT description FROM pg_catalog.pg_shdescription WHERE objoid = %[1]d @@ -1313,10 +1275,7 @@ SELECT description t, err := ctx.Planner.QueryRowEx( ctx.Ctx(), "pg_function_is_visible", ctx.Txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - Database: ctx.SessionData().Database, - }, + ctx.SessionData(), "SELECT * from pg_proc WHERE oid=$1 LIMIT 1", int(oid.DInt)) if err != nil { return nil, err @@ -1827,10 +1786,7 @@ SELECT description if r, err := ctx.Planner.QueryRowEx( ctx.Ctx(), "has-sequence-privilege", ctx.Txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - Database: ctx.SessionData().Database, - }, + ctx.SessionData(), `SELECT sequence_name FROM information_schema.sequences `+ `WHERE sequence_catalog = $1 AND sequence_schema = $2 AND sequence_name = $3`, tn.CatalogName, tn.SchemaName, tn.ObjectName); err != nil { @@ -2311,10 +2267,7 @@ SELECT description r, err := ctx.Planner.QueryRowEx( ctx.Ctx(), "information_schema._pg_index_position", ctx.Txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - Database: ctx.SessionData().Database, - }, + ctx.SessionData(), `SELECT (ss.a).n FROM (SELECT information_schema._pg_expandarray(indkey) AS a FROM pg_catalog.pg_index WHERE indexrelid = $1) ss @@ -2479,10 +2432,7 @@ func getPgObjDesc(ctx *tree.EvalContext, catalogName string, oid int) (tree.Datu } r, err := ctx.Planner.QueryRowEx( ctx.Ctx(), "pg_get_objdesc", ctx.Txn, - sessiondata.InternalExecutorOverride{ - User: security.RootUserName(), - Database: ctx.SessionData().Database, - }, + ctx.SessionData(), fmt.Sprintf(` SELECT description FROM pg_catalog.pg_description diff --git a/pkg/sql/sem/builtins/show_create_all_tables_builtin.go b/pkg/sql/sem/builtins/show_create_all_tables_builtin.go index 815fc06fb911..5bbc68cc9619 100644 --- a/pkg/sql/sem/builtins/show_create_all_tables_builtin.go +++ b/pkg/sql/sem/builtins/show_create_all_tables_builtin.go @@ -17,7 +17,6 @@ import ( "unsafe" "github.com/cockroachdb/cockroach/pkg/kv" - "github.com/cockroachdb/cockroach/pkg/security" "github.com/cockroachdb/cockroach/pkg/sql/memsize" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" @@ -53,10 +52,10 @@ func getTopologicallySortedTableIDs( evalPlanner tree.EvalPlanner, txn *kv.Txn, dbName string, - user security.SQLUsername, + sessionData *sessiondata.SessionData, acc *mon.BoundAccount, ) ([]int64, error) { - ids, err := getTableIDs(ctx, evalPlanner, txn, dbName, user, acc) + ids, err := getTableIDs(ctx, evalPlanner, txn, dbName, sessionData, acc) if err != nil { return nil, err } @@ -80,10 +79,8 @@ func getTopologicallySortedTableIDs( ctx, "crdb_internal.show_create_all_tables", txn, - sessiondata.InternalExecutorOverride{ - User: user, - Database: dbName, - }, query, + sessionData, + query, tid, ) if err != nil { @@ -160,7 +157,7 @@ func getTableIDs( evalPlanner tree.EvalPlanner, txn *kv.Txn, dbName string, - user security.SQLUsername, + sessionData *sessiondata.SessionData, acc *mon.BoundAccount, ) ([]int64, error) { query := fmt.Sprintf(` @@ -174,10 +171,7 @@ func getTableIDs( ctx, "crdb_internal.show_create_all_tables", txn, - sessiondata.InternalExecutorOverride{ - User: user, - Database: dbName, - }, + sessionData, query, dbName, ) @@ -254,7 +248,7 @@ func getCreateStatement( txn *kv.Txn, id int64, dbName string, - user security.SQLUsername, + sessionData *sessiondata.SessionData, ) (tree.Datum, error) { query := fmt.Sprintf(` SELECT @@ -266,10 +260,7 @@ func getCreateStatement( ctx, "crdb_internal.show_create_all_tables", txn, - sessiondata.InternalExecutorOverride{ - User: user, - Database: dbName, - }, + sessionData, query, id, ) @@ -288,7 +279,7 @@ func getAlterStatements( txn *kv.Txn, id int64, dbName string, - user security.SQLUsername, + sessionData *sessiondata.SessionData, statementType string, ) (tree.Datum, error) { query := fmt.Sprintf(` @@ -301,10 +292,7 @@ func getAlterStatements( ctx, "crdb_internal.show_create_all_tables", txn, - sessiondata.InternalExecutorOverride{ - User: user, - Database: dbName, - }, + sessionData, query, id, ) diff --git a/pkg/sql/sem/tree/eval.go b/pkg/sql/sem/tree/eval.go index 74b12ab1e8d4..af3366eba06e 100644 --- a/pkg/sql/sem/tree/eval.go +++ b/pkg/sql/sem/tree/eval.go @@ -3229,7 +3229,7 @@ type EvalPlanner interface { ctx context.Context, opName string, txn *kv.Txn, - session sessiondata.InternalExecutorOverride, + session *sessiondata.SessionData, stmt string, qargs ...interface{}) (Datums, error) @@ -3237,7 +3237,7 @@ type EvalPlanner interface { ctx context.Context, opName string, txn *kv.Txn, - session sessiondata.InternalExecutorOverride, + session *sessiondata.SessionData, stmt string, qargs ...interface{}, ) (InternalRows, error) diff --git a/pkg/sql/sessiondata/internal.go b/pkg/sql/sessiondata/internal.go index b1f785e8f111..e198b1805036 100644 --- a/pkg/sql/sessiondata/internal.go +++ b/pkg/sql/sessiondata/internal.go @@ -26,11 +26,6 @@ type InternalExecutorOverride struct { // DatabaseIDToTempSchemaID represents the mapping for temp schemas used which // allows temporary schema resolution by ID. DatabaseIDToTempSchemaID map[uint32]uint32 - // StubCatalogTables overrides the StubCatalogTables session variable on - // the InternalExecutor. - StubCatalogTables bool - // SessionData overrides all of the InternalExecutor's SessionData. - SessionData *SessionData } // NodeUserSessionDataOverride is an InternalExecutorOverride which overrides