diff --git a/pkg/ccl/backupccl/testdata/backup-restore/show_backup b/pkg/ccl/backupccl/testdata/backup-restore/show_backup index 3328738e5b31..5811ee1f095a 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/show_backup +++ b/pkg/ccl/backupccl/testdata/backup-restore/show_backup @@ -4,6 +4,15 @@ new-cluster name=s1 allow-implicit-access localities=eu-central-1,eu-north-1,us-east-1 ---- +link-backup cluster=s1 src-path=show_backup_validate,invalidDependOnBy_21.1 dest-path=invalidDependOnBy_21.1 +---- + +# This backup intentionally has a dangling invalid depend on by reference. +query-sql regex=invalid\sdepended-on-by +SELECT * FROM [SHOW BACKUP VALIDATE FROM 'invalidDependOnBy_21.1' IN 'nodelocal://1/']; +---- +true + link-backup cluster=s1 src-path=show_backup_validate,valid-22.2 dest-path=valid-22.2 ---- diff --git a/pkg/ccl/backupccl/testdata/show_backup_validate/invalidDependOnBy_21.1/BACKUP_MANIFEST b/pkg/ccl/backupccl/testdata/show_backup_validate/invalidDependOnBy_21.1/BACKUP_MANIFEST new file mode 100644 index 000000000000..07ed4fc32828 Binary files /dev/null and b/pkg/ccl/backupccl/testdata/show_backup_validate/invalidDependOnBy_21.1/BACKUP_MANIFEST differ diff --git a/pkg/ccl/backupccl/testdata/show_backup_validate/invalidDependOnBy_21.1/BACKUP_MANIFEST-CHECKSUM b/pkg/ccl/backupccl/testdata/show_backup_validate/invalidDependOnBy_21.1/BACKUP_MANIFEST-CHECKSUM new file mode 100644 index 000000000000..70747a396825 --- /dev/null +++ b/pkg/ccl/backupccl/testdata/show_backup_validate/invalidDependOnBy_21.1/BACKUP_MANIFEST-CHECKSUM @@ -0,0 +1 @@ +'Üóÿ \ No newline at end of file diff --git a/pkg/sql/catalog/post_deserialization_changes.go b/pkg/sql/catalog/post_deserialization_changes.go index 191588f12491..5d6b51df449b 100644 --- a/pkg/sql/catalog/post_deserialization_changes.go +++ b/pkg/sql/catalog/post_deserialization_changes.go @@ -82,6 +82,10 @@ const ( // references. RemovedDuplicateIDsInRefs + // AddedConstraintIDs indicates that table descriptors had constraint ID + // added. + AddedConstraintIDs + // RemovedSelfEntryInSchemas corresponds to a change which occurred in // database descriptors to recover from an earlier bug whereby when // dropping a schema, we'd mark the database itself as though it was the diff --git a/pkg/sql/catalog/tabledesc/table_desc_builder.go b/pkg/sql/catalog/tabledesc/table_desc_builder.go index c168f5fdd346..6a72c2e8d1fa 100644 --- a/pkg/sql/catalog/tabledesc/table_desc_builder.go +++ b/pkg/sql/catalog/tabledesc/table_desc_builder.go @@ -178,6 +178,16 @@ func (tdb *tableDescriptorBuilder) RunRestoreChanges( tdb.changes.Add(catalog.UpgradedSequenceReference) } + // All backups taken in versions 22.1 and later should already have + // constraint IDs set. Technically, now that we are in v24.1, we no longer + // support restoring from versions older than 22.1, but we still have + // tests that restore from old versions. Until those fixtures are updated, + // we need to handle the case where constraint IDs are not set already. + // TODO(rafi): remove this once all fixtures are updated. See: https://github.com/cockroachdb/cockroach/issues/120146. + if changed := maybeAddConstraintIDs(tdb.maybeModified); changed { + tdb.changes.Add(catalog.AddedConstraintIDs) + } + // Upgrade the declarative schema changer state if scpb.MigrateDescriptorState(version, tdb.maybeModified.ParentID, tdb.maybeModified.DeclarativeSchemaChangerState) { tdb.changes.Add(catalog.UpgradedDeclarativeSchemaChangerState) @@ -778,6 +788,102 @@ func maybeFixPrimaryIndexEncoding(idx *descpb.IndexDescriptor) (hasChanged bool) return true } +// maybeAddConstraintIDs ensures that all constraints have an ID associated with +// them. +func maybeAddConstraintIDs(desc *descpb.TableDescriptor) (hasChanged bool) { + // Only assign constraint IDs to physical tables. + if !desc.IsTable() { + return false + } + // Collect pointers to constraint ID variables. + var idPtrs []*descpb.ConstraintID + if len(desc.PrimaryIndex.KeyColumnIDs) > 0 { + idPtrs = append(idPtrs, &desc.PrimaryIndex.ConstraintID) + } + for i := range desc.Indexes { + idx := &desc.Indexes[i] + if !idx.Unique || idx.UseDeletePreservingEncoding { + continue + } + idPtrs = append(idPtrs, &idx.ConstraintID) + } + checkByName := make(map[string]*descpb.TableDescriptor_CheckConstraint) + for i := range desc.Checks { + ck := desc.Checks[i] + idPtrs = append(idPtrs, &ck.ConstraintID) + checkByName[ck.Name] = ck + } + fkByName := make(map[string]*descpb.ForeignKeyConstraint) + for i := range desc.OutboundFKs { + fk := &desc.OutboundFKs[i] + idPtrs = append(idPtrs, &fk.ConstraintID) + fkByName[fk.Name] = fk + } + for i := range desc.InboundFKs { + idPtrs = append(idPtrs, &desc.InboundFKs[i].ConstraintID) + } + uwoiByName := make(map[string]*descpb.UniqueWithoutIndexConstraint) + for i := range desc.UniqueWithoutIndexConstraints { + uwoi := &desc.UniqueWithoutIndexConstraints[i] + idPtrs = append(idPtrs, &uwoi.ConstraintID) + uwoiByName[uwoi.Name] = uwoi + } + for _, m := range desc.GetMutations() { + if idx := m.GetIndex(); idx != nil && idx.Unique && !idx.UseDeletePreservingEncoding { + idPtrs = append(idPtrs, &idx.ConstraintID) + } else if c := m.GetConstraint(); c != nil { + switch c.ConstraintType { + case descpb.ConstraintToUpdate_CHECK, descpb.ConstraintToUpdate_NOT_NULL: + idPtrs = append(idPtrs, &c.Check.ConstraintID) + case descpb.ConstraintToUpdate_FOREIGN_KEY: + idPtrs = append(idPtrs, &c.ForeignKey.ConstraintID) + case descpb.ConstraintToUpdate_UNIQUE_WITHOUT_INDEX: + idPtrs = append(idPtrs, &c.UniqueWithoutIndexConstraint.ConstraintID) + } + } + } + // Set constraint ID counter to sane initial value. + var maxID descpb.ConstraintID + for _, p := range idPtrs { + if id := *p; id > maxID { + maxID = id + } + } + if desc.NextConstraintID <= maxID { + desc.NextConstraintID = maxID + 1 + hasChanged = true + } + // Update zero constraint IDs using counter. + for _, p := range idPtrs { + if *p != 0 { + continue + } + *p = desc.NextConstraintID + desc.NextConstraintID++ + hasChanged = true + } + // Reconcile constraint IDs between enforced slice and mutation. + for _, m := range desc.GetMutations() { + if c := m.GetConstraint(); c != nil { + switch c.ConstraintType { + case descpb.ConstraintToUpdate_CHECK, descpb.ConstraintToUpdate_NOT_NULL: + if other, ok := checkByName[c.Check.Name]; ok { + c.Check.ConstraintID = other.ConstraintID + } + case descpb.ConstraintToUpdate_FOREIGN_KEY: + if other, ok := fkByName[c.ForeignKey.Name]; ok { + c.ForeignKey.ConstraintID = other.ConstraintID + } + case descpb.ConstraintToUpdate_UNIQUE_WITHOUT_INDEX: + if other, ok := uwoiByName[c.UniqueWithoutIndexConstraint.Name]; ok { + c.UniqueWithoutIndexConstraint.ConstraintID = other.ConstraintID + } + } + } + } + return hasChanged +} + // maybeRemoveDuplicateIDsInRefs ensures that IDs in references to other tables // are not duplicated. func maybeRemoveDuplicateIDsInRefs(d *descpb.TableDescriptor) (hasChanged bool) {