Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

release-22.2: sql: fix CTAS when selecting from some internal virtual tables #105359

Merged
merged 1 commit into from
Jun 22, 2023
Merged

release-22.2: sql: fix CTAS when selecting from some internal virtual tables #105359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 3 additions & 28 deletions pkg/sql/crdb_internal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness"
"github.com/cockroachdb/cockroach/pkg/sql/sqlstats"
"github.com/cockroachdb/cockroach/pkg/sql/sqlstats/persistedsqlstats"
"github.com/cockroachdb/cockroach/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil"
"github.com/cockroachdb/cockroach/pkg/sql/sqlstats/sslocal"
"github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege"
Expand Down Expand Up @@ -1137,18 +1136,6 @@ func execStatVar(count int64, n roachpb.NumericStat) tree.Datum {
return tree.NewDFloat(tree.DFloat(n.GetVariance(count)))
}

// getSQLStats retrieves a sqlStats provider from the planner or
// returns an error if not available. virtualTableName specifies the virtual
// table for which this sqlStats object is needed.
func getSQLStats(
p *planner, virtualTableName string,
) (*persistedsqlstats.PersistedSQLStats, error) {
if p.extendedEvalCtx.statsProvider == nil {
return nil, errors.Newf("%s cannot be used in this context", virtualTableName)
}
return p.extendedEvalCtx.statsProvider, nil
}

// legacyAnonymizedStmt is a placeholder value for the
// crdb_internal.node_statement_statistics(anonymized) column. The column used
// to contain the SQL statement scrubbed of all identifiers. At the time
Expand Down Expand Up @@ -1219,11 +1206,7 @@ CREATE TABLE crdb_internal.node_statement_statistics (
return noViewActivityOrViewActivityRedactedRoleError(p.User())
}

sqlStats, err := getSQLStats(p, "crdb_internal.node_statement_statistics")
if err != nil {
return err
}

sqlStats := p.extendedEvalCtx.statsProvider
nodeID, _ := p.execCfg.NodeInfo.NodeID.OptionalNodeID() // zero if not available

statementVisitor := func(_ context.Context, stats *roachpb.CollectedStatementStatistics) error {
Expand Down Expand Up @@ -1366,11 +1349,7 @@ CREATE TABLE crdb_internal.node_transaction_statistics (
return noViewActivityOrViewActivityRedactedRoleError(p.User())
}

sqlStats, err := getSQLStats(p, "crdb_internal.node_transaction_statistics")
if err != nil {
return err
}

sqlStats := p.extendedEvalCtx.statsProvider
nodeID, _ := p.execCfg.NodeInfo.NodeID.OptionalNodeID() // zero if not available

transactionVisitor := func(_ context.Context, stats *roachpb.CollectedTransactionStatistics) error {
Expand Down Expand Up @@ -1440,11 +1419,7 @@ CREATE TABLE crdb_internal.node_txn_stats (
return err
}

sqlStats, err := getSQLStats(p, "crdb_internal.node_txn_stats")
if err != nil {
return err
}

sqlStats := p.extendedEvalCtx.statsProvider
nodeID, _ := p.execCfg.NodeInfo.NodeID.OptionalNodeID() // zero if not available

appTxnStatsVisitor := func(appName string, stats *roachpb.TxnStats) error {
Expand Down
7 changes: 7 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/crdb_internal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1366,3 +1366,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;
15 changes: 6 additions & 9 deletions pkg/sql/logictest/testdata/logic_test/create_table
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,17 +463,14 @@ 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);
statement ok
CREATE TABLE ctas1 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);
statement ok
CREATE TABLE ctas2 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);
statement ok
CREATE TABLE ctas3 AS (SELECT * FROM crdb_internal.node_txn_stats);

subtest generated_as_identity
statement ok
Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/planner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,12 +472,14 @@ func internalExtendedEvalCtx(
var sqlStatsController eval.SQLStatsController
var schemaTelemetryController eval.SchemaTelemetryController
var indexUsageStatsController eval.IndexUsageStatsController
var sqlStatsProvider *persistedsqlstats.PersistedSQLStats
if execCfg.InternalExecutor != nil {
if execCfg.InternalExecutor.s != nil {
indexUsageStats = execCfg.InternalExecutor.s.indexUsageStats
sqlStatsController = execCfg.InternalExecutor.s.sqlStatsController
schemaTelemetryController = execCfg.InternalExecutor.s.schemaTelemetryController
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 All @@ -488,6 +490,7 @@ func internalExtendedEvalCtx(
sqlStatsController = &persistedsqlstats.Controller{}
schemaTelemetryController = &schematelemetrycontroller.Controller{}
indexUsageStatsController = &idxusage.Controller{}
sqlStatsProvider = &persistedsqlstats.PersistedSQLStats{}
}
}
ret := extendedEvalContext{
Expand All @@ -512,6 +515,7 @@ func internalExtendedEvalCtx(
Tracing: &SessionTracing{},
Descs: tables,
indexUsageStats: indexUsageStats,
statsProvider: sqlStatsProvider,
}
ret.copyFromExecCfg(execCfg)
return ret
Expand Down