Skip to content

Commit 17bda39

Browse files
committed
sql: validate trigger backreferences in table descriptors
Previously, descriptor validation only accounted for backreferences from sequences and views. However, the introduction of triggers introduces table-to-table references, which were not being validated. This change extends the validation logic to cover these cases. Fixes: #148167 Epic: CRDB-42942 Release note: None
1 parent 5e98537 commit 17bda39

File tree

2 files changed

+159
-37
lines changed

2 files changed

+159
-37
lines changed

pkg/sql/catalog/tabledesc/validate.go

Lines changed: 62 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -361,8 +361,6 @@ func (desc *wrapper) ValidateBackReferences(
361361
}
362362
switch depDesc.DescriptorType() {
363363
case catalog.Table:
364-
// If this is a table, it may be referenced by a view, otherwise if this
365-
// is a sequence, then it may be also be referenced by a table.
366364
vea.Report(desc.validateInboundTableRef(by, vdg))
367365
case catalog.Function:
368366
// This relation may be referenced by a function.
@@ -504,49 +502,76 @@ func (desc *wrapper) validateInboundTableRef(
504502
backReferencedTable.GetName(), backReferencedTable.GetID())
505503
}
506504
if desc.IsSequence() {
507-
// The ColumnIDs field takes a different meaning when the validated
508-
// descriptor is for a sequence. In this case, they refer to the columns
509-
// in the referenced descriptor instead.
510-
for _, colID := range by.ColumnIDs {
511-
// Skip this check if the column ID is zero. This can happen due to
512-
// bugs in 20.2.
513-
//
514-
// TODO(ajwerner): Make sure that a migration in 22.2 fixes this issue.
515-
if colID == 0 {
516-
continue
517-
}
518-
col := catalog.FindColumnByID(backReferencedTable, colID)
519-
if col == nil {
520-
return errors.AssertionFailedf("depended-on-by relation %q (%d) does not have a column with ID %d",
521-
backReferencedTable.GetName(), by.ID, colID)
505+
if err := validateSequenceColumnBackrefs(desc, backReferencedTable, by); err != nil {
506+
return err
507+
}
508+
}
509+
510+
// View back-references need corresponding forward reference.
511+
if backReferencedTable.IsView() {
512+
for _, id := range backReferencedTable.TableDesc().DependsOn {
513+
if id == desc.GetID() {
514+
return nil
522515
}
523-
var found bool
524-
for i := 0; i < col.NumUsesSequences(); i++ {
525-
if col.GetUsesSequenceID(i) == desc.GetID() {
526-
found = true
527-
break
516+
}
517+
return errors.AssertionFailedf("depended-on-by view %q (%d) has no corresponding depends-on forward reference",
518+
backReferencedTable.GetName(), by.ID)
519+
}
520+
521+
// Table to table back-references must have a trigger reference.
522+
if backReferencedTable.IsTable() && desc.IsTable() {
523+
for _, trigger := range backReferencedTable.TableDesc().Triggers {
524+
for _, id := range trigger.DependsOn {
525+
if id == desc.GetID() {
526+
return nil
528527
}
529528
}
530-
if found {
531-
continue
532-
}
533-
return errors.AssertionFailedf(
534-
"depended-on-by relation %q (%d) has no reference to this sequence in column %q (%d)",
535-
backReferencedTable.GetName(), by.ID, col.GetName(), col.GetID())
536529
}
537-
}
538530

539-
// View back-references need corresponding forward reference.
540-
if !backReferencedTable.IsView() {
541-
return nil
531+
// No valid forward reference found to justify the backref.
532+
return errors.AssertionFailedf(
533+
"table %q (%d) does not have a forward reference to descriptor %q (%d)",
534+
backReferencedTable.GetName(), by.ID, desc.GetName(), desc.GetID())
542535
}
543-
for _, id := range backReferencedTable.TableDesc().DependsOn {
544-
if id == desc.GetID() {
545-
return nil
536+
return nil
537+
}
538+
539+
func validateSequenceColumnBackrefs(
540+
seq catalog.Descriptor,
541+
backReferencedTable catalog.TableDescriptor,
542+
by descpb.TableDescriptor_Reference,
543+
) error {
544+
// The ColumnIDs field takes a different meaning when the validated
545+
// descriptor is for a sequence. In this case, they refer to the columns
546+
// in the referenced descriptor instead.
547+
for _, colID := range by.ColumnIDs {
548+
// Skip this check if the column ID is zero. This can happen due to
549+
// bugs in 20.2.
550+
//
551+
// TODO(ajwerner): Make sure that a migration in 22.2 fixes this issue.
552+
if colID == 0 {
553+
continue
546554
}
555+
col := catalog.FindColumnByID(backReferencedTable, colID)
556+
if col == nil {
557+
return errors.AssertionFailedf("depended-on-by relation %q (%d) does not have a column with ID %d",
558+
backReferencedTable.GetName(), by.ID, colID)
559+
}
560+
var found bool
561+
for i := 0; i < col.NumUsesSequences(); i++ {
562+
if col.GetUsesSequenceID(i) == seq.GetID() {
563+
found = true
564+
break
565+
}
566+
}
567+
if found {
568+
continue
569+
}
570+
return errors.AssertionFailedf(
571+
"depended-on-by relation %q (%d) has no reference to this sequence in column %q (%d)",
572+
backReferencedTable.GetName(), by.ID, col.GetName(), col.GetID())
547573
}
548-
return errors.AssertionFailedf("depended-on-by view %q (%d) has no corresponding depends-on forward reference",
549-
backReferencedTable.GetName(), by.ID)
574+
return nil
550575
}
551576

552577
// validateFK asserts that references to desc from inbound and outbound FKs are

pkg/sql/catalog/tabledesc/validate_test.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3956,6 +3956,103 @@ func TestValidateCrossTableReferences(t *testing.T) {
39563956
},
39573957
},
39583958
},
3959+
// 28
3960+
{
3961+
err: `depended-on-by relation "user_table_missing" (103) has no reference to this sequence in column "a" (1)`,
3962+
desc: descpb.TableDescriptor{
3963+
Name: "seq_broken",
3964+
ID: 102,
3965+
ParentID: 1,
3966+
UnexposedParentSchemaID: keys.PublicSchemaID,
3967+
SequenceOpts: &descpb.TableDescriptor_SequenceOpts{Increment: 1},
3968+
DependedOnBy: []descpb.TableDescriptor_Reference{
3969+
{ID: 103, ColumnIDs: []descpb.ColumnID{1}},
3970+
},
3971+
},
3972+
otherDescs: []descpb.TableDescriptor{{
3973+
Name: "user_table_missing",
3974+
ID: 103,
3975+
ParentID: 1,
3976+
UnexposedParentSchemaID: keys.PublicSchemaID,
3977+
Columns: []descpb.ColumnDescriptor{{
3978+
ID: 1,
3979+
Name: "a",
3980+
Type: types.Int,
3981+
}},
3982+
}},
3983+
},
3984+
// 29
3985+
{
3986+
err: `table "table_with_trigger" (103) does not have a forward reference to descriptor "table_ref_in_trigger" (102)`,
3987+
desc: descpb.TableDescriptor{
3988+
Name: "table_ref_in_trigger",
3989+
ID: 102,
3990+
ParentID: 1,
3991+
UnexposedParentSchemaID: keys.PublicSchemaID,
3992+
Columns: []descpb.ColumnDescriptor{{
3993+
ID: 1,
3994+
Name: "a",
3995+
Type: types.Int,
3996+
}},
3997+
DependedOnBy: []descpb.TableDescriptor_Reference{
3998+
{ID: 103},
3999+
},
4000+
},
4001+
otherDescs: []descpb.TableDescriptor{{
4002+
Name: "table_with_trigger",
4003+
ID: 103,
4004+
ParentID: 1,
4005+
UnexposedParentSchemaID: keys.PublicSchemaID,
4006+
Columns: []descpb.ColumnDescriptor{{
4007+
ID: 1,
4008+
Name: "a",
4009+
Type: types.Int,
4010+
}},
4011+
Triggers: []descpb.TriggerDescriptor{
4012+
{
4013+
ID: 1,
4014+
Name: "tr1",
4015+
DependsOn: []descpb.ID{}, // Forgot to put forward reference to table_ref_in_trigger
4016+
},
4017+
},
4018+
}},
4019+
},
4020+
// 30: like 29, but it does have a valid forward reference in a table
4021+
{
4022+
err: "",
4023+
desc: descpb.TableDescriptor{
4024+
Name: "table_ref_in_trigger",
4025+
ID: 102,
4026+
ParentID: 1,
4027+
UnexposedParentSchemaID: keys.PublicSchemaID,
4028+
Columns: []descpb.ColumnDescriptor{{
4029+
ID: 1,
4030+
Name: "a",
4031+
Type: types.Int,
4032+
}},
4033+
DependedOnBy: []descpb.TableDescriptor_Reference{
4034+
{ID: 103},
4035+
},
4036+
},
4037+
otherDescs: []descpb.TableDescriptor{{
4038+
Name: "table_with_trigger",
4039+
ID: 103,
4040+
ParentID: 1,
4041+
UnexposedParentSchemaID: keys.PublicSchemaID,
4042+
Columns: []descpb.ColumnDescriptor{{
4043+
ID: 1,
4044+
Name: "a",
4045+
Type: types.Int,
4046+
}},
4047+
Triggers: []descpb.TriggerDescriptor{
4048+
{
4049+
ID: 1,
4050+
Name: "tr1",
4051+
DependsOn: []descpb.ID{102},
4052+
},
4053+
},
4054+
}},
4055+
},
39594056
}
39604057

39614058
for i, test := range tests {

0 commit comments

Comments
 (0)