Skip to content

Commit 83a437d

Browse files
authored
Merge pull request #151593 from fqazi/blathers/backport-release-25.3-151546
release-25.3: sql/schemachanger: fix sequence dependency tracking for triggers
2 parents 2aff32c + 71e5c9a commit 83a437d

File tree

7 files changed

+63
-5
lines changed

7 files changed

+63
-5
lines changed

pkg/ccl/logictestccl/testdata/logic_test/triggers

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5077,3 +5077,40 @@ statement ok
50775077
DROP FUNCTION trigger_func1, trigger_func2, trigger_func3;
50785078

50795079
subtest end
5080+
5081+
5082+
# Previously, we had a bug where we did not properly look at references
5083+
# in a trigger when cleaning up back references on sequences. See
5084+
# issue #148103
5085+
subtest trigger_shared_backrefs
5086+
5087+
statement ok
5088+
CREATE SEQUENCE sc_tr_backref;
5089+
5090+
statement ok
5091+
CREATE TABLE sc_tr_tbl ();
5092+
5093+
statement ok
5094+
CREATE FUNCTION sc_tr_backref_f1() RETURNS TRIGGER AS $$
5095+
BEGIN
5096+
SELECT nextval('sc_tr_backref');
5097+
RETURN NULL;
5098+
END;
5099+
$$ LANGUAGE PLpgSQL;
5100+
5101+
statement ok
5102+
CREATE TRIGGER tr1
5103+
BEFORE INSERT OR UPDATE ON sc_tr_tbl
5104+
FOR EACH ROW EXECUTE FUNCTION sc_tr_backref_f1();
5105+
5106+
statement ok
5107+
ALTER TABLE sc_tr_tbl
5108+
ADD CONSTRAINT ck1 CHECK (nextval('sc_tr_backref') < 1000);
5109+
5110+
statement ok
5111+
ALTER TABLE sc_tr_tbl DROP CONSTRAINT ck1;
5112+
5113+
statement ok
5114+
DROP TRIGGER tr1 ON sc_tr_tbl;
5115+
5116+
subtest end

pkg/sql/catalog/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ go_library(
4848
"//pkg/sql/types",
4949
"//pkg/sql/vecindex/vecpb",
5050
"//pkg/util",
51+
"//pkg/util/buildutil",
5152
"//pkg/util/hlc",
5253
"//pkg/util/intsets",
5354
"//pkg/util/iterutil",

pkg/sql/catalog/funcdesc/func_desc.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ func (desc *immutable) ValidateForwardReferences(
253253
}
254254

255255
for _, depID := range desc.DependsOn {
256-
vea.Report(catalog.ValidateOutboundTableRef(depID, vdg))
256+
vea.Report(catalog.ValidateOutboundTableRef(desc.GetID(), depID, vdg))
257257
}
258258

259259
for _, typeID := range desc.DependsOnTypes {

pkg/sql/catalog/funcdesc/func_desc_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,6 @@ func TestValidateFuncDesc(t *testing.T) {
341341
DependedOnBy: []descpb.FunctionDescriptor_Reference{
342342
{ID: tableID},
343343
},
344-
DependsOn: []descpb.ID{tableID},
345344
DependsOnTypes: []descpb.ID{typeID},
346345
},
347346
},

pkg/sql/catalog/tabledesc/validate.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ func (desc *wrapper) ValidateForwardReferences(
248248
for i := range desc.Triggers {
249249
trigger := &desc.Triggers[i]
250250
for _, id := range trigger.DependsOn {
251-
vea.Report(catalog.ValidateOutboundTableRef(id, vdg))
251+
vea.Report(catalog.ValidateOutboundTableRef(desc.GetID(), id, vdg))
252252
}
253253
for _, id := range trigger.DependsOnTypes {
254254
vea.Report(catalog.ValidateOutboundTypeRef(id, vdg))
@@ -264,7 +264,7 @@ func (desc *wrapper) ValidateForwardReferences(
264264
vea.Report(catalog.ValidateOutboundTypeRef(id, vdg))
265265
}
266266
for _, id := range policy.DependsOnRelations {
267-
vea.Report(catalog.ValidateOutboundTableRef(id, vdg))
267+
vea.Report(catalog.ValidateOutboundTableRef(desc.GetID(), id, vdg))
268268
}
269269
for _, id := range policy.DependsOnFunctions {
270270
vea.Report(catalog.ValidateOutboundFunctionRef(id, vdg))

pkg/sql/catalog/validate.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/cockroachdb/cockroach/pkg/server/telemetry"
1414
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
1515
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
16+
"github.com/cockroachdb/cockroach/pkg/util/buildutil"
1617
"github.com/cockroachdb/errors"
1718
)
1819

@@ -134,7 +135,7 @@ func ValidateOutboundFunctionRef(depID descpb.ID, vdg ValidationDescGetter) erro
134135

135136
// ValidateOutboundTableRef validates outbound reference to relation descriptor
136137
// depID from descriptor selfID.
137-
func ValidateOutboundTableRef(depID descpb.ID, vdg ValidationDescGetter) error {
138+
func ValidateOutboundTableRef(srcDID descpb.ID, depID descpb.ID, vdg ValidationDescGetter) error {
138139
referencedTable, err := vdg.GetTableDescriptor(depID)
139140
if err != nil {
140141
return errors.NewAssertionErrorWithWrappedErrf(err, "invalid depends-on relation reference")
@@ -143,6 +144,20 @@ func ValidateOutboundTableRef(depID descpb.ID, vdg ValidationDescGetter) error {
143144
return errors.AssertionFailedf("depends-on relation %q (%d) is dropped",
144145
referencedTable.GetName(), referencedTable.GetID())
145146
}
147+
// On debug builds validate that a back reference exists.
148+
if buildutil.CrdbTestBuild {
149+
foundReference := false
150+
for _, dep := range referencedTable.GetDependedOnBy() {
151+
if dep.ID == srcDID {
152+
foundReference = true
153+
break
154+
}
155+
}
156+
if !foundReference {
157+
return errors.AssertionFailedf("depends-on relation %q (%d) has no corresponding depended-on-by back reference",
158+
referencedTable.GetName(), referencedTable.GetID())
159+
}
160+
}
146161
return nil
147162
}
148163

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,12 @@ func (i *immediateVisitor) UpdateTableBackReferencesInSequences(
301301
}
302302
}
303303
}
304+
for _, t := range tbl.GetTriggers() {
305+
// This contains all relation references from the trigger.
306+
for _, rel := range t.DependsOn {
307+
forwardRefs.Add(rel)
308+
}
309+
}
304310
}
305311
}
306312
for _, seqID := range op.SequenceIDs {

0 commit comments

Comments
 (0)