diff --git a/pkg/sql/doctor/doctor.go b/pkg/sql/doctor/doctor.go index b918e01b6b5a..c24902a903fd 100644 --- a/pkg/sql/doctor/doctor.go +++ b/pkg/sql/doctor/doctor.go @@ -140,7 +140,7 @@ func ExamineDescriptors( continue } - for _, err := range catalog.Validate(ctx, descGetter, catalog.ValidationLevelSelfAndCrossReferences, desc).Errors() { + for _, err := range validateSafely(ctx, descGetter, desc) { problemsFound = true fmt.Fprint(stdout, reportMsg(desc, "%s", err)) } @@ -237,6 +237,23 @@ func ExamineDescriptors( return !problemsFound, err } +func validateSafely( + ctx context.Context, descGetter catalog.MapDescGetter, desc catalog.Descriptor, +) (errs []error) { + defer func() { + if r := recover(); r != nil { + err, ok := r.(error) + if !ok { + err = errors.Newf("%v", r) + } + err = errors.WithAssertionFailure(errors.Wrap(err, "validation")) + errs = append(errs, err) + } + }() + errs = append(errs, catalog.Validate(ctx, descGetter, catalog.ValidationLevelSelfAndCrossReferences, desc).Errors()...) + return errs +} + // ExamineJobs runs a suite of consistency checks over the system.jobs table. func ExamineJobs( ctx context.Context, diff --git a/pkg/sql/doctor/doctor_test.go b/pkg/sql/doctor/doctor_test.go index 9c33aa45b50d..8101482ec981 100644 --- a/pkg/sql/doctor/doctor_test.go +++ b/pkg/sql/doctor/doctor_test.go @@ -245,6 +245,46 @@ func TestExamineDescriptors(t *testing.T) { expected: `Examining 2 descriptors and 2 namespace entries... ParentID 3, ParentSchemaID 2: type "type" (1): referenced schema ID 2: descriptor not found ParentID 3, ParentSchemaID 2: type "type" (1): arrayTypeID 0 does not exist for "ENUM": referenced type ID 0: descriptor not found +`, + }, + { + descTable: doctor.DescriptorTable{ + { + ID: 51, + DescBytes: toBytes(t, &descpb.Descriptor{Union: &descpb.Descriptor_Database{ + Database: &descpb.DatabaseDescriptor{Name: "db", ID: 51}, + }}), + }, + { + ID: 52, + DescBytes: func() []byte { + // Skip `toBytes` to produce a descriptor with unset privileges field. + // The purpose of this is to produce a nil dereference during validation + // in order to test that doctor recovers from this. + // + // Note that it might be the case that validation aught to check that + // this field is not nil in the first place, in which case this test case + // will need to craft a corrupt descriptor serialization in a more + // creative way. Ideally validation code should never cause runtime errors + // but there's no way to guarantee that short of formally verifying it. We + // therefore have to consider the possibility of runtime errors (sadly) and + // doctor should absolutely make every possible effort to continue executing + // in the face of these, considering its main use case! + desc := &descpb.Descriptor{Union: &descpb.Descriptor_Type{ + Type: &descpb.TypeDescriptor{Name: "type", ID: 52, ParentID: 51, ParentSchemaID: keys.PublicSchemaID}, + }} + res, err := protoutil.Marshal(desc) + require.NoError(t, err) + return res + }(), + }, + }, + namespaceTable: doctor.NamespaceTable{ + {NameInfo: descpb.NameInfo{Name: "db"}, ID: 51}, + {NameInfo: descpb.NameInfo{ParentID: 51, ParentSchemaID: keys.PublicSchemaID, Name: "type"}, ID: 52}, + }, + expected: `Examining 2 descriptors and 2 namespace entries... + ParentID 51, ParentSchemaID 29: type "type" (52): validation: runtime error: invalid memory address or nil pointer dereference `, }, {