Skip to content

Commit

Permalink
sql: resolve error due to drop table after schema change in same txn
Browse files Browse the repository at this point in the history
Previously, if a drop table statement was executed in a transaction
following other schema changes to the table in the same transaction,
an error would occur. This error was due to the drop table statement
marking previous jobs as succeeded and then proceeding to modify them.
This change ensures that drop table statement will delete all existing
jobs from the job cache so that it does not interfere with previous jobs.

Release note (sql change): A table can successfully be dropped in
a transaction following other schema changes to the table in the
same transaction.
  • Loading branch information
jayshrivastava committed Nov 12, 2020
1 parent 791cae9 commit 7039ad8
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
11 changes: 10 additions & 1 deletion pkg/sql/drop_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,14 @@ func (p *planner) initiateDropTable(
tableDesc.DrainingNames = append(tableDesc.DrainingNames, nameDetails)
}

// Mark all jobs scheduled for schema changes as successful.
// For this table descriptor, mark all previous jobs scheduled for schema changes as successful
// and delete them from the schema change job cache.
//
// Since the table is being dropped, any previous schema changes to the table do not need to complete
// and can be put in a terminal state such as Succeeded. Deleting the jobs from the cache ensures that
// subsequent schema changes in the transaction (ie. this drop table statement) do not get a cache hit
// and do not try to update succeeded jobs, which would raise an error. Instead, this drop table
// statement will create a new job to drop the table.
jobIDs := make(map[int64]struct{})
var id descpb.MutationID
for _, m := range tableDesc.Mutations {
Expand All @@ -443,7 +450,9 @@ func (p *planner) initiateDropTable(
return errors.Wrapf(err,
"failed to mark job %d as as successful", errors.Safe(jobID))
}
delete(p.ExtendedEvalContext().SchemaChangeJobCache, tableDesc.ID)
}

// Initiate an immediate schema change. When dropping a table
// in a session, the data and the descriptor are not deleted.
// Instead, that is taken care of asynchronously by the schema
Expand Down
22 changes: 22 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/drop_table
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,25 @@ user testuser
# Being the owner of schema s should allow testuser to drop table s.t.
statement ok
DROP TABLE s.t

# Verify that a table can successfully be dropped after performing
# a schema change to the table in the same transaction.
# See https://github.com/cockroachdb/cockroach/issues/56235.
subtest drop_after_schema_change_in_txn
statement ok
CREATE TABLE to_drop();

statement ok
BEGIN;

statement ok
ALTER TABLE to_drop ADD COLUMN foo int;

statement ok
DROP TABLE to_drop;

statement ok
COMMIT;

statement error pgcode 42P01 relation "to_drop" does not exist
DROP TABLE to_drop;

0 comments on commit 7039ad8

Please sign in to comment.