Skip to content

Commit

Permalink
Merge #58257
Browse files Browse the repository at this point in the history 
58257: sql: ensure event log entries for ALTER/DROP TYPE are fully qualified r=knz a=rohany

Fixes #57734.

Release note (bug fix): Fully qualified names in the `TypeName` field of
the event log for `ALTER TYPE` and `DROP TYPE` statements.

Co-authored-by: Rohan Yadav <[email protected]>
  • Loading branch information
@rohany
craig[bot] and rohany committed Jan 5, 2021
2 parents 6de6fca + 9db726a commit 2d87f9f
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 12 deletions.
7 changes: 3 additions & 4 deletions pkg/sql/alter_type.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ func (p *planner) AlterType(ctx context.Context, n *tree.AlterType) (planNode, e
func (n *alterTypeNode) startExec(params runParams) error {
telemetry.Inc(n.n.Cmd.TelemetryCounter())

typeName := tree.AsStringWithFQNames(n.n.Type, params.p.Ann())
eventLogDone := false
var err error
switch t := n.n.Cmd.(type) {
Expand All @@ -97,9 +98,7 @@ func (n *alterTypeNode) startExec(params runParams) error {
return err
}
err = params.p.logEvent(params.ctx, n.desc.ID, &eventpb.RenameType{
// TODO(knz): This name is insufficiently qualified.
// See: https://github.com/cockroachdb/cockroach/issues/57734
TypeName: n.desc.Name,
TypeName: typeName,
NewTypeName: string(t.NewName),
})
eventLogDone = true
Expand Down Expand Up @@ -130,7 +129,7 @@ func (n *alterTypeNode) startExec(params runParams) error {
if err := params.p.logEvent(params.ctx,
n.desc.ID,
&eventpb.AlterType{
TypeName: n.desc.Name,
TypeName: typeName,
}); err != nil {
return err
}
Expand Down
33 changes: 25 additions & 8 deletions pkg/sql/drop_type.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,14 @@ import (
"github.com/cockroachdb/errors"
)

type typeToDrop struct {
desc *typedesc.Mutable
fqName string
}

type dropTypeNode struct {
n *tree.DropType
td map[descpb.ID]*typedesc.Mutable
td map[descpb.ID]typeToDrop
}

// Use to satisfy the linter.
Expand All @@ -47,7 +52,7 @@ func (p *planner) DropType(ctx context.Context, n *tree.DropType) (planNode, err

node := &dropTypeNode{
n: n,
td: make(map[descpb.ID]*typedesc.Mutable),
td: make(map[descpb.ID]typeToDrop),
}
if n.DropBehavior == tree.DropCascade {
return nil, unimplemented.NewWithIssue(51480, "DROP TYPE CASCADE is not yet supported")
Expand Down Expand Up @@ -102,8 +107,21 @@ func (p *planner) DropType(ctx context.Context, n *tree.DropType) (planNode, err
}

// Record these descriptors for deletion.
node.td[typeDesc.ID] = typeDesc
node.td[mutArrayDesc.ID] = mutArrayDesc
node.td[typeDesc.ID] = typeToDrop{
desc: typeDesc,
fqName: tree.AsStringWithFQNames(name, p.Ann()),
}
arrayFQName, err := getTypeNameFromTypeDescriptor(
oneAtATimeSchemaResolver{ctx, p},
mutArrayDesc,
)
if err != nil {
return nil, err
}
node.td[mutArrayDesc.ID] = typeToDrop{
desc: mutArrayDesc,
fqName: arrayFQName.FQString(),
}
}
return node, nil
}
Expand Down Expand Up @@ -138,14 +156,13 @@ func (p *planner) canDropTypeDesc(
}

func (n *dropTypeNode) startExec(params runParams) error {
for _, typ := range n.td {
for _, toDrop := range n.td {
typ, fqName := toDrop.desc, toDrop.fqName
if err := params.p.dropTypeImpl(params.ctx, typ, tree.AsStringWithFQNames(n.n, params.Ann()), true /* queueJob */); err != nil {
return err
}
// Log a Drop Type event.
// TODO(knz): This logging is imperfect, see this issue:
// https://github.com/cockroachdb/cockroach/issues/57734
if err := params.p.logEvent(params.ctx, typ.ID, &eventpb.DropType{TypeName: typ.Name}); err != nil {
if err := params.p.logEvent(params.ctx, typ.ID, &eventpb.DropType{TypeName: fqName}); err != nil {
return err
}
}
Expand Down
55 changes: 55 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/event_log
View file
Original file line number Diff line number Diff line change
Expand Up @@ -854,3 +854,58 @@ statement ok
DROP DATABASE atest CASCADE;
DROP USER v;
DROP USER u

# Regression for #57734. Ensure that type names are fully qualified in the
# event log.
subtest regression_57734

statement ok
CREATE TYPE eventlog AS ENUM ('event', 'log')

query ITT
SELECT "reportingID", "eventType", info::JSONB - 'Timestamp' - 'DescriptorID'
FROM system.eventlog
WHERE "eventType" = 'create_type' AND info::JSONB->>'TypeName' LIKE '%eventlog'
ORDER BY "timestamp", info
----
1 create_type {"EventType": "create_type", "Statement": "CREATE TYPE defaultdb.public.eventlog AS ENUM ('event', 'log')", "TypeName": "defaultdb.public.eventlog", "User": "root"}

statement ok
ALTER TYPE eventlog ADD VALUE 'test'

statement ok
ALTER TYPE eventlog RENAME VALUE 'test' TO 'testing'

statement ok
CREATE SCHEMA testing

statement ok
ALTER TYPE eventlog SET SCHEMA testing;
ALTER TYPE testing.eventlog SET SCHEMA public

statement ok
ALTER TYPE eventlog RENAME TO eventlog_renamed

query ITT
SELECT "reportingID", "eventType", info::JSONB - 'Timestamp' - 'DescriptorID'
FROM system.eventlog
WHERE ("eventType" = 'alter_type' OR "eventType" = 'rename_type') AND info::JSONB->>'TypeName' LIKE '%eventlog%'
ORDER BY "timestamp", info
----
1 alter_type {"EventType": "alter_type", "Statement": "ALTER TYPE defaultdb.public.eventlog ADD VALUE 'test'", "TypeName": "defaultdb.public.eventlog", "User": "root"}
1 alter_type {"EventType": "alter_type", "Statement": "ALTER TYPE defaultdb.public.eventlog RENAME VALUE 'test' TO 'testing'", "TypeName": "defaultdb.public.eventlog", "User": "root"}
1 alter_type {"EventType": "alter_type", "Statement": "ALTER TYPE defaultdb.public.eventlog SET SCHEMA testing", "TypeName": "defaultdb.public.eventlog", "User": "root"}
1 alter_type {"EventType": "alter_type", "Statement": "ALTER TYPE defaultdb.testing.eventlog SET SCHEMA public", "TypeName": "defaultdb.testing.eventlog", "User": "root"}
1 rename_type {"EventType": "rename_type", "NewTypeName": "eventlog_renamed", "Statement": "ALTER TYPE defaultdb.public.eventlog RENAME TO eventlog_renamed", "TypeName": "defaultdb.public.eventlog", "User": "root"}

statement ok
DROP TYPE eventlog_renamed

query ITT
SELECT "reportingID", "eventType", info::JSONB - 'Timestamp' - 'DescriptorID'
FROM system.eventlog
WHERE "eventType" = 'drop_type' AND info::JSONB->>'TypeName' LIKE '%eventlog%'
ORDER BY "timestamp", info
----
1 drop_type {"EventType": "drop_type", "Statement": "DROP TYPE defaultdb.public.eventlog_renamed", "TypeName": "defaultdb.public.eventlog_renamed", "User": "root"}
1 drop_type {"EventType": "drop_type", "Statement": "DROP TYPE defaultdb.public.eventlog_renamed", "TypeName": "defaultdb.public._eventlog_renamed", "User": "root"}

0 comments on commit 2d87f9f

Please sign in to comment.