From e2bff4d121260351c95e2d7fded9adcf7e636e70 Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Wed, 21 Jun 2023 17:38:09 -0700 Subject: [PATCH] sql: fix CTAS when selecting from some internal virtual tables 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. --- pkg/sql/logictest/testdata/logic_test/crdb_internal | 7 +++++++ pkg/sql/planner.go | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/pkg/sql/logictest/testdata/logic_test/crdb_internal b/pkg/sql/logictest/testdata/logic_test/crdb_internal index 1b8ff2daec05..d3468e5e9818 100644 --- a/pkg/sql/logictest/testdata/logic_test/crdb_internal +++ b/pkg/sql/logictest/testdata/logic_test/crdb_internal @@ -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; diff --git a/pkg/sql/planner.go b/pkg/sql/planner.go index 9632faa6ebc5..4e2bb63520b4 100644 --- a/pkg/sql/planner.go +++ b/pkg/sql/planner.go @@ -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 @@ -518,6 +520,7 @@ func internalExtendedEvalCtx( sqlStatsController = &persistedsqlstats.Controller{} schemaTelemetryController = &schematelemetrycontroller.Controller{} indexUsageStatsController = &idxusage.Controller{} + sqlStatsProvider = &persistedsqlstats.PersistedSQLStats{} } } ret := extendedEvalContext{ @@ -540,6 +543,7 @@ func internalExtendedEvalCtx( Tracing: &SessionTracing{}, Descs: tables, indexUsageStats: indexUsageStats, + statsProvider: sqlStatsProvider, } ret.SetDeprecatedContext(ctx) ret.copyFromExecCfg(execCfg)