sql/schemachanger: block legacy schema changer operations earlier

Previously, it was possible for ADD/DROP CONSTRAINT and
other declarative schema changer operations to collide with
DROP TABLE in the legacy schema changer, since our gate
was not early enough. So, some work would be done before
the schema change was blocked with a retry error.
This patch prevents DROP TABLE/VIEW/SEQUENCE from
executing until a declarative schema change on such
objects is completed earlier.

Fixes: #99379

Release note: None

pkg/sql/drop_sequence.go

@@ -21,6 +21,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
+	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scerrors"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/cockroach/pkg/util/iterutil"
@@ -83,6 +84,12 @@ func (n *dropSequenceNode) startExec(params runParams) error {
 	ctx := params.ctx
 	for _, toDel := range n.td {
 		droppedDesc := toDel.desc
+		// Exit early with an error if the table is undergoing a declarative schema
+		// change, before we try to get job IDs and update job statuses later. See
+		// createOrUpdateSchemaChangeJob.
+		if catalog.HasConcurrentDeclarativeSchemaChange(droppedDesc) {
+			return scerrors.ConcurrentSchemaChangeError(droppedDesc)
+		}
 		err := params.p.dropSequenceImpl(
 			ctx, droppedDesc, true /* queueJob */, tree.AsStringWithFQNames(n.n, params.Ann()), n.n.DropBehavior,
 		)

pkg/sql/drop_table.go

@@ -229,7 +229,12 @@ func (p *planner) dropTableImpl(
 	behavior tree.DropBehavior,
 ) ([]string, error) {
 	var droppedViews []string
-
+	// Exit early with an error if the table is undergoing a declarative schema
+	// change, before we try to get job IDs and update job statuses later. See
+	// createOrUpdateSchemaChangeJob.
+	if catalog.HasConcurrentDeclarativeSchemaChange(tableDesc) {
+		return nil, scerrors.ConcurrentSchemaChangeError(tableDesc)
+	}
 	// Remove foreign key back references from tables that this table has foreign
 	// keys to.
 	// Copy out the set of outbound fks as it may be overwritten in the loop.
@@ -353,13 +358,6 @@ func (p *planner) initiateDropTable(
 		return errors.Errorf("table %q is already being dropped", tableDesc.Name)
 	}
 
-	// Exit early with an error if the table is undergoing a declarative schema
-	// change, before we try to get job IDs and update job statuses later. See
-	// createOrUpdateSchemaChangeJob.
-	if catalog.HasConcurrentDeclarativeSchemaChange(tableDesc) {
-		return scerrors.ConcurrentSchemaChangeError(tableDesc)
-	}
-
 	// Use the delayed GC mechanism to schedule usage of the more efficient
 	// ClearRange pathway.
 	if tableDesc.IsTable() {

pkg/sql/drop_view.go

@@ -20,6 +20,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/funcdesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
+	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scerrors"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -236,6 +237,12 @@ func (p *planner) dropViewImpl(
 ) ([]string, error) {
 	var cascadeDroppedViews []string
 
+	// Exit early with an error if the table is undergoing a declarative schema
+	// change, before we try to get job IDs and update job statuses later. See
+	// createOrUpdateSchemaChangeJob.
+	if catalog.HasConcurrentDeclarativeSchemaChange(viewDesc) {
+		return nil, scerrors.ConcurrentSchemaChangeError(viewDesc)
+	}
 	// Remove back-references from the tables/views this view depends on.
 	dependedOn := append([]descpb.ID(nil), viewDesc.DependsOn...)
 	for _, depID := range dependedOn {