Skip to content

Commit

Permalink
sql: anonymize schema and owner names
Browse files Browse the repository at this point in the history 
Schema and owner names were improperly being represented by strings
rather than `tree.Name`s in the AST. This meant they would not be
properly anonymized in telemetry.

Fixes cockroachdb#55385

Release note: None
  • Loading branch information
@solongordon
solongordon committed Oct 13, 2020
1 parent 8c603a1 commit 50aaaa5
Show file tree
Hide file tree
Showing 21 changed files with 103 additions and 103 deletions.
2 changes: 1 addition & 1 deletion docs/generated/sql/bnf/grant_privileges.bnf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ grant_stmt ::=


| 'GRANT' ( 'ALL' | ( ( ( 'CREATE' | 'GRANT' | 'SELECT' | 'DROP' | 'INSERT' | 'DELETE' | 'UPDATE' ) ) ( ( ',' ( 'CREATE' | 'GRANT' | 'SELECT' | 'DROP' | 'INSERT' | 'DELETE' | 'UPDATE' ) ) )* ) ) 'ON' 'TYPE' target_types 'TO' ( ( user_name ) ( ( ',' user_name ) )* )
| 'GRANT' ( 'ALL' | ( ( ( 'CREATE' | 'GRANT' | 'SELECT' | 'DROP' | 'INSERT' | 'DELETE' | 'UPDATE' ) ) ( ( ',' ( 'CREATE' | 'GRANT' | 'SELECT' | 'DROP' | 'INSERT' | 'DELETE' | 'UPDATE' ) ) )* ) ) 'ON' 'SCHEMA' schema_name_list 'TO' ( ( user_name ) ( ( ',' user_name ) )* )
| 'GRANT' ( 'ALL' | ( ( ( 'CREATE' | 'GRANT' | 'SELECT' | 'DROP' | 'INSERT' | 'DELETE' | 'UPDATE' ) ) ( ( ',' ( 'CREATE' | 'GRANT' | 'SELECT' | 'DROP' | 'INSERT' | 'DELETE' | 'UPDATE' ) ) )* ) ) 'ON' 'SCHEMA' ( ( name ) ( ( ',' name ) )* ) 'TO' ( ( user_name ) ( ( ',' user_name ) )* )
2 changes: 1 addition & 1 deletion docs/generated/sql/bnf/revoke_privileges.bnf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ revoke_stmt ::=


| 'REVOKE' ( 'ALL' | ( ( ( 'CREATE' | 'GRANT' | 'SELECT' | 'DROP' | 'INSERT' | 'DELETE' | 'UPDATE' ) ) ( ( ',' ( 'CREATE' | 'GRANT' | 'SELECT' | 'DROP' | 'INSERT' | 'DELETE' | 'UPDATE' ) ) )* ) ) 'ON' 'TYPE' target_types 'FROM' ( ( user_name ) ( ( ',' user_name ) )* )
| 'REVOKE' ( 'ALL' | ( ( ( 'CREATE' | 'GRANT' | 'SELECT' | 'DROP' | 'INSERT' | 'DELETE' | 'UPDATE' ) ) ( ( ',' ( 'CREATE' | 'GRANT' | 'SELECT' | 'DROP' | 'INSERT' | 'DELETE' | 'UPDATE' ) ) )* ) ) 'ON' 'SCHEMA' schema_name_list 'FROM' ( ( user_name ) ( ( ',' user_name ) )* )
| 'REVOKE' ( 'ALL' | ( ( ( 'CREATE' | 'GRANT' | 'SELECT' | 'DROP' | 'INSERT' | 'DELETE' | 'UPDATE' ) ) ( ( ',' ( 'CREATE' | 'GRANT' | 'SELECT' | 'DROP' | 'INSERT' | 'DELETE' | 'UPDATE' ) ) )* ) ) 'ON' 'SCHEMA' ( ( name ) ( ( ',' name ) )* ) 'FROM' ( ( user_name ) ( ( ',' user_name ) )* )
17 changes: 7 additions & 10 deletions docs/generated/sql/bnf/stmt_block.bnf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ grant_stmt ::=
| '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' 'SCHEMA' name_list 'TO' name_list

prepare_stmt ::=
'PREPARE' table_alias_name prep_type_clause 'AS' preparable_stmt
Expand All @@ -84,7 +84,7 @@ revoke_stmt ::=
| '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' 'SCHEMA' name_list 'FROM' name_list

savepoint_stmt ::=
'SAVEPOINT' name
Expand Down Expand Up @@ -297,9 +297,6 @@ privilege_list ::=
target_types ::=
type_name_list

schema_name_list ::=
( schema_name ) ( ( ',' schema_name ) )*

prep_type_clause ::=
'(' type_list ')'
|
Expand Down Expand Up @@ -1140,9 +1137,6 @@ privilege ::=
type_name_list ::=
( type_name ) ( ( ',' type_name ) )*

schema_name ::=
name

type_list ::=
( typename ) ( ( ',' typename ) )*

Expand Down Expand Up @@ -1347,8 +1341,8 @@ drop_sequence_stmt ::=
| 'DROP' 'SEQUENCE' 'IF' 'EXISTS' table_name_list opt_drop_behavior

drop_schema_stmt ::=
'DROP' 'SCHEMA' schema_name_list opt_drop_behavior
| 'DROP' 'SCHEMA' 'IF' 'EXISTS' schema_name_list opt_drop_behavior
'DROP' 'SCHEMA' name_list opt_drop_behavior
| 'DROP' 'SCHEMA' 'IF' 'EXISTS' name_list opt_drop_behavior

