sql/schemachanger: handle trigger dependents properly

Previously, trigger dependents used the NotImplementedForPublicObjects
operation to indicate that no operations were needed. Sadly because
triggers are not descriptors, this could lead to assertions in more
complex plans, when schema_locked is involved. To address this, this
patch modifies this operation to accept a trigger ID to detect if the
trigger is dropped.

Release note: None

Author: fqazi

pkg/sql/schemachanger/scexec/scmutationexec/scmutationexec.go

@@ -8,6 +8,7 @@ package scmutationexec
 import (
 	"context"
 
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop"
 	"github.com/cockroachdb/errors"
 )
@@ -55,6 +56,12 @@ func (i *immediateVisitor) NotImplementedForPublicObjects(
 	if desc == nil || desc.Dropped() {
 		return nil
 	}
+	if op.TriggerID != 0 {
+		// Validate that the trigger is dropped.
+		if catalog.FindTriggerByID(desc.(catalog.TableDescriptor), op.TriggerID) == nil {
+			return nil
+		}
+	}
 	return errors.AssertionFailedf("not implemented operation was hit "+
 		"unexpectedly, no dropped descriptor was found. %v", op.ElementType)
 }

pkg/sql/schemachanger/scop/immediate_mutation.go

@@ -38,6 +38,7 @@ type NotImplementedForPublicObjects struct {
 	immediateMutationOp
 	ElementType string
 	DescID      catid.DescID
+	TriggerID   catid.TriggerID
 }
 
 // UndoAllInTxnImmediateMutationOpSideEffects undoes the side effects of all

pkg/sql/schemachanger/scplan/internal/opgen/opgen_trigger_enabled.go

@@ -25,7 +25,7 @@ func init() {
 			scpb.Status_PUBLIC,
 			to(scpb.Status_ABSENT,
 				emit(func(this *scpb.TriggerEnabled) *scop.NotImplementedForPublicObjects {
-					return notImplementedForPublicObjects(this)
+					return notImplementedForPublicTriggers(this, this.TriggerID)
 				}),
 			),
 		),

pkg/sql/schemachanger/scplan/internal/opgen/opgen_trigger_events.go

@@ -25,7 +25,7 @@ func init() {
 			scpb.Status_PUBLIC,
 			to(scpb.Status_ABSENT,
 				emit(func(this *scpb.TriggerEvents) *scop.NotImplementedForPublicObjects {
-					return notImplementedForPublicObjects(this)
+					return notImplementedForPublicTriggers(this, this.TriggerID)
 				}),
 			),
 		),

pkg/sql/schemachanger/scplan/internal/opgen/opgen_trigger_function_call.go

@@ -27,7 +27,7 @@ func init() {
 			scpb.Status_PUBLIC,
 			to(scpb.Status_ABSENT,
 				emit(func(this *scpb.TriggerFunctionCall) *scop.NotImplementedForPublicObjects {
-					return notImplementedForPublicObjects(this)
+					return notImplementedForPublicTriggers(this, this.TriggerID)
 				}),
 			),
 		),

pkg/sql/schemachanger/scplan/internal/opgen/opgen_trigger_name.go

@@ -25,7 +25,7 @@ func init() {
 			scpb.Status_PUBLIC,
 			to(scpb.Status_ABSENT,
 				emit(func(this *scpb.TriggerName) *scop.NotImplementedForPublicObjects {
-					return notImplementedForPublicObjects(this)
+					return notImplementedForPublicTriggers(this, this.TriggerID)
 				}),
 			),
 		),

pkg/sql/schemachanger/scplan/internal/opgen/opgen_trigger_timing.go

@@ -25,7 +25,7 @@ func init() {
 			scpb.Status_PUBLIC,
 			to(scpb.Status_ABSENT,
 				emit(func(this *scpb.TriggerTiming) *scop.NotImplementedForPublicObjects {
-					return notImplementedForPublicObjects(this)
+					return notImplementedForPublicTriggers(this, this.TriggerID)
 				}),
 			),
 		),

