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.

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: add a virtual index on database name for SHOW GRANTS ON DATABASE #90712

Merged
merged 1 commit into from
Oct 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -317,4 +317,3 @@ select
│ └── [/'foo'/e'bar\x00'/5 - ]
└── filters
└── c = 5

92 changes: 66 additions & 26 deletions pkg/sql/crdb_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -5023,38 +5023,78 @@ CREATE TABLE crdb_internal.cluster_database_privileges (
database_name STRING NOT NULL,
grantee STRING NOT NULL,
privilege_type STRING NOT NULL,
is_grantable STRING
is_grantable STRING,
INDEX(database_name)
)`,
populate: func(ctx context.Context, p *planner, dbContext catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error {
return forEachDatabaseDesc(ctx, p, dbContext, true, /* requiresPrivileges */
func(db catalog.DatabaseDescriptor) error {
privs := db.GetPrivileges().Show(privilege.Database, true /* showImplicitOwnerPrivs */)
dbNameStr := tree.NewDString(db.GetName())
// TODO(knz): This should filter for the current user, see
// https://github.com/cockroachdb/cockroach/issues/35572
for _, u := range privs {
userNameStr := tree.NewDString(u.User.Normalized())
for _, priv := range u.Privileges {
// We use this function to check for the grant option so that the
// object owner also gets is_grantable=true.
grantOptionErr := p.CheckGrantOptionsForUser(
ctx, db.GetPrivileges(), db, []privilege.Kind{priv.Kind}, u.User, true, /* isGrant */
)
if err := addRow(
dbNameStr, // database_name
userNameStr, // grantee
tree.NewDString(priv.Kind.String()), // privilege_type
yesOrNoDatum(grantOptionErr == nil), // is_grantable
); err != nil {
return err
}
}
}
return nil
})
makeClusterDatabasePrivilegesFromDescriptor(ctx, p, addRow))
},
indexes: []virtualIndex{
{populate: func(ctx context.Context, unwrappedConstraint tree.Datum, p *planner, dbContext catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) (matched bool, err error) {
if unwrappedConstraint == tree.DNull {
return false, nil
}
dbName := string(tree.MustBeDString(unwrappedConstraint))
if dbContext != nil && dbContext.GetName() != dbName {
return false, nil
}

flags := tree.CommonLookupFlags{
AvoidLeased: true,
}
dbDesc, err := p.Descriptors().GetImmutableDatabaseByName(ctx, p.Txn(), dbName, flags)
if err != nil || dbDesc == nil {
return false, err
}
hasPriv, err := userCanSeeDescriptor(ctx, p, dbDesc, nil /* parentDBDesc */, false /* allowAdding */)
if err != nil || !hasPriv {
return false, err
}
var called bool
if err := makeClusterDatabasePrivilegesFromDescriptor(
ctx, p, func(datum ...tree.Datum) error {
called = true
return addRow(datum...)
},
)(dbDesc); err != nil {
return false, err
}
return called, nil
}},
},
}

func makeClusterDatabasePrivilegesFromDescriptor(
ctx context.Context, p *planner, addRow func(...tree.Datum) error,
) func(catalog.DatabaseDescriptor) error {
return func(db catalog.DatabaseDescriptor) error {
privs := db.GetPrivileges().Show(privilege.Database, true /* showImplicitOwnerPrivs */)
dbNameStr := tree.NewDString(db.GetName())
// TODO(knz): This should filter for the current user, see
// https://github.com/cockroachdb/cockroach/issues/35572
for _, u := range privs {
userNameStr := tree.NewDString(u.User.Normalized())
for _, priv := range u.Privileges {
// We use this function to check for the grant option so that the
// object owner also gets is_grantable=true.
grantOptionErr := p.CheckGrantOptionsForUser(
ctx, db.GetPrivileges(), db, []privilege.Kind{priv.Kind}, u.User, true, /* isGrant */
)
if err := addRow(
dbNameStr, // database_name
userNameStr, // grantee
tree.NewDString(priv.Kind.String()), // privilege_type
yesOrNoDatum(grantOptionErr == nil), // is_grantable
); err != nil {
return err
}
}
}
return nil
}
}

var crdbInternalCrossDbReferences = virtualSchemaTable{
comment: `virtual table with cross db references`,
schema: `
Expand Down
6 changes: 4 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/create_statements
Original file line number Diff line number Diff line change
Expand Up @@ -227,12 +227,14 @@ CREATE TABLE crdb_internal.cluster_database_privileges (
database_name STRING NOT NULL,
grantee STRING NOT NULL,
privilege_type STRING NOT NULL,
is_grantable STRING NULL
is_grantable STRING NULL,
INDEX cluster_database_privileges_database_name_idx (database_name ASC) STORING (grantee, privilege_type, is_grantable)
) CREATE TABLE crdb_internal.cluster_database_privileges (
database_name STRING NOT NULL,
grantee STRING NOT NULL,
privilege_type STRING NOT NULL,
is_grantable STRING NULL
is_grantable STRING NULL,
INDEX cluster_database_privileges_database_name_idx (database_name ASC) STORING (grantee, privilege_type, is_grantable)
) {} {}
CREATE TABLE crdb_internal.cluster_distsql_flows (
flow_id UUID NOT NULL,
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/drop_owned_by
Original file line number Diff line number Diff line change
Expand Up @@ -622,9 +622,9 @@ DROP OWNED BY testuser
query TTTB
SHOW GRANTS ON DATABASE d3
----
d3 admin ALL true
d3 public CONNECT false
d3 root ALL true
d3 admin ALL true
d3 public CONNECT false
d3 root ALL true

# Drop owned by should not work if the user has synthetic privileges.

Expand Down
20 changes: 20 additions & 0 deletions pkg/sql/opt/exec/execbuilder/testdata/virtual
Original file line number Diff line number Diff line change
Expand Up @@ -193,3 +193,23 @@ vectorized: true
table: pg_class@pg_class_oid_idx
spans: [/50 - /50]
limit: 1

# Test that the virtual index is used when looking up database grants

statement ok
CREATE DATABASE t

query T
EXPLAIN SHOW GRANTS ON DATABASE t
----
distribution: local
vectorized: true
·
• sort
│ order: +grantee,+privilege_type
└── • render
└── • virtual table
table: cluster_database_privileges@cluster_database_privileges_database_name_idx
spans: [/'t' - /'t']