diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go index 5a465fc44699..4a8e29d1c0c5 100644 --- a/pkg/sql/alter_table.go +++ b/pkg/sql/alter_table.go @@ -333,7 +333,7 @@ func (n *alterTableNode) startExec(params runParams) error { return err } - if err := params.p.dropSequencesOwnedByCol(params.ctx, colToDrop); err != nil { + if err := params.p.dropSequencesOwnedByCol(params.ctx, colToDrop, true /* queueJob */); err != nil { return err } diff --git a/pkg/sql/drop_table.go b/pkg/sql/drop_table.go index 22b616620230..095952992f17 100644 --- a/pkg/sql/drop_table.go +++ b/pkg/sql/drop_table.go @@ -291,7 +291,7 @@ func (p *planner) dropTableImpl( // Drop sequences that the columns of the table own for _, col := range tableDesc.Columns { - if err := p.dropSequencesOwnedByCol(ctx, &col); err != nil { + if err := p.dropSequencesOwnedByCol(ctx, &col, queueJob); err != nil { return droppedViews, err } } @@ -338,7 +338,7 @@ func (p *planner) initiateDropTable( drainName bool, ) error { if tableDesc.Dropped() { - return fmt.Errorf("table %q is being dropped", tableDesc.Name) + return errors.Errorf("table %q is already being dropped", tableDesc.Name) } // If the table is not interleaved , use the delayed GC mechanism to diff --git a/pkg/sql/logictest/testdata/logic_test/sequences b/pkg/sql/logictest/testdata/logic_test/sequences index 2bf2292603f0..46d72702117f 100644 --- a/pkg/sql/logictest/testdata/logic_test/sequences +++ b/pkg/sql/logictest/testdata/logic_test/sequences @@ -1,3 +1,4 @@ +# LogicTest: !3node-tenant(50840) # see also files `drop_sequence`, `alter_sequence`, `rename_sequence` # USING THE `lastval` FUNCTION @@ -1095,3 +1096,89 @@ DROP SEQUENCE seq_50649 statement ok DROP TABLE t_50649 + +subtest regression_50712 + +statement ok +CREATE DATABASE db_50712 + +statement ok +CREATE TABLE db_50712.t_50712(a INT PRIMARY KEY) + +statement ok +CREATE SEQUENCE db_50712.seq_50712 OWNED BY db_50712.t_50712.a + +statement ok +DROP DATABASE db_50712 CASCADE + +# Same test like above, except the table is lexicographically less than the +# sequence, which results in drop database dropping the table before the +# sequence. +statement ok +CREATE DATABASE db_50712 + +statement ok +CREATE TABLE db_50712.a_50712(a INT PRIMARY KEY) + +statement ok +CREATE SEQUENCE db_50712.seq_50712 OWNED BY db_50712.a_50712.a + +statement ok +DROP DATABASE db_50712 CASCADE + +# Same test like above, except the db is switched as the current db +statement ok +CREATE DATABASE db_50712 + +statement ok +SET DATABASE = db_50712 + +statement ok +CREATE TABLE a_50712(a INT PRIMARY KEY) + +statement ok +CREATE SEQUENCE seq_50712 OWNED BY a_50712.a + +statement ok +DROP DATABASE db_50712 + +statement ok +SET DATABASE = test + +# Tests db drop. +# Sequence: outside db. +# Owner: inside db. +# The sequence should be automatically dropped. +statement ok +CREATE DATABASE db_50712 + +statement ok +CREATE TABLE db_50712.t_50712(a INT PRIMARY KEY) + +statement ok +CREATE SEQUENCE seq_50712 OWNED BY db_50712.t_50712.a + +statement ok +DROP DATABASE db_50712 CASCADE + +statement error pq: relation "seq_50712" does not exist +SELECT * FROM seq_50712 + +# Tests db drop. +# Sequence: inside db +# Owner: outside db +# It should be possible to drop the table later. +statement ok +CREATE DATABASE db_50712 + +statement ok +CREATE TABLE t_50712(a INT PRIMARY KEY) + +statement ok +CREATE SEQUENCE db_50712.seq_50712 OWNED BY t_50712.a + +statement ok +DROP DATABASE db_50712 CASCADE + +statement ok +DROP TABLE t_50712 diff --git a/pkg/sql/sequence.go b/pkg/sql/sequence.go index 84868b7c40f9..6c86d6c4157a 100644 --- a/pkg/sql/sequence.go +++ b/pkg/sql/sequence.go @@ -335,6 +335,11 @@ func removeSequenceOwnerIfExists( if err != nil { return err } + // If the table descriptor has already been dropped, there is no need to + // remove the reference. + if tableDesc.Dropped() { + return nil + } col, err := tableDesc.FindColumnByID(opts.SequenceOwner.OwnerColumnID) if err != nil { return err @@ -469,17 +474,21 @@ func maybeAddSequenceDependencies( // dropSequencesOwnedByCol drops all the sequences from col.OwnsSequenceIDs. // Called when the respective column (or the whole table) is being dropped. func (p *planner) dropSequencesOwnedByCol( - ctx context.Context, col *sqlbase.ColumnDescriptor, + ctx context.Context, col *sqlbase.ColumnDescriptor, queueJob bool, ) error { for _, sequenceID := range col.OwnsSequenceIds { seqDesc, err := p.Tables().GetMutableTableVersionByID(ctx, sequenceID, p.txn) if err != nil { return err } + // This sequence is already getting dropped. Don't do it twice. + if seqDesc.Dropped() { + continue + } jobDesc := fmt.Sprintf("removing sequence %q dependent on column %q which is being dropped", seqDesc.Name, col.ColName()) if err := p.dropSequenceImpl( - ctx, seqDesc, true /* queueJob */, jobDesc, tree.DropRestrict, + ctx, seqDesc, queueJob, jobDesc, tree.DropRestrict, ); err != nil { return err } diff --git a/pkg/sql/table.go b/pkg/sql/table.go index 7339ca387bbd..650f34ad7952 100644 --- a/pkg/sql/table.go +++ b/pkg/sql/table.go @@ -12,7 +12,6 @@ package sql import ( "context" - "fmt" "strings" "github.com/cockroachdb/cockroach/pkg/clusterversion" @@ -209,7 +208,8 @@ func (p *planner) writeSchemaChange( } if tableDesc.Dropped() { // We don't allow schema changes on a dropped table. - return fmt.Errorf("table %q is being dropped", tableDesc.Name) + return errors.Errorf("no schema changes allowed on table %q as it is being dropped", + tableDesc.Name) } if err := p.createOrUpdateSchemaChangeJob(ctx, tableDesc, jobDesc, mutationID); err != nil { return err @@ -225,7 +225,8 @@ func (p *planner) writeSchemaChangeToBatch( } if tableDesc.Dropped() { // We don't allow schema changes on a dropped table. - return fmt.Errorf("table %q is being dropped", tableDesc.Name) + return errors.Errorf("no schema changes allowed on table %q as it is being dropped", + tableDesc.Name) } return p.writeTableDescToBatch(ctx, tableDesc, b) }