sql/schemachanger: fix sequence dependency tracking for triggers

Previously, when the declarative schema changer was cleaning up
back-references for sequences, it ignored references that existed in
triggers. This could lead to scenarios where back-references were
incorrectly cleaned up. To address this, this patch updates the
back-reference update logic to include references from triggers.

Additionally, new build-only test validation has been added to detect
this scenario.

Fixes: #148103

Release note (bug fix): Fixed a bug where sequences could lose
references to triggers, allowing them to be dropped incorrectly.

Author: fqazi

pkg/ccl/logictestccl/testdata/logic_test/triggers

@@ -5081,3 +5081,40 @@ statement ok
 DROP FUNCTION trigger_func1, trigger_func2, trigger_func3;
 
 subtest end
+
+
+# Previously, we had a bug where we did not properly look at references
+# in a trigger when cleaning up back references on sequences. See
+# issue #148103
+subtest trigger_shared_backrefs
+
+statement ok
+CREATE SEQUENCE sc_tr_backref;
+
+statement ok
+CREATE TABLE sc_tr_tbl ();
+
+statement ok
+CREATE FUNCTION sc_tr_backref_f1() RETURNS TRIGGER AS $$
+BEGIN
+  SELECT nextval('sc_tr_backref');
+  RETURN NULL;
+END;
+$$ LANGUAGE PLpgSQL;
+
+statement ok
+CREATE TRIGGER tr1
+BEFORE INSERT OR UPDATE ON sc_tr_tbl
+FOR EACH ROW EXECUTE FUNCTION sc_tr_backref_f1();
+
+statement ok
+ALTER TABLE sc_tr_tbl
+  ADD CONSTRAINT ck1 CHECK (nextval('sc_tr_backref') < 1000);
+
+statement ok
+ALTER TABLE sc_tr_tbl DROP CONSTRAINT ck1;
+
+statement ok
+DROP TRIGGER tr1 ON sc_tr_tbl;
+
+subtest end

pkg/sql/catalog/BUILD.bazel

@@ -49,6 +49,7 @@ go_library(
         "//pkg/sql/types",
         "//pkg/sql/vecindex/vecpb",
         "//pkg/util",
+        "//pkg/util/buildutil",
         "//pkg/util/hlc",
         "//pkg/util/intsets",
         "//pkg/util/iterutil",

pkg/sql/catalog/funcdesc/func_desc.go

@@ -253,7 +253,7 @@ func (desc *immutable) ValidateForwardReferences(
 	}
 
 	for _, depID := range desc.DependsOn {
-		vea.Report(catalog.ValidateOutboundTableRef(depID, vdg))
+		vea.Report(catalog.ValidateOutboundTableRef(desc.GetID(), depID, vdg))
 	}
 
 	for _, typeID := range desc.DependsOnTypes {

pkg/sql/catalog/funcdesc/func_desc_test.go

@@ -341,7 +341,6 @@ func TestValidateFuncDesc(t *testing.T) {
 				DependedOnBy: []descpb.FunctionDescriptor_Reference{
 					{ID: tableID},
 				},
-				DependsOn:      []descpb.ID{tableID},
 				DependsOnTypes: []descpb.ID{typeID},
 			},
 		},

pkg/sql/catalog/tabledesc/validate.go

@@ -248,7 +248,7 @@ func (desc *wrapper) ValidateForwardReferences(
 	for i := range desc.Triggers {
 		trigger := &desc.Triggers[i]
 		for _, id := range trigger.DependsOn {
-			vea.Report(catalog.ValidateOutboundTableRef(id, vdg))
+			vea.Report(catalog.ValidateOutboundTableRef(desc.GetID(), id, vdg))
 		}
 		for _, id := range trigger.DependsOnTypes {
 			vea.Report(catalog.ValidateOutboundTypeRef(id, vdg))
@@ -264,7 +264,7 @@ func (desc *wrapper) ValidateForwardReferences(
 			vea.Report(catalog.ValidateOutboundTypeRef(id, vdg))
 		}
 		for _, id := range policy.DependsOnRelations {
-			vea.Report(catalog.ValidateOutboundTableRef(id, vdg))
+			vea.Report(catalog.ValidateOutboundTableRef(desc.GetID(), id, vdg))
 		}
 		for _, id := range policy.DependsOnFunctions {
 			vea.Report(catalog.ValidateOutboundFunctionRef(id, vdg))

pkg/sql/catalog/validate.go

@@ -13,6 +13,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
+	"github.com/cockroachdb/cockroach/pkg/util/buildutil"
 	"github.com/cockroachdb/errors"
 )
 
@@ -134,7 +135,7 @@ func ValidateOutboundFunctionRef(depID descpb.ID, vdg ValidationDescGetter) erro
 
 // ValidateOutboundTableRef validates outbound reference to relation descriptor
 // depID from descriptor selfID.
-func ValidateOutboundTableRef(depID descpb.ID, vdg ValidationDescGetter) error {
+func ValidateOutboundTableRef(srcDID descpb.ID, depID descpb.ID, vdg ValidationDescGetter) error {
 	referencedTable, err := vdg.GetTableDescriptor(depID)
 	if err != nil {
 		return errors.NewAssertionErrorWithWrappedErrf(err, "invalid depends-on relation reference")
@@ -143,6 +144,20 @@ func ValidateOutboundTableRef(depID descpb.ID, vdg ValidationDescGetter) error {
 		return errors.AssertionFailedf("depends-on relation %q (%d) is dropped",
 			referencedTable.GetName(), referencedTable.GetID())
 	}
+	// On debug builds validate that a back reference exists.
+	if buildutil.CrdbTestBuild {
+		foundReference := false
+		for _, dep := range referencedTable.GetDependedOnBy() {
+			if dep.ID == srcDID {
+				foundReference = true
+				break
+			}
+		}
+		if !foundReference {
+			return errors.AssertionFailedf("depends-on relation %q (%d) has no corresponding depended-on-by back reference",
+				referencedTable.GetName(), referencedTable.GetID())
+		}
+	}
 	return nil
 }
 

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

@@ -301,6 +301,12 @@ func (i *immediateVisitor) UpdateTableBackReferencesInSequences(
 					}
 				}
 			}
+			for _, t := range tbl.GetTriggers() {
+				// This contains all relation references from the trigger.
+				for _, rel := range t.DependsOn {
+					forwardRefs.Add(rel)
+				}
+			}
 		}
 	}
 	for _, seqID := range op.SequenceIDs {