Skip to content

Commit

Permalink
sql: fix username parsing for CURRENT_USER/SESSION_USER
Browse files Browse the repository at this point in the history
Release note (sql change): Fix bug where previously CURRENT_USER and
SESSION_USER were parsed incorrectly.
  • Loading branch information
RichardJCai committed Oct 12, 2021
1 parent f730d79 commit d13a9a3
Show file tree
Hide file tree
Showing 42 changed files with 425 additions and 295 deletions.
2 changes: 1 addition & 1 deletion docs/generated/sql/bnf/drop_owned_by_stmt.bnf
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
drop_owned_by_stmt ::=
'DROP' 'OWNED' 'BY' name_list opt_drop_behavior
'DROP' 'OWNED' 'BY' role_spec_list opt_drop_behavior
10 changes: 5 additions & 5 deletions docs/generated/sql/bnf/grant_stmt.bnf
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
grant_stmt ::=
'GRANT' 'ALL' 'PRIVILEGES' 'ON' targets 'TO' name_list
| 'GRANT' 'ALL' 'ON' targets 'TO' name_list
| 'GRANT' privilege_list 'ON' targets 'TO' name_list
| 'GRANT' privilege_list 'TO' name_list
| 'GRANT' privilege_list 'TO' name_list 'WITH' 'ADMIN' 'OPTION'
'GRANT' 'ALL' 'PRIVILEGES' 'ON' targets 'TO' role_spec_list
| 'GRANT' 'ALL' 'ON' targets 'TO' role_spec_list
| 'GRANT' privilege_list 'ON' targets 'TO' role_spec_list
| 'GRANT' privilege_list 'TO' role_spec_list
| 'GRANT' privilege_list 'TO' role_spec_list 'WITH' 'ADMIN' 'OPTION'
2 changes: 1 addition & 1 deletion docs/generated/sql/bnf/reassign_owned_by_stmt.bnf
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
reassign_owned_by_stmt ::=
'REASSIGN' 'OWNED' 'BY' name_list 'TO' role_spec
'REASSIGN' 'OWNED' 'BY' role_spec_list 'TO' role_spec
10 changes: 5 additions & 5 deletions docs/generated/sql/bnf/revoke_stmt.bnf
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
revoke_stmt ::=
'REVOKE' 'ALL' 'PRIVILEGES' 'ON' targets 'FROM' name_list
| 'REVOKE' 'ALL' 'ON' targets 'FROM' name_list
| 'REVOKE' privilege_list 'ON' targets 'FROM' name_list
| 'REVOKE' privilege_list 'FROM' name_list
| 'REVOKE' 'ADMIN' 'OPTION' 'FOR' privilege_list 'FROM' name_list
'REVOKE' 'ALL' 'PRIVILEGES' 'ON' targets 'FROM' role_spec_list
| 'REVOKE' 'ALL' 'ON' targets 'FROM' role_spec_list
| 'REVOKE' privilege_list 'ON' targets 'FROM' role_spec_list
| 'REVOKE' privilege_list 'FROM' role_spec_list
| 'REVOKE' 'ADMIN' 'OPTION' 'FOR' privilege_list 'FROM' role_spec_list
12 changes: 6 additions & 6 deletions docs/generated/sql/bnf/show_grants_stmt.bnf
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
show_grants_stmt ::=
'SHOW' 'GRANTS' 'ON' 'ROLE' name ( ( ',' name ) )* 'FOR' name ( ( ',' name ) )*
| 'SHOW' 'GRANTS' 'ON' 'ROLE' name ( ( ',' name ) )*
| 'SHOW' 'GRANTS' 'ON' 'SCHEMA' schema_name ( ( ',' schema_name ) )* 'FOR' name ( ( ',' name ) )*
'SHOW' 'GRANTS' 'ON' 'ROLE' role_spec_list 'FOR' role_spec_list
| 'SHOW' 'GRANTS' 'ON' 'ROLE' role_spec_list
| 'SHOW' 'GRANTS' 'ON' 'SCHEMA' schema_name ( ( ',' schema_name ) )* 'FOR' role_spec_list
| 'SHOW' 'GRANTS' 'ON' 'SCHEMA' schema_name ( ( ',' schema_name ) )*
| 'SHOW' 'GRANTS' 'ON' 'TYPE' type_name ( ( ',' type_name ) )* 'FOR' name ( ( ',' name ) )*
| 'SHOW' 'GRANTS' 'ON' 'TYPE' type_name ( ( ',' type_name ) )* 'FOR' role_spec_list
| 'SHOW' 'GRANTS' 'ON' 'TYPE' type_name ( ( ',' type_name ) )*
| 'SHOW' 'GRANTS' 'ON' ( | 'TABLE' table_name ( ( ',' table_name ) )* | 'DATABASE' database_name ( ( ',' database_name ) )* ) 'FOR' name ( ( ',' name ) )*
| 'SHOW' 'GRANTS' 'ON' ( | 'TABLE' table_name ( ( ',' table_name ) )* | 'DATABASE' database_name ( ( ',' database_name ) )* ) 'FOR' role_spec_list
| 'SHOW' 'GRANTS' 'ON' ( | 'TABLE' table_name ( ( ',' table_name ) )* | 'DATABASE' database_name ( ( ',' database_name ) )* )
| 'SHOW' 'GRANTS' 'FOR' name ( ( ',' name ) )*
| 'SHOW' 'GRANTS' 'FOR' role_spec_list
| 'SHOW' 'GRANTS'
66 changes: 33 additions & 33 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -74,32 +74,32 @@ discard_stmt ::=
'DISCARD' 'ALL'

