Skip to content

Commit

Permalink
sql: fix CTAS when selecting from some internal virtual tables
Browse files Browse the repository at this point in the history
This commit fixes an oversight where we forgot to set the SQL stats
provider in some code paths (namely when `NewInternalPlanner` is called,
one example is backfilling query into table that powers CREATE TABLE AS
and CREATE MATERIALIZED VIEW AS features), and this is now fixed.

Release note (bug fix): Previously, CockroachDB would crash when
evaluating CREATE TABLE .. AS or CREATE MATERIALIZED VIEW .. AS
statements when AS clause selected data from
`crdb_internal.cluster_statement_statistics` or
`crdb_internal.cluster_transaction_statistics` virtual tables. The bug
has been present since at least 22.1 and is now fixed.
  • Loading branch information
yuzefovich committed Jun 22, 2023
1 parent b539561 commit e2bff4d
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 0 deletions.
7 changes: 7 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/crdb_internal
Original file line number Diff line number Diff line change
Expand Up @@ -1424,3 +1424,10 @@ user root

statement ok
REVOKE SYSTEM VIEWACTIVITY FROM testuser

# Regression tests for not setting SQL stats provider in the backfill (#76710).
statement ok
CREATE TABLE t76710_1 AS SELECT * FROM crdb_internal.statement_statistics;

statement ok
CREATE MATERIALIZED VIEW t76710_2 AS SELECT fingerprint_id FROM crdb_internal.cluster_statement_statistics;
4 changes: 4 additions & 0 deletions pkg/sql/planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,12 +502,14 @@ func internalExtendedEvalCtx(
var sqlStatsController eval.SQLStatsController
var schemaTelemetryController eval.SchemaTelemetryController
var indexUsageStatsController eval.IndexUsageStatsController
var sqlStatsProvider *persistedsqlstats.PersistedSQLStats
if ief := execCfg.InternalDB; ief != nil {
if ief.server != nil {
indexUsageStats = ief.server.indexUsageStats
sqlStatsController = ief.server.sqlStatsController
schemaTelemetryController = ief.server.schemaTelemetryController
indexUsageStatsController = ief.server.indexUsageStatsController
sqlStatsProvider = ief.server.sqlStats
} else {
// If the indexUsageStats is nil from the sql.Server, we create a dummy
// index usage stats collector. The sql.Server in the ExecutorConfig
Expand All @@ -518,6 +520,7 @@ func internalExtendedEvalCtx(
sqlStatsController = &persistedsqlstats.Controller{}
schemaTelemetryController = &schematelemetrycontroller.Controller{}
indexUsageStatsController = &idxusage.Controller{}
sqlStatsProvider = &persistedsqlstats.PersistedSQLStats{}
}
}
ret := extendedEvalContext{
Expand All @@ -540,6 +543,7 @@ func internalExtendedEvalCtx(
Tracing: &SessionTracing{},
Descs: tables,
indexUsageStats: indexUsageStats,
statsProvider: sqlStatsProvider,
}
ret.SetDeprecatedContext(ctx)
ret.copyFromExecCfg(execCfg)
Expand Down

0 comments on commit e2bff4d

Please sign in to comment.