Skip to content

Commit

Permalink
sql: do not require system table privileges to use SHOW STATISTICS
Browse files Browse the repository at this point in the history
See the docs at: https://www.cockroachlabs.com/docs/stable/show-statistics
They say that no special privileges are needed.

Release note (bug fix): The SHOW STATISTICS command previously incorrectly
required the user to have the admin role. It was intended to only
require the user to have any privilege on the table being inspected.
This is now fixed.
  • Loading branch information
rafiss committed Nov 9, 2023
1 parent 6430899 commit 14f4ef9
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 6 deletions.
11 changes: 6 additions & 5 deletions pkg/sql/explain_bundle_test.go
Expand Up @@ -357,16 +357,17 @@ CREATE TABLE users(id UUID DEFAULT gen_random_uuid() PRIMARY KEY, promo_id INT R
defer r.Exec(t, "SET ROLE root")
r.Exec(t, "CREATE TABLE permissions (k PRIMARY KEY) AS SELECT 1")
rows := r.QueryStr(t, "EXPLAIN ANALYZE (DEBUG) SELECT * FROM permissions")
// Check that we see two errors about missing privileges as warnings
// (one for the cluster settings and another for the table statistics).
// Check that we see an error about missing privileges for the cluster
// settings as a warnings. (Since `test` is the table owner, it already
// has permissions on the table itself.)
var numErrors int
for _, row := range rows {
if strings.HasPrefix(row[0], "-- error") {
if strings.HasPrefix(row[0], "-- error getting cluster settings:") {
numErrors++
}
}
if numErrors != 2 {
t.Fatalf("didn't see 2 errors in %v", rows)
if numErrors != 1 {
t.Fatalf("didn't see 1 error in %v", rows)
}
checkBundle(
t, fmt.Sprint(rows), "permission" /* tableName */, nil /* contentCheck */, true, /* expectErrors */
Expand Down
30 changes: 30 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/distsql_stats
Expand Up @@ -2461,3 +2461,33 @@ SELECT statistics_name, created FROM
[SHOW STATISTICS FOR TABLE t107651]
----
__auto__ 2023-09-15 17:00:00 -0400 -0400

# Test that a non-admin can use SHOW STATISTICS.

statement ok
CREATE TABLE tab_test_privileges (a INT PRIMARY KEY);

statement ok
INSERT INTO tab_test_privileges VALUES (1);

statement ok
CREATE STATISTICS tab_test_privileges_stat ON a FROM tab_test_privileges;

user testuser

query error testuser has no privileges on relation tab_test_privileges
SELECT statistics_name, created FROM [SHOW STATISTICS FOR TABLE tab_test_privileges]

user root

statement ok
GRANT SELECT ON tab_test_privileges TO testuser

user testuser

query T
SELECT statistics_name FROM [SHOW STATISTICS FOR TABLE tab_test_privileges]
----
tab_test_privileges_stat

user root
9 changes: 8 additions & 1 deletion pkg/sql/show_stats.go
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/exprutil"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/sqlerrors"
"github.com/cockroachdb/cockroach/pkg/sql/stats"
"github.com/cockroachdb/cockroach/pkg/sql/types"
Expand Down Expand Up @@ -142,10 +143,16 @@ func (p *planner) ShowTableStats(ctx context.Context, n *tree.ShowTableStats) (p
FROM system.table_statistics
WHERE "tableID" = $1
ORDER BY "createdAt", "columnIDs", "statisticID"`, partialPredicateCol, fullStatisticIDCol)
rows, err := p.InternalSQLTxn().QueryBuffered(

// There is a privilege check above to make sure the user has any
// privilege on the table being inspected. We use the node user to execute
// the query against the system table so that SHOW STATISTICS does not
// _also_ need privileges on the system.table_statistics table.
rows, err := p.InternalSQLTxn().QueryBufferedEx(
ctx,
"read-table-stats",
p.txn,
sessiondata.NodeUserSessionDataOverride,
stmt,
desc.GetID(),
)
Expand Down

0 comments on commit 14f4ef9

Please sign in to comment.