grant_stmt ::=
'GRANT' privileges 'ON' targets 'TO' name_list
| 'GRANT' privilege_list 'TO' name_list
| 'GRANT' privilege_list 'TO' name_list 'WITH' 'ADMIN' 'OPTION'
| 'GRANT' privileges 'ON' 'TYPE' target_types 'TO' name_list
| 'GRANT' privileges 'ON' 'SCHEMA' schema_name_list 'TO' name_list
| 'GRANT' privileges 'ON' 'ALL' 'TABLES' 'IN' 'SCHEMA' schema_name_list 'TO' name_list
'GRANT' privileges 'ON' targets 'TO' role_spec_list
| 'GRANT' privilege_list 'TO' role_spec_list
| 'GRANT' privilege_list 'TO' role_spec_list 'WITH' 'ADMIN' 'OPTION'
| 'GRANT' privileges 'ON' 'TYPE' target_types 'TO' role_spec_list
| 'GRANT' privileges 'ON' 'SCHEMA' schema_name_list 'TO' role_spec_list
| 'GRANT' privileges 'ON' 'ALL' 'TABLES' 'IN' 'SCHEMA' schema_name_list 'TO' role_spec_list

prepare_stmt ::=
'PREPARE' table_alias_name prep_type_clause 'AS' preparable_stmt

revoke_stmt ::=
'REVOKE' privileges 'ON' targets 'FROM' name_list
| 'REVOKE' privilege_list 'FROM' name_list
| 'REVOKE' 'ADMIN' 'OPTION' 'FOR' privilege_list 'FROM' name_list
| 'REVOKE' privileges 'ON' 'TYPE' target_types 'FROM' name_list
| 'REVOKE' privileges 'ON' 'SCHEMA' schema_name_list 'FROM' name_list
| 'REVOKE' privileges 'ON' 'ALL' 'TABLES' 'IN' 'SCHEMA' schema_name_list 'FROM' name_list
'REVOKE' privileges 'ON' targets 'FROM' role_spec_list
| 'REVOKE' privilege_list 'FROM' role_spec_list
| 'REVOKE' 'ADMIN' 'OPTION' 'FOR' privilege_list 'FROM' role_spec_list
| 'REVOKE' privileges 'ON' 'TYPE' target_types 'FROM' role_spec_list
| 'REVOKE' privileges 'ON' 'SCHEMA' schema_name_list 'FROM' role_spec_list
| 'REVOKE' privileges 'ON' 'ALL' 'TABLES' 'IN' 'SCHEMA' schema_name_list 'FROM' role_spec_list

savepoint_stmt ::=
'SAVEPOINT' name

reassign_owned_by_stmt ::=
'REASSIGN' 'OWNED' 'BY' name_list 'TO' role_spec
'REASSIGN' 'OWNED' 'BY' role_spec_list 'TO' role_spec

drop_owned_by_stmt ::=
'DROP' 'OWNED' 'BY' name_list opt_drop_behavior
'DROP' 'OWNED' 'BY' role_spec_list opt_drop_behavior

release_stmt ::=
'RELEASE' savepoint_name
Expand Down Expand Up @@ -320,8 +320,8 @@ targets ::=
| 'TENANT' iconst64
| 'DATABASE' name_list

name_list ::=
( name ) ( ( ',' name ) )*
role_spec_list ::=
( role_spec ) ( ( ',' role_spec ) )*

privilege_list ::=
( privilege ) ( ( ',' privilege ) )*
Expand All @@ -337,7 +337,8 @@ prep_type_clause ::=
|

role_spec ::=
username_or_sconst
'identifier'
| unreserved_keyword
| 'CURRENT_USER'
| 'SESSION_USER'

