From 12ad408d162b38e528306aaa7b07454fe0904ab0 Mon Sep 17 00:00:00 2001 From: Solon Gordon Date: Mon, 12 Oct 2020 17:04:02 -0400 Subject: [PATCH 1/2] sql: anonymize schema and owner names 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 #55385 Release note: None --- docs/generated/sql/bnf/grant_privileges.bnf | 2 +- docs/generated/sql/bnf/revoke_privileges.bnf | 2 +- docs/generated/sql/bnf/stmt_block.bnf | 17 ++--- pkg/sql/alter_database.go | 7 +- pkg/sql/alter_schema.go | 8 ++- pkg/sql/alter_table.go | 2 +- pkg/sql/alter_table_set_schema.go | 2 +- pkg/sql/alter_type.go | 6 +- pkg/sql/create_schema.go | 2 +- pkg/sql/delegate/show_grants.go | 3 +- pkg/sql/drop_schema.go | 2 +- pkg/sql/parser/sql.y | 71 +++++++++----------- pkg/sql/resolver.go | 3 +- pkg/sql/sem/tree/alter_database.go | 4 +- pkg/sql/sem/tree/alter_schema.go | 14 ++-- pkg/sql/sem/tree/alter_table.go | 8 +-- pkg/sql/sem/tree/alter_type.go | 13 ++-- pkg/sql/sem/tree/create.go | 4 +- pkg/sql/sem/tree/drop.go | 9 +-- pkg/sql/sem/tree/format_test.go | 18 +++++ pkg/sql/sem/tree/grant.go | 9 +-- 21 files changed, 103 insertions(+), 103 deletions(-) diff --git a/docs/generated/sql/bnf/grant_privileges.bnf b/docs/generated/sql/bnf/grant_privileges.bnf index eda2f20149d4..233ef522c5f5 100644 --- a/docs/generated/sql/bnf/grant_privileges.bnf +++ b/docs/generated/sql/bnf/grant_privileges.bnf @@ -3,4 +3,4 @@ grant_stmt ::= | 'GRANT' ( 'ALL' opt_privileges_clause | ( ( ( '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' opt_privileges_clause | ( ( ( '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' opt_privileges_clause | ( ( ( 'CREATE' | 'GRANT' | 'SELECT' | 'DROP' | 'INSERT' | 'DELETE' | 'UPDATE' ) ) ( ( ',' ( 'CREATE' | 'GRANT' | 'SELECT' | 'DROP' | 'INSERT' | 'DELETE' | 'UPDATE' ) ) )* ) ) 'ON' 'SCHEMA' ( ( name ) ( ( ',' name ) )* ) 'TO' ( ( user_name ) ( ( ',' user_name ) )* ) diff --git a/docs/generated/sql/bnf/revoke_privileges.bnf b/docs/generated/sql/bnf/revoke_privileges.bnf index 657db2700f83..584bf6a6fad2 100644 --- a/docs/generated/sql/bnf/revoke_privileges.bnf +++ b/docs/generated/sql/bnf/revoke_privileges.bnf @@ -3,4 +3,4 @@ revoke_stmt ::= | 'REVOKE' ( 'ALL' opt_privileges_clause | ( ( ( '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' opt_privileges_clause | ( ( ( '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' opt_privileges_clause | ( ( ( 'CREATE' | 'GRANT' | 'SELECT' | 'DROP' | 'INSERT' | 'DELETE' | 'UPDATE' ) ) ( ( ',' ( 'CREATE' | 'GRANT' | 'SELECT' | 'DROP' | 'INSERT' | 'DELETE' | 'UPDATE' ) ) )* ) ) 'ON' 'SCHEMA' ( ( name ) ( ( ',' name ) )* ) 'FROM' ( ( user_name ) ( ( ',' user_name ) )* ) diff --git a/docs/generated/sql/bnf/stmt_block.bnf b/docs/generated/sql/bnf/stmt_block.bnf index 0916c42fbfd6..6b4d63c86ac8 100644 --- a/docs/generated/sql/bnf/stmt_block.bnf +++ b/docs/generated/sql/bnf/stmt_block.bnf @@ -75,7 +75,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 @@ -85,7 +85,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 @@ -301,9 +301,6 @@ privilege_list ::= target_types ::= type_name_list -schema_name_list ::= - ( schema_name ) ( ( ',' schema_name ) )* - prep_type_clause ::= '(' type_list ')' | @@ -1160,9 +1157,6 @@ privilege ::= type_name_list ::= ( type_name ) ( ( ',' type_name ) )* -schema_name ::= - name - type_list ::= ( typename ) ( ( ',' typename ) )* @@ -1371,8 +1365,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 @@ -1642,6 +1636,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 + opt_add_val_placement ::= 'BEFORE' 'SCONST' | 'AFTER' 'SCONST' diff --git a/pkg/sql/alter_database.go b/pkg/sql/alter_database.go index a896391aafe4..bc7b927b80d5 100644 --- a/pkg/sql/alter_database.go +++ b/pkg/sql/alter_database.go @@ -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 } @@ -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, diff --git a/pkg/sql/alter_schema.go b/pkg/sql/alter_schema.go index 187189cad3cc..d9d6f6ae92a1 100644 --- a/pkg/sql/alter_schema.go +++ b/pkg/sql/alter_schema.go @@ -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 } @@ -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) } diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go index b1e5df18c3e1..5b1d2076de47 100644 --- a/pkg/sql/alter_table.go +++ b/pkg/sql/alter_table.go @@ -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 } diff --git a/pkg/sql/alter_table_set_schema.go b/pkg/sql/alter_table_set_schema.go index f6834c6f68c9..64a77fdf2b10 100644 --- a/pkg/sql/alter_table_set_schema.go +++ b/pkg/sql/alter_table_set_schema.go @@ -77,7 +77,7 @@ func (p *planner) AlterTableSetSchema( } return &alterTableSetSchemaNode{ - newSchema: n.Schema, + newSchema: string(n.Schema), tableDesc: tableDesc, n: n, }, nil diff --git a/pkg/sql/alter_type.go b/pkg/sql/alter_type.go index efb3182c25c2..fcf0387aee2e 100644 --- a/pkg/sql/alter_type.go +++ b/pkg/sql/alter_type.go @@ -73,11 +73,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) } diff --git a/pkg/sql/create_schema.go b/pkg/sql/create_schema.go index 2384f5a07061..f53bf6400b32 100644 --- a/pkg/sql/create_schema.go +++ b/pkg/sql/create_schema.go @@ -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 } diff --git a/pkg/sql/delegate/show_grants.go b/pkg/sql/delegate/show_grants.go index 431bb2ec82f1..0ce9b90e10c7 100644 --- a/pkg/sql/delegate/show_grants.go +++ b/pkg/sql/delegate/show_grants.go @@ -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, diff --git a/pkg/sql/drop_schema.go b/pkg/sql/drop_schema.go index c325ef1c8223..512bbf963298 100644 --- a/pkg/sql/drop_schema.go +++ b/pkg/sql/drop_schema.go @@ -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 diff --git a/pkg/sql/parser/sql.y b/pkg/sql/parser/sql.y index 672d5711714f..ffe29713ef4d 100644 --- a/pkg/sql/parser/sql.y +++ b/pkg/sql/parser/sql.y @@ -944,7 +944,6 @@ func (u *sqlSymUnion) refreshDataOption() tree.RefreshDataOption { %type <*tree.UnresolvedObjectName> table_name standalone_index_name sequence_name type_name view_name db_object_name simple_db_object_name complex_db_object_name %type <[]*tree.UnresolvedObjectName> type_name_list %type schema_name opt_schema_name -%type <[]string> schema_name_list %type <*tree.UnresolvedName> table_pattern complex_table_pattern %type <*tree.UnresolvedName> column_path prefixed_column_path column_path_with_star %type insert_target create_stats_target analyze_target @@ -1426,7 +1425,7 @@ alter_database_stmt: alter_database_owner: ALTER DATABASE database_name OWNER TO role_spec { - $$.val = &tree.AlterDatabaseOwner{Name: tree.Name($3), Owner: $6} + $$.val = &tree.AlterDatabaseOwner{Name: tree.Name($3), Owner: tree.Name($6)} } // %Help: ALTER RANGE - change the parameters of a range @@ -1916,7 +1915,7 @@ alter_table_cmd: | OWNER TO role_spec { $$.val = &tree.AlterTableOwner{ - Owner: $3, + Owner: tree.Name($3), } } @@ -2044,7 +2043,7 @@ alter_type_stmt: $$.val = &tree.AlterType{ Type: $3.unresolvedObjectName(), Cmd: &tree.AlterTypeRename{ - NewName: $6, + NewName: tree.Name($6), }, } } @@ -2053,7 +2052,7 @@ alter_type_stmt: $$.val = &tree.AlterType{ Type: $3.unresolvedObjectName(), Cmd: &tree.AlterTypeSetSchema{ - Schema: $6, + Schema: tree.Name($6), }, } } @@ -2062,7 +2061,7 @@ alter_type_stmt: $$.val = &tree.AlterType{ Type: $3.unresolvedObjectName(), Cmd: &tree.AlterTypeOwner{ - Owner: $6, + Owner: tree.Name($6), }, } } @@ -3391,34 +3390,24 @@ type_name_list: // %Category: DDL // %Text: DROP SCHEMA [IF EXISTS] [, ...] [CASCADE | RESTRICT] drop_schema_stmt: - DROP SCHEMA schema_name_list opt_drop_behavior + DROP SCHEMA name_list opt_drop_behavior { $$.val = &tree.DropSchema{ - Names: $3.strs(), + Names: $3.nameList(), IfExists: false, DropBehavior: $4.dropBehavior(), } } -| DROP SCHEMA IF EXISTS schema_name_list opt_drop_behavior +| DROP SCHEMA IF EXISTS name_list opt_drop_behavior { $$.val = &tree.DropSchema{ - Names: $5.strs(), + Names: $5.nameList(), IfExists: true, DropBehavior: $6.dropBehavior(), } } | DROP SCHEMA error // SHOW HELP: DROP SCHEMA -schema_name_list: - schema_name - { - $$.val = []string{$1} - } -| schema_name_list ',' schema_name - { - $$.val = append($1.strs(), $3) - } - // %Help: DROP ROLE - remove a user // %Category: Priv // %Text: DROP ROLE [IF EXISTS] [, ...] @@ -3721,12 +3710,12 @@ grant_stmt: { $$.val = &tree.Grant{Privileges: $2.privilegeList(), Targets: $5.targetList(), Grantees: $7.nameList()} } -| GRANT privileges ON SCHEMA schema_name_list TO name_list +| GRANT privileges ON SCHEMA name_list TO name_list { $$.val = &tree.Grant{ Privileges: $2.privilegeList(), Targets: tree.TargetList{ - Schemas: $5.strs(), + Schemas: $5.nameList(), }, Grantees: $7.nameList(), } @@ -3768,12 +3757,12 @@ revoke_stmt: { $$.val = &tree.Revoke{Privileges: $2.privilegeList(), Targets: $5.targetList(), Grantees: $7.nameList()} } -| REVOKE privileges ON SCHEMA schema_name_list FROM name_list +| REVOKE privileges ON SCHEMA name_list FROM name_list { $$.val = &tree.Revoke{ Privileges: $2.privilegeList(), Targets: tree.TargetList{ - Schemas: $5.strs(), + Schemas: $5.nameList(), }, Grantees: $7.nameList(), } @@ -5245,7 +5234,7 @@ targets_roles: } | SCHEMA name_list { - $$.val = tree.TargetList{Schemas: $2.nameList().ToStrings()} + $$.val = tree.TargetList{Schemas: $2.nameList()} } | TYPE type_name_list { @@ -5363,27 +5352,27 @@ create_schema_stmt: CREATE SCHEMA schema_name { $$.val = &tree.CreateSchema{ - Schema: $3, + Schema: tree.Name($3), } } | CREATE SCHEMA IF NOT EXISTS schema_name { $$.val = &tree.CreateSchema{ - Schema: $6, + Schema: tree.Name($6), IfNotExists: true, } } | CREATE SCHEMA opt_schema_name AUTHORIZATION role_spec { $$.val = &tree.CreateSchema{ - Schema: $3, + Schema: tree.Name($3), AuthRole: $5, } } | CREATE SCHEMA IF NOT EXISTS opt_schema_name AUTHORIZATION role_spec { $$.val = &tree.CreateSchema{ - Schema: $6, + Schema: tree.Name($6), IfNotExists: true, AuthRole: $8, } @@ -5401,18 +5390,18 @@ alter_schema_stmt: ALTER SCHEMA schema_name RENAME TO schema_name { $$.val = &tree.AlterSchema{ - Schema: $3, + Schema: tree.Name($3), Cmd: &tree.AlterSchemaRename{ - NewName: $6, + NewName: tree.Name($6), }, } } | ALTER SCHEMA schema_name OWNER TO role_spec { $$.val = &tree.AlterSchema{ - Schema: $3, + Schema: tree.Name($3), Cmd: &tree.AlterSchemaOwner{ - Owner: $6, + Owner: tree.Name($6), }, } } @@ -6896,13 +6885,13 @@ alter_table_set_schema_stmt: ALTER TABLE relation_expr SET SCHEMA schema_name { $$.val = &tree.AlterTableSetSchema{ - Name: $3.unresolvedObjectName(), Schema: $6, IfExists: false, + Name: $3.unresolvedObjectName(), Schema: tree.Name($6), IfExists: false, } } | ALTER TABLE IF EXISTS relation_expr SET SCHEMA schema_name { $$.val = &tree.AlterTableSetSchema{ - Name: $5.unresolvedObjectName(), Schema: $8, IfExists: true, + Name: $5.unresolvedObjectName(), Schema: tree.Name($8), IfExists: true, } } @@ -6910,14 +6899,14 @@ alter_view_set_schema_stmt: ALTER VIEW relation_expr SET SCHEMA schema_name { $$.val = &tree.AlterTableSetSchema{ - Name: $3.unresolvedObjectName(), Schema: $6, IfExists: false, IsView: true, + Name: $3.unresolvedObjectName(), Schema: tree.Name($6), IfExists: false, IsView: true, } } | ALTER MATERIALIZED VIEW relation_expr SET SCHEMA schema_name { $$.val = &tree.AlterTableSetSchema{ Name: $4.unresolvedObjectName(), - Schema: $7, + Schema: tree.Name($7), IfExists: false, IsView: true, IsMaterialized: true, @@ -6926,14 +6915,14 @@ alter_view_set_schema_stmt: | ALTER VIEW IF EXISTS relation_expr SET SCHEMA schema_name { $$.val = &tree.AlterTableSetSchema{ - Name: $5.unresolvedObjectName(), Schema: $8, IfExists: true, IsView: true, + Name: $5.unresolvedObjectName(), Schema: tree.Name($8), IfExists: true, IsView: true, } } | ALTER MATERIALIZED VIEW IF EXISTS relation_expr SET SCHEMA schema_name { $$.val = &tree.AlterTableSetSchema{ Name: $6.unresolvedObjectName(), - Schema: $9, + Schema: tree.Name($9), IfExists: true, IsView: true, IsMaterialized: true, @@ -6944,13 +6933,13 @@ alter_sequence_set_schema_stmt: ALTER SEQUENCE relation_expr SET SCHEMA schema_name { $$.val = &tree.AlterTableSetSchema{ - Name: $3.unresolvedObjectName(), Schema: $6, IfExists: false, IsSequence: true, + Name: $3.unresolvedObjectName(), Schema: tree.Name($6), IfExists: false, IsSequence: true, } } | ALTER SEQUENCE IF EXISTS relation_expr SET SCHEMA schema_name { $$.val = &tree.AlterTableSetSchema{ - Name: $5.unresolvedObjectName(), Schema: $8, IfExists: true, IsSequence: true, + Name: $5.unresolvedObjectName(), Schema: tree.Name($8), IfExists: true, IsSequence: true, } } diff --git a/pkg/sql/resolver.go b/pkg/sql/resolver.go index 5ad966d2a710..7c96db8e2eb8 100644 --- a/pkg/sql/resolver.go +++ b/pkg/sql/resolver.go @@ -344,7 +344,8 @@ func getDescriptorsFromTargetListForPrivilegeChange( return nil, err } for _, sc := range targets.Schemas { - _, resSchema, err := p.ResolveMutableSchemaDescriptor(ctx, curDB.ID, sc, true /* required */) + _, resSchema, err := p.ResolveMutableSchemaDescriptor( + ctx, curDB.ID, string(sc), true /* required */) if err != nil { return nil, err } diff --git a/pkg/sql/sem/tree/alter_database.go b/pkg/sql/sem/tree/alter_database.go index 8dfb49bb7745..db96ff483dda 100644 --- a/pkg/sql/sem/tree/alter_database.go +++ b/pkg/sql/sem/tree/alter_database.go @@ -13,7 +13,7 @@ package tree // AlterDatabaseOwner represents a ALTER DATABASE OWNER TO statement. type AlterDatabaseOwner struct { Name Name - Owner string + Owner Name } // Format implements the NodeFormatter interface. @@ -21,5 +21,5 @@ func (node *AlterDatabaseOwner) Format(ctx *FmtCtx) { ctx.WriteString("ALTER DATABASE ") ctx.FormatNode(&node.Name) ctx.WriteString(" OWNER TO ") - ctx.FormatNameP(&node.Owner) + ctx.FormatNode(&node.Owner) } diff --git a/pkg/sql/sem/tree/alter_schema.go b/pkg/sql/sem/tree/alter_schema.go index bf0b2994afa6..cb3a7bd5be37 100644 --- a/pkg/sql/sem/tree/alter_schema.go +++ b/pkg/sql/sem/tree/alter_schema.go @@ -12,7 +12,7 @@ package tree // AlterSchema represents an ALTER SCHEMA statement. type AlterSchema struct { - Schema string + Schema Name Cmd AlterSchemaCmd } @@ -21,7 +21,7 @@ var _ Statement = &AlterSchema{} // Format implements the NodeFormatter interface. func (node *AlterSchema) Format(ctx *FmtCtx) { ctx.WriteString("ALTER SCHEMA ") - ctx.FormatNameP(&node.Schema) + ctx.FormatNode(&node.Schema) ctx.FormatNode(node.Cmd) } @@ -35,24 +35,24 @@ func (*AlterSchemaRename) alterSchemaCmd() {} // AlterSchemaRename represents an ALTER SCHEMA RENAME command. type AlterSchemaRename struct { - NewName string + NewName Name } // Format implements the NodeFormatter interface. func (node *AlterSchemaRename) Format(ctx *FmtCtx) { ctx.WriteString(" RENAME TO ") - ctx.FormatNameP(&node.NewName) + ctx.FormatNode(&node.NewName) } func (*AlterSchemaOwner) alterSchemaCmd() {} -// AlterSchemaOwner represents an ALTER SCHEMA RENAME command. +// AlterSchemaOwner represents an ALTER SCHEMA OWNER TO command. type AlterSchemaOwner struct { - Owner string + Owner Name } // Format implements the NodeFormatter interface. func (node *AlterSchemaOwner) Format(ctx *FmtCtx) { ctx.WriteString(" OWNER TO ") - ctx.FormatNameP(&node.Owner) + ctx.FormatNode(&node.Owner) } diff --git a/pkg/sql/sem/tree/alter_table.go b/pkg/sql/sem/tree/alter_table.go index dd5c3117ad67..3124775bd1d4 100644 --- a/pkg/sql/sem/tree/alter_table.go +++ b/pkg/sql/sem/tree/alter_table.go @@ -551,7 +551,7 @@ func (node *AlterTableInjectStats) Format(ctx *FmtCtx) { // AlterTableSetSchema represents an ALTER TABLE SET SCHEMA command. type AlterTableSetSchema struct { Name *UnresolvedObjectName - Schema string + Schema Name IfExists bool IsView bool IsMaterialized bool @@ -576,12 +576,12 @@ func (node *AlterTableSetSchema) Format(ctx *FmtCtx) { } node.Name.Format(ctx) ctx.WriteString(" SET SCHEMA ") - ctx.WriteString(node.Schema) + ctx.FormatNode(&node.Schema) } // AlterTableOwner represents an ALTER TABLE OWNER TO command. type AlterTableOwner struct { - Owner string + Owner Name } // TelemetryCounter implements the AlterTableCmd interface. @@ -592,5 +592,5 @@ func (node *AlterTableOwner) TelemetryCounter() telemetry.Counter { // Format implements the NodeFormatter interface. func (node *AlterTableOwner) Format(ctx *FmtCtx) { ctx.WriteString(" OWNER TO ") - ctx.FormatNameP(&node.Owner) + ctx.FormatNode(&node.Owner) } diff --git a/pkg/sql/sem/tree/alter_type.go b/pkg/sql/sem/tree/alter_type.go index db491eca47f7..8cfecff1cb47 100644 --- a/pkg/sql/sem/tree/alter_type.go +++ b/pkg/sql/sem/tree/alter_type.go @@ -88,6 +88,7 @@ type AlterTypeAddValuePlacement struct { // AlterTypeRenameValue represents an ALTER TYPE RENAME VALUE command. type AlterTypeRenameValue struct { + // TODO? OldVal string NewVal string } @@ -107,13 +108,13 @@ func (node *AlterTypeRenameValue) TelemetryCounter() telemetry.Counter { // AlterTypeRename represents an ALTER TYPE RENAME command. type AlterTypeRename struct { - NewName string + NewName Name } // Format implements the NodeFormatter interface. func (node *AlterTypeRename) Format(ctx *FmtCtx) { ctx.WriteString(" RENAME TO ") - ctx.WriteString(node.NewName) + ctx.FormatNode(&node.NewName) } // TelemetryCounter implements the AlterTypeCmd interface. @@ -123,13 +124,13 @@ func (node *AlterTypeRename) TelemetryCounter() telemetry.Counter { // AlterTypeSetSchema represents an ALTER TYPE SET SCHEMA command. type AlterTypeSetSchema struct { - Schema string + Schema Name } // Format implements the NodeFormatter interface. func (node *AlterTypeSetSchema) Format(ctx *FmtCtx) { ctx.WriteString(" SET SCHEMA ") - ctx.WriteString(node.Schema) + ctx.FormatNode(&node.Schema) } // TelemetryCounter implements the AlterTypeCmd interface. @@ -139,13 +140,13 @@ func (node *AlterTypeSetSchema) TelemetryCounter() telemetry.Counter { // AlterTypeOwner represents an ALTER TYPE OWNER TO command. type AlterTypeOwner struct { - Owner string + Owner Name } // Format implements the NodeFormatter interface. func (node *AlterTypeOwner) Format(ctx *FmtCtx) { ctx.WriteString(" OWNER TO ") - ctx.FormatNameP(&node.Owner) + ctx.FormatNode(&node.Owner) } // TelemetryCounter implements the AlterTypeCmd interface. diff --git a/pkg/sql/sem/tree/create.go b/pkg/sql/sem/tree/create.go index 97720c737c96..7d423443148f 100644 --- a/pkg/sql/sem/tree/create.go +++ b/pkg/sql/sem/tree/create.go @@ -1277,7 +1277,7 @@ func (node *CreateTable) HoistConstraints() { // CreateSchema represents a CREATE SCHEMA statement. type CreateSchema struct { IfNotExists bool - Schema string + Schema Name AuthRole string } @@ -1291,7 +1291,7 @@ func (node *CreateSchema) Format(ctx *FmtCtx) { if node.Schema != "" { ctx.WriteString(" ") - ctx.WriteString(node.Schema) + ctx.FormatNode(&node.Schema) } if node.AuthRole != "" { diff --git a/pkg/sql/sem/tree/drop.go b/pkg/sql/sem/tree/drop.go index 9059dffb0d80..e7cfaf9a9879 100644 --- a/pkg/sql/sem/tree/drop.go +++ b/pkg/sql/sem/tree/drop.go @@ -198,7 +198,7 @@ func (node *DropType) Format(ctx *FmtCtx) { // DropSchema represents a DROP SCHEMA command. type DropSchema struct { - Names []string + Names NameList IfExists bool DropBehavior DropBehavior } @@ -211,12 +211,7 @@ func (node *DropSchema) Format(ctx *FmtCtx) { if node.IfExists { ctx.WriteString("IF EXISTS ") } - for i := range node.Names { - if i > 0 { - ctx.WriteString(", ") - } - ctx.FormatNameP(&node.Names[i]) - } + ctx.FormatNode(&node.Names) if node.DropBehavior != DropDefault { ctx.WriteString(" ") ctx.WriteString(node.DropBehavior.String()) diff --git a/pkg/sql/sem/tree/format_test.go b/pkg/sql/sem/tree/format_test.go index b433bc4f5ba6..b8dcf9a18a39 100644 --- a/pkg/sql/sem/tree/format_test.go +++ b/pkg/sql/sem/tree/format_test.go @@ -98,6 +98,24 @@ func TestFormatStatement(t *testing.T) { `SET time zone = utc`}, {`SET "time zone" = UTC`, tree.FmtBareStrings, `SET "time zone" = utc`}, + + // Test schema anonymization. + {`CREATE SCHEMA s`, tree.FmtAnonymize, + `CREATE SCHEMA _`}, + {`ALTER SCHEMA s1 RENAME TO s2`, tree.FmtAnonymize, + `ALTER SCHEMA _ RENAME TO _`}, + {`DROP SCHEMA a, b`, tree.FmtAnonymize, + `DROP SCHEMA _, _`}, + {`GRANT SELECT ON SCHEMA a TO b, c`, tree.FmtAnonymize, + `GRANT SELECT ON SCHEMA _ TO _, _`}, + {`ALTER TYPE t SET SCHEMA s`, tree.FmtAnonymize, + `ALTER TYPE _ SET SCHEMA _`}, + + // Test owner anonymization. + {`ALTER DATABASE d OWNER TO o`, tree.FmtAnonymize, + `ALTER DATABASE _ OWNER TO _`}, + {`ALTER SCHEMA s OWNER TO o`, tree.FmtAnonymize, + `ALTER SCHEMA _ OWNER TO _`}, } for i, test := range testData { diff --git a/pkg/sql/sem/tree/grant.go b/pkg/sql/sem/tree/grant.go index da51948516a3..36e787a79653 100644 --- a/pkg/sql/sem/tree/grant.go +++ b/pkg/sql/sem/tree/grant.go @@ -37,7 +37,7 @@ type Grant struct { // Only one field may be non-nil. type TargetList struct { Databases NameList - Schemas []string + Schemas NameList Tables TablePatterns Tenant roachpb.TenantID Types []*UnresolvedObjectName @@ -56,12 +56,7 @@ func (tl *TargetList) Format(ctx *FmtCtx) { ctx.FormatNode(&tl.Databases) } else if tl.Schemas != nil { ctx.WriteString("SCHEMA ") - for i := range tl.Schemas { - if i != 0 { - ctx.WriteString(", ") - } - ctx.FormatNameP(&tl.Schemas[i]) - } + ctx.FormatNode(&tl.Schemas) } else if tl.Tenant != (roachpb.TenantID{}) { ctx.WriteString(fmt.Sprintf("TENANT %d", tl.Tenant.ToUint64())) } else if tl.Types != nil { From bad7df0d5d21e45372f315f52752323a024c583d Mon Sep 17 00:00:00 2001 From: Solon Gordon Date: Tue, 13 Oct 2020 07:48:50 -0400 Subject: [PATCH 2/2] sql: anonymize enum values ENUM values were improperly being represented as strings in the AST, which meant that they were not anonymized in telemetry. I created a new AST node type for them which respects anonymization. Fixes #55385 Release note: None --- pkg/sql/alter_type.go | 4 +-- pkg/sql/catalog/typedesc/type_desc.go | 4 +-- pkg/sql/crdb_internal.go | 4 +-- pkg/sql/create_type.go | 4 +-- pkg/sql/parser/sql.y | 25 ++++++++++-------- pkg/sql/rowenc/testutils.go | 6 ++--- pkg/sql/sem/tree/alter_type.go | 18 ++++++------- pkg/sql/sem/tree/create.go | 37 +++++++++++++++++++++------ pkg/sql/sem/tree/format_test.go | 8 ++++++ 9 files changed, 70 insertions(+), 40 deletions(-) diff --git a/pkg/sql/alter_type.go b/pkg/sql/alter_type.go index fcf0387aee2e..663eba4a7786 100644 --- a/pkg/sql/alter_type.go +++ b/pkg/sql/alter_type.go @@ -71,7 +71,7 @@ func (n *alterTypeNode) startExec(params runParams) error { case *tree.AlterTypeAddValue: err = params.p.addEnumValue(params.ctx, n, t) case *tree.AlterTypeRenameValue: - err = params.p.renameTypeValue(params.ctx, n, t.OldVal, t.NewVal) + err = params.p.renameTypeValue(params.ctx, n, string(t.OldVal), string(t.NewVal)) case *tree.AlterTypeRename: err = params.p.renameType(params.ctx, n, string(t.NewName)) case *tree.AlterTypeSetSchema: @@ -114,7 +114,7 @@ func (p *planner) addEnumValue( } // See if the value already exists in the enum or not. for _, member := range n.desc.EnumMembers { - if member.LogicalRepresentation == node.NewVal { + if member.LogicalRepresentation == string(node.NewVal) { if node.IfNotExists { p.BufferClientNotice( ctx, diff --git a/pkg/sql/catalog/typedesc/type_desc.go b/pkg/sql/catalog/typedesc/type_desc.go index b61f4a61949e..0a9fc09b4db4 100644 --- a/pkg/sql/catalog/typedesc/type_desc.go +++ b/pkg/sql/catalog/typedesc/type_desc.go @@ -301,7 +301,7 @@ func (desc *Mutable) AddEnumValue(node *tree.AlterTypeAddValue) error { // If the value was requested to be added before or after an existing // value, then find the index of where it should be inserted. foundIndex := -1 - existing := node.Placement.ExistingVal + existing := string(node.Placement.ExistingVal) for i, member := range desc.EnumMembers { if member.LogicalRepresentation == existing { foundIndex = i @@ -324,7 +324,7 @@ func (desc *Mutable) AddEnumValue(node *tree.AlterTypeAddValue) error { // how to decode the physical representation. newPhysicalRep := enum.GenByteStringBetween(getPhysicalRep(pos), getPhysicalRep(pos+1), enum.SpreadSpacing) newMember := descpb.TypeDescriptor_EnumMember{ - LogicalRepresentation: node.NewVal, + LogicalRepresentation: string(node.NewVal), PhysicalRepresentation: newPhysicalRep, Capability: descpb.TypeDescriptor_EnumMember_READ_ONLY, } diff --git a/pkg/sql/crdb_internal.go b/pkg/sql/crdb_internal.go index 81c5fadc2d05..8f6261af5caf 100644 --- a/pkg/sql/crdb_internal.go +++ b/pkg/sql/crdb_internal.go @@ -1668,11 +1668,11 @@ CREATE TABLE crdb_internal.create_type_statements ( return forEachTypeDesc(ctx, p, db, func(db *dbdesc.Immutable, sc string, typeDesc *typedesc.Immutable) error { switch typeDesc.Kind { case descpb.TypeDescriptor_ENUM: - var enumLabels []string + var enumLabels tree.EnumValueList enumLabelsDatum := tree.NewDArray(types.String) for i := range typeDesc.EnumMembers { rep := typeDesc.EnumMembers[i].LogicalRepresentation - enumLabels = append(enumLabels, rep) + enumLabels = append(enumLabels, tree.EnumValue(rep)) if err := enumLabelsDatum.Append(tree.NewDString(rep)); err != nil { return err } diff --git a/pkg/sql/create_type.go b/pkg/sql/create_type.go index 514ed66565da..790ea16a74e0 100644 --- a/pkg/sql/create_type.go +++ b/pkg/sql/create_type.go @@ -238,7 +238,7 @@ func (p *planner) createEnum(params runParams, n *tree.CreateType) error { sqltelemetry.IncrementEnumCounter(sqltelemetry.EnumCreate) // Ensure there are no duplicates in the input enum values. - seenVals := make(map[string]struct{}) + seenVals := make(map[tree.EnumValue]struct{}) for _, value := range n.EnumLabels { _, ok := seenVals[value] if ok { @@ -265,7 +265,7 @@ func (p *planner) createEnum(params runParams, n *tree.CreateType) error { physReps := enum.GenerateNEvenlySpacedBytes(len(n.EnumLabels)) for i := range n.EnumLabels { members[i] = descpb.TypeDescriptor_EnumMember{ - LogicalRepresentation: n.EnumLabels[i], + LogicalRepresentation: string(n.EnumLabels[i]), PhysicalRepresentation: physReps[i], Capability: descpb.TypeDescriptor_EnumMember_ALL, } diff --git a/pkg/sql/parser/sql.y b/pkg/sql/parser/sql.y index ffe29713ef4d..5f574dbbb4ea 100644 --- a/pkg/sql/parser/sql.y +++ b/pkg/sql/parser/sql.y @@ -156,6 +156,9 @@ func (u *sqlSymUnion) shardedIndexDef() *tree.ShardedIndexDef { func (u *sqlSymUnion) nameList() tree.NameList { return u.val.(tree.NameList) } +func (u *sqlSymUnion) enumValueList() tree.EnumValueList { + return u.val.(tree.EnumValueList) +} func (u *sqlSymUnion) unresolvedName() *tree.UnresolvedName { return u.val.(*tree.UnresolvedName) } @@ -2011,7 +2014,7 @@ alter_type_stmt: $$.val = &tree.AlterType{ Type: $3.unresolvedObjectName(), Cmd: &tree.AlterTypeAddValue{ - NewVal: $6, + NewVal: tree.EnumValue($6), IfNotExists: false, Placement: $7.alterTypeAddValuePlacement(), }, @@ -2022,7 +2025,7 @@ alter_type_stmt: $$.val = &tree.AlterType{ Type: $3.unresolvedObjectName(), Cmd: &tree.AlterTypeAddValue{ - NewVal: $9, + NewVal: tree.EnumValue($9), IfNotExists: true, Placement: $10.alterTypeAddValuePlacement(), }, @@ -2033,8 +2036,8 @@ alter_type_stmt: $$.val = &tree.AlterType{ Type: $3.unresolvedObjectName(), Cmd: &tree.AlterTypeRenameValue{ - OldVal: $6, - NewVal: $8, + OldVal: tree.EnumValue($6), + NewVal: tree.EnumValue($8), }, } } @@ -2080,14 +2083,14 @@ opt_add_val_placement: { $$.val = &tree.AlterTypeAddValuePlacement{ Before: true, - ExistingVal: $2, + ExistingVal: tree.EnumValue($2), } } | AFTER SCONST { $$.val = &tree.AlterTypeAddValuePlacement{ Before: false, - ExistingVal: $2, + ExistingVal: tree.EnumValue($2), } } | /* EMPTY */ @@ -6610,7 +6613,7 @@ create_type_stmt: $$.val = &tree.CreateType{ TypeName: $3.unresolvedObjectName(), Variety: tree.Enum, - EnumLabels: $7.strs(), + EnumLabels: $7.enumValueList(), } } | CREATE TYPE error // SHOW HELP: CREATE TYPE @@ -6628,21 +6631,21 @@ create_type_stmt: opt_enum_val_list: enum_val_list { - $$.val = $1.strs() + $$.val = $1.enumValueList() } | /* EMPTY */ { - $$.val = []string(nil) + $$.val = tree.EnumValueList(nil) } enum_val_list: SCONST { - $$.val = []string{$1} + $$.val = tree.EnumValueList{tree.EnumValue($1)} } | enum_val_list ',' SCONST { - $$.val = append($1.strs(), $3) + $$.val = append($1.enumValueList(), tree.EnumValue($3)) } // %Help: CREATE INDEX - create a new index diff --git a/pkg/sql/rowenc/testutils.go b/pkg/sql/rowenc/testutils.go index 134a31be6e66..d463ded70a94 100644 --- a/pkg/sql/rowenc/testutils.go +++ b/pkg/sql/rowenc/testutils.go @@ -1042,7 +1042,7 @@ type Mutator interface { func MakeSchemaName(ifNotExists bool, schema string, authRole string) *tree.CreateSchema { return &tree.CreateSchema{ IfNotExists: ifNotExists, - Schema: schema, + Schema: tree.Name(schema), AuthRole: authRole, } } @@ -1709,13 +1709,13 @@ func RandString(rng *rand.Rand, length int, alphabet string) string { // be random strings generated from alphabet. func RandCreateType(rng *rand.Rand, name, alphabet string) tree.Statement { numLabels := rng.Intn(6) + 1 - labels := make([]string, numLabels) + labels := make(tree.EnumValueList, numLabels) labelsMap := make(map[string]struct{}) i := 0 for i < numLabels { s := RandString(rng, rng.Intn(6)+1, alphabet) if _, ok := labelsMap[s]; !ok { - labels[i] = s + labels[i] = tree.EnumValue(s) labelsMap[s] = struct{}{} i++ } diff --git a/pkg/sql/sem/tree/alter_type.go b/pkg/sql/sem/tree/alter_type.go index 8cfecff1cb47..c022843cb0c3 100644 --- a/pkg/sql/sem/tree/alter_type.go +++ b/pkg/sql/sem/tree/alter_type.go @@ -12,7 +12,6 @@ package tree import ( "github.com/cockroachdb/cockroach/pkg/server/telemetry" - "github.com/cockroachdb/cockroach/pkg/sql/lex" "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" ) @@ -52,7 +51,7 @@ var _ AlterTypeCmd = &AlterTypeOwner{} // AlterTypeAddValue represents an ALTER TYPE ADD VALUE command. type AlterTypeAddValue struct { - NewVal string + NewVal EnumValue IfNotExists bool Placement *AlterTypeAddValuePlacement } @@ -63,14 +62,14 @@ func (node *AlterTypeAddValue) Format(ctx *FmtCtx) { if node.IfNotExists { ctx.WriteString("IF NOT EXISTS ") } - lex.EncodeSQLString(&ctx.Buffer, node.NewVal) + ctx.FormatNode(&node.NewVal) if node.Placement != nil { if node.Placement.Before { ctx.WriteString(" BEFORE ") } else { ctx.WriteString(" AFTER ") } - lex.EncodeSQLString(&ctx.Buffer, node.Placement.ExistingVal) + ctx.FormatNode(&node.Placement.ExistingVal) } } @@ -83,22 +82,21 @@ func (node *AlterTypeAddValue) TelemetryCounter() telemetry.Counter { // TYPE ADD VALUE command ([BEFORE | AFTER] value). type AlterTypeAddValuePlacement struct { Before bool - ExistingVal string + ExistingVal EnumValue } // AlterTypeRenameValue represents an ALTER TYPE RENAME VALUE command. type AlterTypeRenameValue struct { - // TODO? - OldVal string - NewVal string + OldVal EnumValue + NewVal EnumValue } // Format implements the NodeFormatter interface. func (node *AlterTypeRenameValue) Format(ctx *FmtCtx) { ctx.WriteString(" RENAME VALUE ") - lex.EncodeSQLString(&ctx.Buffer, node.OldVal) + ctx.FormatNode(&node.OldVal) ctx.WriteString(" TO ") - lex.EncodeSQLString(&ctx.Buffer, node.NewVal) + ctx.FormatNode(&node.NewVal) } // TelemetryCounter implements the AlterTypeCmd interface. diff --git a/pkg/sql/sem/tree/create.go b/pkg/sql/sem/tree/create.go index 7d423443148f..ac11be23e0fe 100644 --- a/pkg/sql/sem/tree/create.go +++ b/pkg/sql/sem/tree/create.go @@ -261,12 +261,38 @@ const ( Domain ) +// EnumValue represents a single enum value. +type EnumValue string + +// Format implements the NodeFormatter interface. +func (n *EnumValue) Format(ctx *FmtCtx) { + f := ctx.flags + if f.HasFlags(FmtAnonymize) { + ctx.WriteByte('_') + } else { + lex.EncodeSQLString(&ctx.Buffer, string(*n)) + } +} + +// EnumValueList represents a list of enum values. +type EnumValueList []EnumValue + +// Format implements the NodeFormatter interface. +func (l *EnumValueList) Format(ctx *FmtCtx) { + for i := range *l { + if i > 0 { + ctx.WriteString(", ") + } + ctx.FormatNode(&(*l)[i]) + } +} + // CreateType represents a CREATE TYPE statement. type CreateType struct { TypeName *UnresolvedObjectName Variety CreateTypeVariety // EnumLabels is set when this represents a CREATE TYPE ... AS ENUM statement. - EnumLabels []string + EnumLabels EnumValueList } var _ Statement = &CreateType{} @@ -274,17 +300,12 @@ var _ Statement = &CreateType{} // Format implements the NodeFormatter interface. func (node *CreateType) Format(ctx *FmtCtx) { ctx.WriteString("CREATE TYPE ") - ctx.WriteString(node.TypeName.String()) + ctx.FormatNode(node.TypeName) ctx.WriteString(" ") switch node.Variety { case Enum: ctx.WriteString("AS ENUM (") - for i := range node.EnumLabels { - if i > 0 { - ctx.WriteString(", ") - } - lex.EncodeSQLString(&ctx.Buffer, node.EnumLabels[i]) - } + ctx.FormatNode(&node.EnumLabels) ctx.WriteString(")") } } diff --git a/pkg/sql/sem/tree/format_test.go b/pkg/sql/sem/tree/format_test.go index b8dcf9a18a39..2f2ffc06f3d3 100644 --- a/pkg/sql/sem/tree/format_test.go +++ b/pkg/sql/sem/tree/format_test.go @@ -116,6 +116,14 @@ func TestFormatStatement(t *testing.T) { `ALTER DATABASE _ OWNER TO _`}, {`ALTER SCHEMA s OWNER TO o`, tree.FmtAnonymize, `ALTER SCHEMA _ OWNER TO _`}, + + // Test ENUM anonymization. + {`CREATE TYPE a AS ENUM ('a', 'b', 'c')`, tree.FmtAnonymize, + `CREATE TYPE _ AS ENUM (_, _, _)`}, + {`ALTER TYPE a ADD VALUE 'hi' BEFORE 'hello'`, tree.FmtAnonymize, + `ALTER TYPE _ ADD VALUE _ BEFORE _`}, + {`ALTER TYPE a RENAME VALUE 'value1' TO 'value2'`, tree.FmtAnonymize, + `ALTER TYPE _ RENAME VALUE _ TO _`}, } for i, test := range testData {