pkg/sql/schemachanger/scplan/internal/opgen/opgen_trigger_transition.go

@@ -27,7 +27,7 @@ func init() {
 			scpb.Status_PUBLIC,
 			to(scpb.Status_ABSENT,
 				emit(func(this *scpb.TriggerTransition) *scop.NotImplementedForPublicObjects {
-					return notImplementedForPublicObjects(this)
+					return notImplementedForPublicTriggers(this, this.TriggerID)
 				}),
 			),
 		),

pkg/sql/schemachanger/scplan/internal/opgen/opgen_trigger_when.go

@@ -25,7 +25,7 @@ func init() {
 			scpb.Status_PUBLIC,
 			to(scpb.Status_ABSENT,
 				emit(func(this *scpb.TriggerWhen) *scop.NotImplementedForPublicObjects {
-					return notImplementedForPublicObjects(this)
+					return notImplementedForPublicTriggers(this, this.TriggerID)
 				}),
 			),
 		),

pkg/sql/schemachanger/scplan/internal/opgen/register.go

@@ -11,6 +11,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/screl"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/catid"
 	"github.com/cockroachdb/errors"
 )
 
@@ -32,6 +33,16 @@ func notImplementedForPublicObjects(e scpb.Element) *scop.NotImplementedForPubli
 	}
 }
 
+func notImplementedForPublicTriggers(
+	e scpb.Element, triggerID catid.TriggerID,
+) *scop.NotImplementedForPublicObjects {
+	return &scop.NotImplementedForPublicObjects{
+		ElementType: reflect.ValueOf(e).Type().Elem().String(),
+		DescID:      screl.GetDescID(e),
+		TriggerID:   triggerID,
+	}
+}
+
 func toPublic(initialStatus scpb.Status, specs ...transitionSpec) targetSpec {
 	return asTargetSpec(scpb.Status_PUBLIC, initialStatus, specs...)
 }

pkg/sql/schemachanger/scplan/testdata/drop_table

@@ -432,18 +432,23 @@ StatementPhase stage 1 of 1 with 141 MutationType ops
     *scop.NotImplementedForPublicObjects
       DescID: 109
       ElementType: scpb.TriggerName
+      TriggerID: 1
     *scop.NotImplementedForPublicObjects
       DescID: 109
       ElementType: scpb.TriggerEnabled
+      TriggerID: 1
     *scop.NotImplementedForPublicObjects
       DescID: 109
       ElementType: scpb.TriggerTiming
+      TriggerID: 1
     *scop.NotImplementedForPublicObjects
       DescID: 109
       ElementType: scpb.TriggerEvents
+      TriggerID: 1
     *scop.NotImplementedForPublicObjects
       DescID: 109
       ElementType: scpb.TriggerFunctionCall
+      TriggerID: 1
     *scop.UpdateTableBackReferencesInTypes
       BackReferencedTableID: 109
       TypeIDs:
@@ -1136,18 +1141,23 @@ PreCommitPhase stage 2 of 2 with 152 MutationType ops
     *scop.NotImplementedForPublicObjects
       DescID: 109
       ElementType: scpb.TriggerName
+      TriggerID: 1
     *scop.NotImplementedForPublicObjects
       DescID: 109
       ElementType: scpb.TriggerEnabled
+      TriggerID: 1
     *scop.NotImplementedForPublicObjects
       DescID: 109
       ElementType: scpb.TriggerTiming
+      TriggerID: 1
     *scop.NotImplementedForPublicObjects
       DescID: 109
       ElementType: scpb.TriggerEvents
+      TriggerID: 1
     *scop.NotImplementedForPublicObjects
       DescID: 109
       ElementType: scpb.TriggerFunctionCall
+      TriggerID: 1
     *scop.UpdateTableBackReferencesInTypes
       BackReferencedTableID: 109
       TypeIDs: