From 566ac261dd96d4aa3c5fe80b3ff5977c319d3d2d Mon Sep 17 00:00:00 2001 From: Azhng Date: Wed, 16 Feb 2022 22:39:05 +0000 Subject: [PATCH] sql: fix null pointer error in cluster_statement_statistics virtual table Previously, a null pointer error in crdb_internal.cluster_statement_statistics can cause a runtime panic when it is read by internal executor. This commit fixes the null pointer error by properly propagating state in internalExtendedEvalCtx(). Partially resolves #76710 Release note (bug fix): creating materialized view on crdb_internal.cluster_statement_statistics no longer cause panic. --- .../logictest/testdata/logic_test/create_table | 12 ------------ pkg/sql/planner.go | 3 +++ .../sqlstats/persistedsqlstats/reader_test.go | 17 +++++++++++++++++ 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/create_table b/pkg/sql/logictest/testdata/logic_test/create_table index 0b54c4b56a52..0283c45705e8 100644 --- a/pkg/sql/logictest/testdata/logic_test/create_table +++ b/pkg/sql/logictest/testdata/logic_test/create_table @@ -458,18 +458,6 @@ CREATE TABLE error (a INT, b INT, INDEX idx (a), UNIQUE INDEX idx (b)) statement error pgcode 42P07 duplicate index name: \"idx\" CREATE TABLE error (a INT, b INT, UNIQUE INDEX idx (a), UNIQUE INDEX idx (b)) -# Regression test for using some virtual tables in CREATE TABLE AS which is not -# supported at the moment (#65512). - -query error crdb_internal.node_statement_statistics cannot be used in this context -CREATE TABLE ctas AS (SELECT * FROM crdb_internal.node_statement_statistics); - -query error crdb_internal.node_transaction_statistics cannot be used in this context -CREATE TABLE ctas AS (SELECT * FROM crdb_internal.node_transaction_statistics); - -query error crdb_internal.node_txn_stats cannot be used in this context -CREATE TABLE ctas AS (SELECT * FROM crdb_internal.node_txn_stats); - subtest generated_as_identity statement ok CREATE TABLE generated_always_t ( diff --git a/pkg/sql/planner.go b/pkg/sql/planner.go index 7b48e264aecf..79e6fc857dc5 100644 --- a/pkg/sql/planner.go +++ b/pkg/sql/planner.go @@ -435,11 +435,13 @@ func internalExtendedEvalCtx( var indexUsageStats *idxusage.LocalIndexUsageStats var sqlStatsController tree.SQLStatsController var indexUsageStatsController tree.IndexUsageStatsController + var sqlStatsProvider *persistedsqlstats.PersistedSQLStats if execCfg.InternalExecutor != nil { if execCfg.InternalExecutor.s != nil { indexUsageStats = execCfg.InternalExecutor.s.indexUsageStats sqlStatsController = execCfg.InternalExecutor.s.sqlStatsController indexUsageStatsController = execCfg.InternalExecutor.s.indexUsageStatsController + sqlStatsProvider = execCfg.InternalExecutor.s.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 @@ -467,6 +469,7 @@ func internalExtendedEvalCtx( }, Tracing: &SessionTracing{}, Descs: tables, + statsProvider: sqlStatsProvider, indexUsageStats: indexUsageStats, } ret.copyFromExecCfg(execCfg) diff --git a/pkg/sql/sqlstats/persistedsqlstats/reader_test.go b/pkg/sql/sqlstats/persistedsqlstats/reader_test.go index 1580d542c5c6..4fb877808531 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/reader_test.go +++ b/pkg/sql/sqlstats/persistedsqlstats/reader_test.go @@ -143,6 +143,23 @@ func TestPersistedSQLStatsRead(t *testing.T) { }) } +// https://github.com/cockroachdb/cockroach/issues/76710 +func TestInternalExecutorReadVirtualTable(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + s, conn, _ := serverutils.StartServer(t, base.TestServerArgs{}) + ctx := context.Background() + defer s.Stopper().Stop(ctx) + + sqlConn := sqlutils.MakeSQLRunner(conn) + + sqlConn.Exec(t, ` + CREATE MATERIALIZED VIEW temp + AS SELECT fingerprint_id FROM crdb_internal.cluster_statement_statistics +`) +} + func verifyStoredStmtFingerprints( t *testing.T, expectedStmtFingerprints map[string]int64,