Skip to content

Commit

Permalink
sql: fix null pointer error in cluster_statement_statistics virtual t…
Browse files Browse the repository at this point in the history
…able

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 cockroachdb#76710

Release note (bug fix): creating materialized view on
crdb_internal.cluster_statement_statistics no longer cause panic.
  • Loading branch information
Azhng committed Feb 17, 2022
1 parent 4996177 commit 566ac26
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 12 deletions.
12 changes: 0 additions & 12 deletions pkg/sql/logictest/testdata/logic_test/create_table
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -467,6 +469,7 @@ func internalExtendedEvalCtx(
},
Tracing: &SessionTracing{},
Descs: tables,
statsProvider: sqlStatsProvider,
indexUsageStats: indexUsageStats,
}
ret.copyFromExecCfg(execCfg)
Expand Down
17 changes: 17 additions & 0 deletions pkg/sql/sqlstats/persistedsqlstats/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 566ac26

Please sign in to comment.