Skip to content

Commit

Permalink
sql, sessioninit: lookup tables by ID, not name
Browse files Browse the repository at this point in the history 
These tables all have fixed IDs, so looking them up by name is not
necessary.

Release note: None
  • Loading branch information
@rafiss
rafiss committed Dec 3, 2024
1 parent a63a689 commit 97edf1f
Show file tree
Hide file tree
Showing 11 changed files with 35 additions and 45 deletions.
17 changes: 9 additions & 8 deletions pkg/sql/alter_role.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice"
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
"github.com/cockroachdb/cockroach/pkg/sql/roleoption"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
Expand Down Expand Up @@ -193,7 +194,7 @@ func (n *alterRoleNode) startExec(params runParams) error {
opName,
params.p.txn,
sessiondata.NodeUserSessionDataOverride,
fmt.Sprintf("SELECT 1 FROM %s WHERE username = $1", sessioninit.UsersTableName),
fmt.Sprintf("SELECT 1 FROM system.public.%s WHERE username = $1", catconstants.UsersTableName),
n.roleName,
)
if err != nil {
Expand Down Expand Up @@ -442,19 +443,19 @@ func (n *alterRoleSetNode) startExec(params runParams) error {
}

var deleteQuery = fmt.Sprintf(
`DELETE FROM %s WHERE database_id = $1 AND role_name = $2`,
sessioninit.DatabaseRoleSettingsTableName,
`DELETE FROM system.public.%s WHERE database_id = $1 AND role_name = $2`,
catconstants.DatabaseRoleSettingsTableName,
)

var upsertQuery = fmt.Sprintf(`
UPSERT INTO %s (database_id, role_name, settings, role_id)
UPSERT INTO system.public.%s (database_id, role_name, settings, role_id)
VALUES ($1, $2, $3, (
SELECT CASE $2
WHEN '%s' THEN %d
ELSE (SELECT user_id FROM system.users WHERE username = $2)
END
))`,
sessioninit.DatabaseRoleSettingsTableName, username.EmptyRole, username.EmptyRoleID,
catconstants.DatabaseRoleSettingsTableName, username.EmptyRole, username.EmptyRoleID,
)

// Instead of inserting an empty settings array, this function will make
Expand Down Expand Up @@ -583,7 +584,7 @@ func (n *alterRoleSetNode) getRoleName(
opName,
params.p.txn,
sessiondata.NodeUserSessionDataOverride,
fmt.Sprintf("SELECT 1 FROM %s WHERE username = $1", sessioninit.UsersTableName),
fmt.Sprintf("SELECT 1 FROM system.public.%s WHERE username = $1", catconstants.UsersTableName),
n.roleName,
)
if err != nil {
Expand Down Expand Up @@ -636,8 +637,8 @@ func (n *alterRoleSetNode) makeNewSettings(
params runParams, opName redact.RedactableString, roleName username.SQLUsername,
) (oldSettings []string, newSettings []string, err error) {
var selectQuery = fmt.Sprintf(
`SELECT settings FROM %s WHERE database_id = $1 AND role_name = $2`,
sessioninit.DatabaseRoleSettingsTableName,
`SELECT settings FROM system.public.%s WHERE database_id = $1 AND role_name = $2`,
catconstants.DatabaseRoleSettingsTableName,
)
datums, err := params.p.InternalSQLTxn().QueryRowEx(
params.ctx,
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/authorization.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
"github.com/cockroachdb/cockroach/pkg/sql/roleoption"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/sessioninit"
"github.com/cockroachdb/cockroach/pkg/sql/sqlerrors"
"github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege"
"github.com/cockroachdb/errors"
Expand Down Expand Up @@ -649,8 +649,8 @@ func (p *planner) UserHasRoleOption(
ctx, "has-role-option", p.Txn(),
sessiondata.NodeUserSessionDataOverride,
fmt.Sprintf(
`SELECT 1 from %s WHERE option = '%s' AND username = $1 LIMIT 1`,
sessioninit.RoleOptionsTableName, roleOption.String()), user.Normalized())
`SELECT 1 from system.public.%s WHERE option = '%s' AND username = $1 LIMIT 1`,
catconstants.RoleOptionsTableName, roleOption.String()), user.Normalized())
if err != nil {
return false, err
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/sql/create_role.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice"
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
"github.com/cockroachdb/cockroach/pkg/sql/roleoption"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/sessioninit"
Expand Down Expand Up @@ -138,7 +139,7 @@ func (n *CreateRoleNode) startExec(params runParams) error {
opName,
params.p.txn,
sessiondata.NodeUserSessionDataOverride,
fmt.Sprintf(`select "isRole" from %s where username = $1`, sessioninit.UsersTableName),
fmt.Sprintf(`select "isRole" from system.public.%s where username = $1`, catconstants.UsersTableName),
n.roleName,
)
if err != nil {
Expand All @@ -153,7 +154,7 @@ func (n *CreateRoleNode) startExec(params runParams) error {
}

// TODO(richardjcai): move hashedPassword column to system.role_options.
stmt := fmt.Sprintf("INSERT INTO %s VALUES ($1, $2, $3, $4)", sessioninit.UsersTableName)
stmt := fmt.Sprintf("INSERT INTO system.public.%s VALUES ($1, $2, $3, $4)", catconstants.UsersTableName)
roleID, err := descidgen.GenerateUniqueRoleID(params.ctx, params.ExecCfg().DB, params.ExecCfg().Codec)
if err != nil {
return err
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/descmetadata/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ go_library(
"//pkg/sql/catalog/tabledesc",
"//pkg/sql/isql",
"//pkg/sql/schemachanger/scexec",
"//pkg/sql/sem/catconstants",
"//pkg/sql/sessiondata",
"//pkg/sql/sessioninit",
"//pkg/sql/ttl/ttlbase",
Expand Down
5 changes: 3 additions & 2 deletions pkg/sql/descmetadata/metadata_updater.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
"github.com/cockroachdb/cockroach/pkg/sql/isql"
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scexec"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/sessioninit"
"github.com/cockroachdb/cockroach/pkg/sql/ttl/ttlbase"
Expand Down Expand Up @@ -58,8 +59,8 @@ func (mu metadataUpdater) DeleteDatabaseRoleSettings(ctx context.Context, dbID d
mu.txn.KV(),
sessiondata.NodeUserSessionDataOverride,
fmt.Sprintf(
`DELETE FROM %s WHERE database_id = $1`,
sessioninit.DatabaseRoleSettingsTableName,
`DELETE FROM system.public.%s WHERE database_id = $1`,
catconstants.DatabaseRoleSettingsTableName,
),
dbID,
)
Expand Down
9 changes: 5 additions & 4 deletions pkg/sql/drop_role.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/sessioninit"
Expand Down Expand Up @@ -447,8 +448,8 @@ func (n *DropRoleNode) startExec(params runParams) error {
params.p.txn,
sessiondata.NodeUserSessionDataOverride,
fmt.Sprintf(
`DELETE FROM %s WHERE username=$1`,
sessioninit.RoleOptionsTableName,
`DELETE FROM system.public.%s WHERE username=$1`,
catconstants.RoleOptionsTableName,
),
normalizedUsername,
)
Expand All @@ -462,8 +463,8 @@ func (n *DropRoleNode) startExec(params runParams) error {
params.p.txn,
sessiondata.NodeUserSessionDataOverride,
fmt.Sprintf(
`DELETE FROM %s WHERE role_name = $1`,
sessioninit.DatabaseRoleSettingsTableName,
`DELETE FROM system.public.%s WHERE role_name = $1`,
catconstants.DatabaseRoleSettingsTableName,
),
normalizedUsername,
); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/sessioninit/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ go_library(
importpath = "github.com/cockroachdb/cockroach/pkg/sql/sessioninit",
visibility = ["//visibility:public"],
deps = [
"//pkg/keys",
"//pkg/security/password",
"//pkg/security/username",
"//pkg/settings",
"//pkg/settings/cluster",
"//pkg/sql/catalog",
"//pkg/sql/catalog/descpb",
"//pkg/sql/catalog/descs",
"//pkg/sql/sem/catconstants",
"//pkg/sql/sem/tree",
"//pkg/util/log",
"//pkg/util/mon",
Expand Down
7 changes: 4 additions & 3 deletions pkg/sql/sessioninit/cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"fmt"
"unsafe"

"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/security/password"
"github.com/cockroachdb/cockroach/pkg/security/username"
"github.com/cockroachdb/cockroach/pkg/settings"
Expand Down Expand Up @@ -132,11 +133,11 @@ func (a *Cache) GetAuthInfo(
if err := txn.Descriptors().MaybeSetReplicationSafeTS(ctx, txn.KV()); err != nil {
return err
}
_, usersTableDesc, err = descs.PrefixAndTable(ctx, txn.Descriptors().ByNameWithLeased(txn.KV()).Get(), UsersTableName)
usersTableDesc, err = txn.Descriptors().ByIDWithLeased(txn.KV()).Get().Table(ctx, keys.UsersTableID)
if err != nil {
return err
}
_, roleOptionsTableDesc, err = descs.PrefixAndTable(ctx, txn.Descriptors().ByNameWithLeased(txn.KV()).Get(), RoleOptionsTableName)
roleOptionsTableDesc, err = txn.Descriptors().ByIDWithLeased(txn.KV()).Get().Table(ctx, keys.RoleOptionsTableID)
return err
})
if err != nil {
Expand Down Expand Up @@ -296,7 +297,7 @@ func (a *Cache) GetDefaultSettings(
err = db.DescsTxn(ctx, func(
ctx context.Context, txn descs.Txn,
) error {
_, dbRoleSettingsTableDesc, err = descs.PrefixAndTable(ctx, txn.Descriptors().ByNameWithLeased(txn.KV()).Get(), DatabaseRoleSettingsTableName)
dbRoleSettingsTableDesc, err = txn.Descriptors().ByIDWithLeased(txn.KV()).Get().Table(ctx, keys.DatabaseRoleSettingsTableID)
if err != nil {
return err
}
Expand Down
15 changes: 1 addition & 14 deletions pkg/sql/sessioninit/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,7 @@

package sessioninit

import (
"github.com/cockroachdb/cockroach/pkg/security/username"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
)

// UsersTableName represents system.users.
var UsersTableName = tree.NewTableNameWithSchema("system", catconstants.PublicSchemaName, "users")

// RoleOptionsTableName represents system.role_options.
var RoleOptionsTableName = tree.NewTableNameWithSchema("system", catconstants.PublicSchemaName, "role_options")

// DatabaseRoleSettingsTableName represents system.database_role_settings.
var DatabaseRoleSettingsTableName = tree.NewTableNameWithSchema("system", catconstants.PublicSchemaName, "database_role_settings")
import "github.com/cockroachdb/cockroach/pkg/security/username"

// defaultDatabaseID is used in the settingsCache for entries that should
// apply to all database.
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/syntheticprivilege/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
)

// SystemPrivilegesTableName represents system.database_role_settings.
// SystemPrivilegesTableName represents system.privileges.
var SystemPrivilegesTableName = tree.NewTableNameWithSchema("system", catconstants.PublicSchemaName, "privileges")
11 changes: 4 additions & 7 deletions pkg/sql/user.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/sessioninit"
Expand Down Expand Up @@ -532,12 +531,10 @@ func RoleExists(ctx context.Context, txn isql.Txn, role username.SQLUsername) (b
return row != nil, nil
}

var roleMembersTableName = tree.MakeTableNameWithSchema("system", catconstants.PublicSchemaName, "role_members")

// BumpRoleMembershipTableVersion increases the table version for the
// role membership table.
func (p *planner) BumpRoleMembershipTableVersion(ctx context.Context) error {
_, tableDesc, err := p.ResolveMutableTableDescriptor(ctx, &roleMembersTableName, true, tree.ResolveAnyTableKind)
tableDesc, err := p.Descriptors().MutableByID(p.Txn()).Table(ctx, keys.RoleMembersTableID)
if err != nil {
return err
}
Expand All @@ -550,7 +547,7 @@ func (p *planner) BumpRoleMembershipTableVersion(ctx context.Context) error {
// bumpUsersTableVersion increases the table version for the
// users table.
func (p *planner) bumpUsersTableVersion(ctx context.Context) error {
_, tableDesc, err := p.ResolveMutableTableDescriptor(ctx, sessioninit.UsersTableName, true, tree.ResolveAnyTableKind)
tableDesc, err := p.Descriptors().MutableByID(p.Txn()).Table(ctx, keys.UsersTableID)
if err != nil {
return err
}
Expand All @@ -563,7 +560,7 @@ func (p *planner) bumpUsersTableVersion(ctx context.Context) error {
// bumpRoleOptionsTableVersion increases the table version for the
// role_options table.
func (p *planner) bumpRoleOptionsTableVersion(ctx context.Context) error {
_, tableDesc, err := p.ResolveMutableTableDescriptor(ctx, sessioninit.RoleOptionsTableName, true, tree.ResolveAnyTableKind)
tableDesc, err := p.Descriptors().MutableByID(p.Txn()).Table(ctx, keys.RoleOptionsTableID)
if err != nil {
return err
}
Expand All @@ -576,7 +573,7 @@ func (p *planner) bumpRoleOptionsTableVersion(ctx context.Context) error {
// bumpDatabaseRoleSettingsTableVersion increases the table version for the
// database_role_settings table.
func (p *planner) bumpDatabaseRoleSettingsTableVersion(ctx context.Context) error {
_, tableDesc, err := p.ResolveMutableTableDescriptor(ctx, sessioninit.DatabaseRoleSettingsTableName, true, tree.ResolveAnyTableKind)
tableDesc, err := p.Descriptors().MutableByID(p.Txn()).Table(ctx, keys.DatabaseRoleSettingsTableID)
if err != nil {
return err
}
Expand Down

0 comments on commit 97edf1f

Please sign in to comment.