From 7039ad815700733314a728fc68f83d2741deeb55 Mon Sep 17 00:00:00 2001
From: Jayant Shrivastava <jayants@cockroachlabs.com>
Date: Wed, 11 Nov 2020 17:04:33 -0500
Subject: [PATCH] sql: resolve error due to drop table after schema change in
 same txn

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.
---
 pkg/sql/drop_table.go                         | 11 +++++++++-
 .../logictest/testdata/logic_test/drop_table  | 22 +++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/pkg/sql/drop_table.go b/pkg/sql/drop_table.go
index c927f80773bf..32d2427f2b24 100644
--- a/pkg/sql/drop_table.go
+++ b/pkg/sql/drop_table.go
@@ -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 {
@@ -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
diff --git a/pkg/sql/logictest/testdata/logic_test/drop_table b/pkg/sql/logictest/testdata/logic_test/drop_table
index c30f0f5402b9..0285a5b688a9 100644
--- a/pkg/sql/logictest/testdata/logic_test/drop_table
+++ b/pkg/sql/logictest/testdata/logic_test/drop_table
@@ -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;