drop_type_stmt ::=
'DROP' 'TYPE' type_name_list opt_drop_behavior
Expand Down Expand Up @@ -1616,6 +1610,9 @@ alter_zone_partition_stmt ::=
| 'ALTER' 'PARTITION' partition_name 'OF' 'INDEX' table_index_name set_zone_config
| 'ALTER' 'PARTITION' partition_name 'OF' 'INDEX' table_name '@' '*' set_zone_config

schema_name ::=
name

role_spec ::=
non_reserved_word_or_sconst
| 'CURRENT_USER'
Expand Down
7 changes: 4 additions & 3 deletions pkg/sql/alter_database.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ func (p *planner) AlterDatabaseOwner(
}
privs := dbDesc.GetPrivileges()

if err := p.checkCanAlterToNewOwner(ctx, dbDesc, n.Owner); err != nil {
newOwner := string(n.Owner)
if err := p.checkCanAlterToNewOwner(ctx, dbDesc, newOwner); err != nil {
return nil, err
}

Expand All @@ -43,14 +44,14 @@ func (p *planner) AlterDatabaseOwner(
}

// If the owner we want to set to is the current owner, do a no-op.
if n.Owner == privs.Owner {
if newOwner == privs.Owner {
return nil, nil
}
return &alterDatabaseOwnerNode{n: n, desc: dbDesc}, nil
}

func (n *alterDatabaseOwnerNode) startExec(params runParams) error {
n.desc.GetPrivileges().SetOwner(n.n.Owner)
n.desc.GetPrivileges().SetOwner(string(n.n.Owner))
return params.p.writeNonDropDatabaseChange(
params.ctx,
n.desc,
Expand Down
8 changes: 5 additions & 3 deletions pkg/sql/alter_schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (p *planner) AlterSchema(ctx context.Context, n *tree.AlterSchema) (planNod
if err != nil {
return nil, err
}
found, schema, err := p.ResolveMutableSchemaDescriptor(ctx, db.ID, n.Schema, true /* required */)
found, schema, err := p.ResolveMutableSchemaDescriptor(ctx, db.ID, string(n.Schema), true /* required */)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -78,9 +78,11 @@ func (p *planner) AlterSchema(ctx context.Context, n *tree.AlterSchema) (planNod
func (n *alterSchemaNode) startExec(params runParams) error {
switch t := n.n.Cmd.(type) {
case *tree.AlterSchemaRename:
return params.p.renameSchema(params.ctx, n.db, n.desc, t.NewName, tree.AsStringWithFQNames(n.n, params.Ann()))
return params.p.renameSchema(
params.ctx, n.db, n.desc, string(t.NewName), tree.AsStringWithFQNames(n.n, params.Ann()))
case *tree.AlterSchemaOwner:
return params.p.alterSchemaOwner(params.ctx, n.db, n.desc, t.Owner, tree.AsStringWithFQNames(n.n, params.Ann()))
return params.p.alterSchemaOwner(
params.ctx, n.db, n.desc, string(t.Owner), tree.AsStringWithFQNames(n.n, params.Ann()))
default:
return errors.AssertionFailedf("unknown schema cmd %T", t)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/alter_table.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,7 @@ func (n *alterTableNode) startExec(params runParams) error {
}
descriptorChanged = true
case *tree.AlterTableOwner:
changed, err := params.p.alterTableOwner(params.p.EvalContext().Context, n, t.Owner)
changed, err := params.p.alterTableOwner(params.p.EvalContext().Context, n, string(t.Owner))
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/alter_table_set_schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (p *planner) AlterTableSetSchema(
}

return &alterTableSetSchemaNode{
newSchema: n.Schema,
newSchema: string(n.Schema),
tableDesc: tableDesc,
n: n,
}, nil
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/alter_type.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,11 @@ func (n *alterTypeNode) startExec(params runParams) error {
case *tree.AlterTypeRenameValue:
err = params.p.renameTypeValue(params.ctx, n, t.OldVal, t.NewVal)
case *tree.AlterTypeRename:
err = params.p.renameType(params.ctx, n, t.NewName)
err = params.p.renameType(params.ctx, n, string(t.NewName))
case *tree.AlterTypeSetSchema:
err = params.p.setTypeSchema(params.ctx, n, t.Schema)
err = params.p.setTypeSchema(params.ctx, n, string(t.Schema))
case *tree.AlterTypeOwner:
err = params.p.alterTypeOwner(params.ctx, n, t.Owner)
err = params.p.alterTypeOwner(params.ctx, n, string(t.Owner))
default:
err = errors.AssertionFailedf("unknown alter type cmd %s", t)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/create_schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (p *planner) createUserDefinedSchema(params runParams, n *tree.CreateSchema
return err
}

schemaName := n.Schema
schemaName := string(n.Schema)
if n.Schema == "" {
schemaName = n.AuthRole
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/sql/delegate/show_grants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ FROM "".information_schema.type_privileges`
fmt.Fprintf(&cond, `WHERE database_name IN (%s)`, strings.Join(params, ","))
}
} else if n.Targets != nil && len(n.Targets.Schemas) > 0 {
for _, schema := range n.Targets.Schemas {
schemaNames := n.Targets.Schemas.ToStrings()
for _, schema := range schemaNames {
name := cat.SchemaName{
SchemaName: tree.Name(schema),
ExplicitSchema: true,
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/drop_schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (p *planner) DropSchema(ctx context.Context, n *tree.DropSchema) (planNode,
d := newDropCascadeState()

// Collect all schemas to be deleted.
for _, scName := range n.Names {
for _, scName := range n.Names.ToStrings() {
found, sc, err := p.ResolveMutableSchemaDescriptor(ctx, db.ID, scName, false /* required */)
if err != nil {
return nil, err
Expand Down
Loading

0 comments on commit 50aaaa5

Please sign in to comment.