Skip to content

Commit

Permalink
sql: drop view cascade does not handle deps being dropped earlier
Browse files Browse the repository at this point in the history
Fixes: cockroachdb#68600

Previously, drop view cascade would only check if a dependency
was dropped when initially going over the list. It did not check
before executing the drop logic, so an earlier dependency could
cause the current object to move to a dropped state. This was
inadequate because cascaded drops of views could fail due to this
reason. To address this, this patch skips dropping dependencies
if at a later stage they are found to be dropped.

Release note (bug fix): Cascaded drop of views could run into
'table ...is already being dropped' errors incorrectly.
  • Loading branch information
fqazi authored and Sajjad Rizvi committed Aug 10, 2021
1 parent 7d6353e commit 99cb425
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 6 deletions.
15 changes: 9 additions & 6 deletions pkg/sql/drop_view.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,13 +236,16 @@ func (p *planner) dropViewImpl(
if err != nil {
return cascadeDroppedViews, err
}

cascadedViews, err := p.dropViewImpl(ctx, dependentDesc, queueJob, "dropping dependent view", behavior)
if err != nil {
return cascadeDroppedViews, err
// Check if the dependency was already marked as dropped,
// while dealing with any earlier dependent views.
if !dependentDesc.Dropped() {
cascadedViews, err := p.dropViewImpl(ctx, dependentDesc, queueJob, "dropping dependent view", behavior)
if err != nil {
return cascadeDroppedViews, err
}
cascadeDroppedViews = append(cascadeDroppedViews, cascadedViews...)
cascadeDroppedViews = append(cascadeDroppedViews, qualifiedView.FQString())
}
cascadeDroppedViews = append(cascadeDroppedViews, cascadedViews...)
cascadeDroppedViews = append(cascadeDroppedViews, qualifiedView.FQString())
}
}

Expand Down
27 changes: 27 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/views
Original file line number Diff line number Diff line change
Expand Up @@ -1202,3 +1202,30 @@ query T
SELECT * FROM v15
----
b

subtest view-cascade-nesting

statement ok
CREATE TABLE t1nest (id INT PRIMARY KEY, name varchar(256))

statement ok
CREATE VIEW v1nest AS (SELECT name FROM t1nest)

statement ok
CREATE VIEW v2nest AS (SELECT name AS n1, name AS n2 FROM v1nest)

statement ok
CREATE VIEW v3nest AS (SELECT name, n1 FROM v1nest, v2nest);

statement ok
DROP table t1nest CASCADE

# Validate the objects being dropped
query IT
SELECT "reportingID", info::JSONB - 'Timestamp' - 'DescriptorID'
FROM system.eventlog
WHERE "eventType" in ('drop_view', 'drop_table')
ORDER BY "timestamp" DESC, info
LIMIT 1
----
1 {"CascadeDroppedViews": ["db2.public.v3nest", "db2.public.v2nest", "db2.public.v1nest"], "EventType": "drop_table", "Statement": "DROP TABLE db2.public.t1nest CASCADE", "TableName": "db2.public.t1nest", "Tag": "DROP TABLE", "User": "root"}

0 comments on commit 99cb425

Please sign in to comment.