sql: DROP DATABASE CASCADE leaves tables which utilize sequences

[ajwerner]

Describe the problem

It seems that tables which utilize sequences are not properly dropped. It is interesting to note that this is only a problem if the sequence is also in the database being dropped. If the sequence is part of a different database, it will not be dropped.

The root cause of that issue seems to be filterCascadedTables, which seems to be designed for table-view / view-view dependency relationships so that we don't try to unnecessarily drop views that would be dropped in other table/view's drop cascade. This doesn't seem to play well with sequences, which don't drop the tables that depend on them during cascade (aside we currently don't support cascade for sequences).

cockroach/pkg/sql/drop_database.go

Lines 273 to 277 in deb468b

	// filterCascadedTables takes a list of table descriptors and removes any
	// descriptors from the list that are dependent on other descriptors in the
	// list (e.g. if view v1 depends on table t1, then v1 will be filtered from
	// the list).
	func (p *planner) filterCascadedTables(ctx context.Context, tables []toDelete) ([]toDelete, error) {

To Reproduce

Set up CockroachDB cluster using v20.1.3 or earlier in the 20.1 release and do the following:

CREATE DATABASE good_stuff;
CREATE TABLE good_stuff.data_to_backup ();

SET experimental_serial_normalization = 'sql_sequence';
CREATE DATABASE to_corrupt;
USE to_corrupt;
CREATE TABLE uses_sequence ( k SERIAL PRIMARY KEY );

USE good_stuff;
DROP DATABASE to_corrupt CASCADE;
BACKUP DATABASE good_stuff to 'nodelocal://1/this_wont_work';

ERROR: table "uses_sequence" has unknown ParentID 57

At this point we'll have failed to drop the table uses_sequence and manually repairing it is painful.

Expected behavior
The drop will work.

Additional context

It's not yet clear whether this bug affects earlier versions.

[blathers-crl]

Hi @ajwerner, I've guessed the C-ategory of your issue and suitably labeled it. Please re-label if inaccurate.

While you're here, please consider adding an A- label to help keep our repository tidy.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[ajwerner]

@arulajmani if you'd be willing to pick this up, that'd be great!

[knz]

Nice catch. I'm sorry I didn't catch this last time I was spelunking in that area.

[sitapriyanka]

Still this issue persists. I am facing this issue in my local after using drop database cascade.
cockroach-v20.2.0-alpha.2.windows-6.2-amd64
i am using wondows 10 enterprise.

[knz]

Arul can you interact with @sitapriyanka to get more data about the situation? Or maybe have support look into it? We probably want to iron this out thoroughly before the release.

[arulajmani]

I don't think this change made alpha2. The release issue's SHA is from 7/21 and the PR that fixed this issue went in on 7/24. I ran the steps listed in the issue description above and this is indeed fixed on master, so this should be fixed in alpha3.

@sitapriyanka could you please elaborate on the steps you took to run into this issue? I just wanna make sure you're not getting into a bad state even after the fix.

[solongordon]

@sitapriyanka I'm closing this for now but please comment if this is still an issue in alpha3.