diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go index 9cdae7c79eef..18cf81c8b816 100644 --- a/pkg/sql/alter_table.go +++ b/pkg/sql/alter_table.go @@ -437,10 +437,17 @@ func (n *alterTableNode) startExec(params runParams) error { } jobDesc := fmt.Sprintf("removing view %q dependent on column %q which is being dropped", viewDesc.Name, colToDrop.ColName()) - droppedViews, err = params.p.removeDependentView(params.ctx, n.tableDesc, viewDesc, jobDesc) + cascadedViews, err := params.p.removeDependentView(params.ctx, n.tableDesc, viewDesc, jobDesc) if err != nil { return err } + qualifiedView, err := params.p.getQualifiedTableName(params.ctx, viewDesc) + if err != nil { + return err + } + + droppedViews = append(droppedViews, cascadedViews...) + droppedViews = append(droppedViews, qualifiedView.String()) } // We cannot remove this column if there are computed columns that use it. @@ -924,10 +931,8 @@ func (n *alterTableNode) startExec(params runParams) error { return params.p.logEvent(params.ctx, n.tableDesc.ID, &eventpb.AlterTable{ - TableName: params.p.ResolvedName(n.n.Table).FQString(), - MutationID: uint32(mutationID), - // TODO(knz): This is missing some qualification, see - // https://github.com/cockroachdb/cockroach/issues/57735 + TableName: params.p.ResolvedName(n.n.Table).FQString(), + MutationID: uint32(mutationID), CascadeDroppedViews: droppedViews, }) } diff --git a/pkg/sql/drop_index.go b/pkg/sql/drop_index.go index 79ba2d1f3310..89e42f1f2e93 100644 --- a/pkg/sql/drop_index.go +++ b/pkg/sql/drop_index.go @@ -424,7 +424,13 @@ func (p *planner) dropIndexByName( if err != nil { return err } - droppedViews = append(droppedViews, viewDesc.Name) + + qualifiedView, err := p.getQualifiedTableName(ctx, viewDesc) + if err != nil { + return err + } + + droppedViews = append(droppedViews, qualifiedView.String()) droppedViews = append(droppedViews, cascadedViews...) } } @@ -522,11 +528,9 @@ func (p *planner) dropIndexByName( return p.logEvent(ctx, tableDesc.ID, &eventpb.DropIndex{ - TableName: tn.FQString(), - IndexName: string(idxName), - MutationID: uint32(mutationID), - // TODO(knz): the dropped views are improperly qualified. - // See: https://github.com/cockroachdb/cockroach/issues/57735 + TableName: tn.FQString(), + IndexName: string(idxName), + MutationID: uint32(mutationID), CascadeDroppedViews: droppedViews, }) } diff --git a/pkg/sql/drop_table.go b/pkg/sql/drop_table.go index 609884ae1a85..5035eb60b866 100644 --- a/pkg/sql/drop_table.go +++ b/pkg/sql/drop_table.go @@ -143,9 +143,7 @@ func (n *dropTableNode) startExec(params runParams) error { if err := params.p.logEvent(params.ctx, droppedDesc.ID, &eventpb.DropTable{ - TableName: toDel.tn.FQString(), - // TODO(knz): the droppedViews are insufficiently qualified - // See: https://github.com/cockroachdb/cockroach/issues/57735 + TableName: toDel.tn.FQString(), CascadeDroppedViews: droppedViews, }); err != nil { return err @@ -348,8 +346,14 @@ func (p *planner) dropTableImpl( if err != nil { return droppedViews, err } + + qualifiedView, err := p.getQualifiedTableName(ctx, viewDesc) + if err != nil { + return droppedViews, err + } + droppedViews = append(droppedViews, cascadedViews...) - droppedViews = append(droppedViews, viewDesc.Name) + droppedViews = append(droppedViews, qualifiedView.String()) } err := p.removeTableComments(ctx, tableDesc) diff --git a/pkg/sql/drop_view.go b/pkg/sql/drop_view.go index 1a2b8ae8cb86..01a5ed59256b 100644 --- a/pkg/sql/drop_view.go +++ b/pkg/sql/drop_view.go @@ -223,12 +223,18 @@ func (p *planner) dropViewImpl( if err != nil { return cascadeDroppedViews, err } + + qualifiedView, err := p.getQualifiedTableName(ctx, dependentDesc) + if err != nil { + return cascadeDroppedViews, err + } + cascadedViews, err := p.dropViewImpl(ctx, dependentDesc, queueJob, "dropping dependent view", behavior) if err != nil { return cascadeDroppedViews, err } cascadeDroppedViews = append(cascadeDroppedViews, cascadedViews...) - cascadeDroppedViews = append(cascadeDroppedViews, dependentDesc.Name) + cascadeDroppedViews = append(cascadeDroppedViews, qualifiedView.String()) } } diff --git a/pkg/sql/logictest/testdata/logic_test/event_log b/pkg/sql/logictest/testdata/logic_test/event_log index 8182ef6b3210..eae8adabe418 100644 --- a/pkg/sql/logictest/testdata/logic_test/event_log +++ b/pkg/sql/logictest/testdata/logic_test/event_log @@ -949,3 +949,69 @@ FROM system.eventlog WHERE "eventType" = 'comment_on_table' ---- 1 {"Comment": "This is a table.", "EventType": "comment_on_table", "Statement": "COMMENT ON TABLE defaultdb.public.a IS 'This is a table.'", "TableName": "defaultdb.public.a", "User": "root"} + +# Test the event logs generated by commands that drop views. +subtest eventlog_dropped_views + +statement ok +CREATE TABLE x (a INT PRIMARY KEY, b INT) + +statement ok +CREATE VIEW y AS SELECT a FROM x + +statement ok +CREATE VIEW z AS SELECT b FROM x + +statement ok +DROP TABLE x CASCADE + +query IT +SELECT "reportingID", info::JSONB - 'Timestamp' - 'DescriptorID' +FROM system.eventlog +WHERE "eventType" = 'drop_table' +ORDER BY "timestamp" DESC, info +LIMIT 1 +---- +1 {"CascadeDroppedViews": ["defaultdb.public.y", "defaultdb.public.z"], "EventType": "drop_table", "Statement": "DROP TABLE defaultdb.public.x CASCADE", "TableName": "defaultdb.public.x", "User": "root"} + +statement ok +CREATE TABLE t (i INT PRIMARY KEY, INDEX (i)) + +statement ok +CREATE VIEW v AS (SELECT i FROM t@t_i_idx) + +statement ok +CREATE VIEW w AS (SELECT I FROM t@t_i_idx) + +statement ok +DROP INDEX t@t_i_idx CASCADE + +query IT +SELECT "reportingID", info::JSONB - 'Timestamp' - 'DescriptorID' +FROM system.eventlog +WHERE "eventType" = 'drop_index' +ORDER BY "timestamp" DESC, info +LIMIT 1 +---- +1 {"EventType": "drop_index", "IndexName": "t_i_idx", "MutationID": 1, "Statement": "DROP INDEX defaultdb.public.t@t_i_idx CASCADE", "TableName": "defaultdb.public.t", "User": "root", "cascade_dropped_views": ["defaultdb.public.v", "defaultdb.public.w"]} + +statement ok +CREATE TABLE x (a INT PRIMARY KEY, b INT) + +statement ok +CREATE VIEW v AS SELECT b FROM x + +statement ok +CREATE VIEW vv as SELECT b FROM x + +statement ok +ALTER TABLE x DROP COLUMN b CASCADE + +query IT +SELECT "reportingID", info::JSONB - 'Timestamp' - 'DescriptorID' +FROM system.eventlog +WHERE "eventType" = 'alter_table' +ORDER BY "timestamp" DESC, info +LIMIT 1 +---- +1 {"CascadeDroppedViews": ["defaultdb.public.v", "defaultdb.public.vv"], "EventType": "alter_table", "MutationID": 1, "Statement": "ALTER TABLE defaultdb.public.x DROP COLUMN b CASCADE", "TableName": "defaultdb.public.x", "User": "root"} diff --git a/pkg/sql/pgwire/testdata/pgtest/notice b/pkg/sql/pgwire/testdata/pgtest/notice index 774aba1a948f..c670417ea373 100644 --- a/pkg/sql/pgwire/testdata/pgtest/notice +++ b/pkg/sql/pgwire/testdata/pgtest/notice @@ -55,7 +55,7 @@ Query {"String": "DROP INDEX t_x_idx"} until crdb_only CommandComplete ---- -{"Severity":"NOTICE","Code":"00000","Message":"the data for dropped indexes is reclaimed asynchronously","Detail":"","Hint":"The reclamation delay can be customized in the zone configuration for the table.","Position":0,"InternalPosition":0,"InternalQuery":"","Where":"","SchemaName":"","TableName":"","ColumnName":"","DataTypeName":"","ConstraintName":"","File":"drop_index.go","Line":515,"Routine":"dropIndexByName","UnknownFields":null} +{"Severity":"NOTICE","Code":"00000","Message":"the data for dropped indexes is reclaimed asynchronously","Detail":"","Hint":"The reclamation delay can be customized in the zone configuration for the table.","Position":0,"InternalPosition":0,"InternalQuery":"","Where":"","SchemaName":"","TableName":"","ColumnName":"","DataTypeName":"","ConstraintName":"","File":"drop_index.go","Line":521,"Routine":"dropIndexByName","UnknownFields":null} {"Type":"CommandComplete","CommandTag":"DROP INDEX"} until noncrdb_only diff --git a/pkg/sql/resolver.go b/pkg/sql/resolver.go index dbeb77d8e771..2165d4de80b6 100644 --- a/pkg/sql/resolver.go +++ b/pkg/sql/resolver.go @@ -405,13 +405,19 @@ func (p *planner) getQualifiedTableName( ctx context.Context, desc catalog.TableDescriptor, ) (*tree.TableName, error) { dbDesc, err := p.Descriptors().GetImmutableDatabaseByID(ctx, p.txn, desc.GetParentID(), - tree.DatabaseLookupFlags{}) + tree.DatabaseLookupFlags{ + IncludeOffline: true, + IncludeDropped: true, + }) if err != nil { return nil, err } schemaID := desc.GetParentSchemaID() resolvedSchema, err := p.Descriptors().GetImmutableSchemaByID(ctx, p.txn, schemaID, - tree.SchemaLookupFlags{}) + tree.SchemaLookupFlags{ + IncludeOffline: true, + IncludeDropped: true, + }) if err != nil { return nil, err }