sql: dropping a sequence with a sequence owners corrupts the table that owns the sequence

[jordanlewis]

Repro:

root@:26257/defaultdb> create table a (a int primary key);
CREATE TABLE

Time: 49.454ms

root@:26257/defaultdb> create sequence foo owned by a.a;
CREATE SEQUENCE

Time: 162.234ms

root@:26257/defaultdb> drop sequence foo;
DROP SEQUENCE

Time: 251.705ms

root@:26257/defaultdb> drop table a;
ERROR: table is being dropped

It looks like there is no code to remove the sequence owner ID from the column descriptor that owns the sequence when the sequence is dropped.

[rohany]

Huh, @RichardJCai found this a bit ago -- #48249

[RichardJCai]

Huh, @RichardJCai found this a bit ago -- #48249

I didn't realize that it actually prevents dropping the table that owns the sequence after.

[thoszhang]

I'm reopening the issue to discuss how we want to get users' table descriptors out of this state, if they're already in it. This problem (dropped/nonexistent sequence descriptors) is a special case of #50651, but it seems worth bringing up separately.

Is there anything targeted we can do to avoid this failure if we hit a dropped/nonexistent sequence while dropping a column or table? Based on a very cursory look at the code, I was wondering if we can just check for ErrDescriptorNotFound and not return an error in one of these places (not sure which one is the problem):

cockroach/pkg/sql/drop_sequence.go

Line 161 in fe78658

seqLookup, err := p.LookupTableByID(ctx, sequenceID)

cockroach/pkg/sql/sequence.go

Line 475 in fe78658

seqDesc, err := p.Tables().GetMutableTableVersionByID(ctx, sequenceID, p.txn)

[ajwerner]

We've got a related and big problem which is sort of the opposite. Dropping a database which has tables which utilizes sequences corrupts, well, everything. I'll be creating another issue to track that but it's bad.

[arulajmani]

I looked into the issue of dropping databases which have tables that utilize sequences and it seems like the table that uses the sequence isn't getting dropped. I don't think that problem is related to this issue though, as there is no "ownership" in that case.

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).

I think we can fix this by ensuring filterCascadedTables skips sequences when accumulating dependencies, so that it returns both the table descriptor for the sequence that uses it and the sequence itself so that they both can be cleaned up. Feel free to assign me the issue, I'm happy to take it on!

[ajwerner]

@arulajmani thanks for taking a look last week. I've written the issue up as #50997 and tentatively assigned it to you. Let me know if you run into any trouble or would like me to look.

[arulajmani]

I've opened #51170 for discussion about how to get users' table descriptors out of invalid states if they're already in it. Fixes for a few issues could get users' into invalid states and I've tagged all of them in the issue above.

Closing this.