Expand Down Expand Up @@ -808,6 +809,9 @@ db_object_name ::=
simple_db_object_name
| complex_db_object_name

name_list ::=
( name ) ( ( ',' name ) )*

opt_with ::=
'WITH'
|
Expand Down Expand Up @@ -1287,10 +1291,6 @@ qualifiable_schema_name ::=
type_list ::=
( typename ) ( ( ',' typename ) )*

username_or_sconst ::=
non_reserved_word
| 'SCONST'

transaction_mode_list ::=
( transaction_mode ) ( ( opt_comma transaction_mode ) )*

Expand Down Expand Up @@ -1625,7 +1625,7 @@ opt_on_targets_roles ::=
|

for_grantee_clause ::=
'FOR' name_list
'FOR' role_spec_list
|

opt_schedule_executor_type ::=
Expand Down Expand Up @@ -1662,7 +1662,7 @@ partition_name ::=
unrestricted_name

opt_for_roles ::=
'FOR' role_or_group_or_user name_list
'FOR' role_or_group_or_user role_spec_list
|

relation_expr ::=
Expand Down Expand Up @@ -1711,12 +1711,6 @@ typename ::=
simple_typename opt_array_bounds
| simple_typename 'ARRAY'

non_reserved_word ::=
'identifier'
| unreserved_keyword
| col_name_keyword
| type_func_name_keyword

transaction_mode ::=
transaction_user_priority
| transaction_read_mode
Expand Down Expand Up @@ -1864,11 +1858,11 @@ opt_in_schemas ::=
|

abbreviated_grant_stmt ::=
'GRANT' privileges 'ON' alter_default_privileges_target_object 'TO' name_list opt_with_grant_option
'GRANT' privileges 'ON' alter_default_privileges_target_object 'TO' role_spec_list opt_with_grant_option

abbreviated_revoke_stmt ::=
'REVOKE' privileges 'ON' alter_default_privileges_target_object 'FROM' name_list opt_drop_behavior
| 'REVOKE' 'GRANT' 'OPTION' 'FOR' privileges 'ON' alter_default_privileges_target_object 'FROM' name_list opt_drop_behavior
'REVOKE' privileges 'ON' alter_default_privileges_target_object 'FROM' role_spec_list opt_drop_behavior
| 'REVOKE' 'GRANT' 'OPTION' 'FOR' privileges 'ON' alter_default_privileges_target_object 'FROM' role_spec_list opt_drop_behavior

role_options ::=
( role_option ) ( ( role_option ) )*
Expand Down Expand Up @@ -2085,6 +2079,12 @@ table_index_name_list ::=
table_name_list ::=
( table_name ) ( ( ',' table_name ) )*

non_reserved_word ::=
'identifier'
| unreserved_keyword
| col_name_keyword
| type_func_name_keyword

kv_option ::=
name '=' string_or_placeholder
| name
Expand Down Expand Up @@ -2166,7 +2166,7 @@ extra_var_value ::=
| cockroachdb_extra_reserved_keyword

targets_roles ::=
'ROLE' name_list
'ROLE' role_spec_list
| 'SCHEMA' schema_name_list
| 'TYPE' type_name_list
| targets
Expand Down
7 changes: 4 additions & 3 deletions pkg/ccl/importccl/read_import_pgdump.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/row"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
Expand Down Expand Up @@ -241,14 +242,14 @@ func createPostgresSchemas(
parentID descpb.ID,
schemasToCreate map[string]*tree.CreateSchema,
execCfg *sql.ExecutorConfig,
user security.SQLUsername,
sessionData *sessiondata.SessionData,
) ([]*schemadesc.Mutable, error) {
createSchema := func(
ctx context.Context, txn *kv.Txn, descriptors *descs.Collection,
dbDesc catalog.DatabaseDescriptor, schema *tree.CreateSchema,
) (*schemadesc.Mutable, error) {
desc, _, err := sql.CreateUserDefinedSchemaDescriptor(
ctx, user, schema, txn, descriptors, execCfg, dbDesc, false, /* allocateID */
ctx, sessionData, schema, txn, descriptors, execCfg, dbDesc, false, /* allocateID */
)
if err != nil {
return nil, err
Expand Down Expand Up @@ -494,7 +495,7 @@ func readPostgresCreateTable(
tables := make([]*tabledesc.Mutable, 0, len(schemaObjects.createTbl))
schemaNameToDesc := make(map[string]*schemadesc.Mutable)
schemaDescs, err := createPostgresSchemas(ctx, parentDB.GetID(), schemaObjects.createSchema,
p.ExecCfg(), p.User())
p.ExecCfg(), p.SessionData())
if err != nil {
return nil, nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/docgen/diagrams.go
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ var specs = []stmtSpec{
},
{
name: "create_schedule_for_backup_stmt",
inline: []string{"opt_description", "string_or_placeholder_opt_list", "string_or_placeholder_list", "opt_with_backup_options", "cron_expr", "opt_full_backup_clause", "opt_with_schedule_options", "opt_backup_targets"},
inline: []string{"string_or_placeholder_opt_list", "string_or_placeholder_list", "opt_with_backup_options", "cron_expr", "opt_full_backup_clause", "opt_with_schedule_options", "opt_backup_targets"},
replace: map[string]string{
"string_or_placeholder 'FOR'": "label 'FOR'",
"'RECURRING' sconst_or_placeholder": "'RECURRING' cronexpr",
Expand Down
5 changes: 4 additions & 1 deletion pkg/sql/alter_database.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ func (p *planner) AlterDatabaseOwner(
}

func (n *alterDatabaseOwnerNode) startExec(params runParams) error {
newOwner := n.n.Owner
newOwner, err := n.n.Owner.ToSQLUsername(params.p.SessionData())
if err != nil {
return err
}
oldOwner := n.desc.GetPrivileges().Owner()

if err := params.p.checkCanAlterToNewOwner(params.ctx, n.desc, newOwner); err != nil {
Expand Down
15 changes: 5 additions & 10 deletions pkg/sql/alter_default_privileges.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"context"

"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catprivilege"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
Expand Down Expand Up @@ -77,7 +76,7 @@ func (p *planner) alterDefaultPrivileges(
}

func (n *alterDefaultPrivilegesNode) startExec(params runParams) error {
targetRoles, err := n.n.Roles.ToSQLUsernames()
targetRoles, err := n.n.Roles.ToSQLUsernames(params.SessionData())
if err != nil {
return err
}
Expand All @@ -99,13 +98,9 @@ func (n *alterDefaultPrivilegesNode) startExec(params runParams) error {
objectType = n.n.Revoke.Target
}

granteeSQLUsernames := make([]security.SQLUsername, len(grantees))
for i, grantee := range grantees {
user, err := security.MakeSQLUsernameFromUserInput(string(grantee), security.UsernameValidation)
if err != nil {
return err
}
granteeSQLUsernames[i] = user
granteeSQLUsernames, err := grantees.ToSQLUsernames(params.p.SessionData())
if err != nil {
return err
}

if err := params.p.validateRoles(params.ctx, granteeSQLUsernames, true /* isPublicValid */); err != nil {
Expand Down Expand Up @@ -162,7 +157,7 @@ func (n *alterDefaultPrivilegesNode) startExec(params runParams) error {
}

var events []eventLogEntry
granteeSQLUsernames, err = grantees.ToSQLUsernames()
granteeSQLUsernames, err = grantees.ToSQLUsernames(params.SessionData())
if err != nil {
return err
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/sql/alter_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,10 @@ func (n *alterSchemaNode) startExec(params runParams) error {
NewSchemaName: newQualifiedSchemaName.String(),
})
case *tree.AlterSchemaOwner:
newOwner := t.Owner
newOwner, err := t.Owner.ToSQLUsername(params.p.SessionData())
if err != nil {
return err
}
return params.p.alterSchemaOwner(
params.ctx, n.desc, newOwner, tree.AsStringWithFQNames(n.n, params.Ann()),
)
Expand Down
6 changes: 5 additions & 1 deletion pkg/sql/alter_table_owner.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,12 @@ func (p *planner) AlterTableOwner(ctx context.Context, n *tree.AlterTableOwner)
return nil, err
}

owner, err := n.Owner.ToSQLUsername(p.SessionData())
if err != nil {
return nil, err
}
return &alterTableOwnerNode{
owner: n.Owner,
owner: owner,
desc: tableDesc,
n: n,
prefix: prefix,
Expand Down
6 changes: 5 additions & 1 deletion pkg/sql/alter_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,11 @@ func (n *alterTypeNode) startExec(params runParams) error {
// See https://github.com/cockroachdb/cockroach/issues/57741
err = params.p.setTypeSchema(params.ctx, n, string(t.Schema))
case *tree.AlterTypeOwner:
if err = params.p.alterTypeOwner(params.ctx, n, t.Owner); err != nil {
owner, err := t.Owner.ToSQLUsername(params.SessionData())
if err != nil {
return err
}
if err = params.p.alterTypeOwner(params.ctx, n, owner); err != nil {
return err
}
eventLogDone = true // done inside alterTypeOwner().
Expand Down
Loading

0 comments on commit d13a9a3

Please sign in to comment.