From 2c2b6cee2ab81d13f9d09cc750212e43b75ea798 Mon Sep 17 00:00:00 2001 From: Marius Posta Date: Thu, 15 Dec 2022 12:57:19 -0500 Subject: [PATCH] nstree: fix Catalog namespace validation error messages These mention draining names, but draining names have been removed two major versions ago. Fixes #93724. Release note: None --- pkg/cli/testdata/doctor/test_examine_zipdir | 2 +- .../testdata/doctor/test_examine_zipdir_verbose | 2 +- pkg/sql/catalog/nstree/catalog.go | 14 ++++++-------- pkg/sql/crdb_internal_test.go | 6 +++--- pkg/sql/doctor/doctor_test.go | 8 ++++---- .../logictest/testdata/logic_test/crdb_internal | 8 ++++---- ...ion_before_starting_an_upgrade_external_test.go | 2 +- 7 files changed, 20 insertions(+), 22 deletions(-) diff --git a/pkg/cli/testdata/doctor/test_examine_zipdir b/pkg/cli/testdata/doctor/test_examine_zipdir index 2d54d445e6f5..06cb44526222 100644 --- a/pkg/cli/testdata/doctor/test_examine_zipdir +++ b/pkg/cli/testdata/doctor/test_examine_zipdir @@ -9,7 +9,7 @@ Examining 37 descriptors and 42 namespace entries... ParentID 52, ParentSchemaID 29: relation "vehicle_location_histories" (56): referenced database ID 52: referenced descriptor not found ParentID 52, ParentSchemaID 29: relation "promo_codes" (57): referenced database ID 52: referenced descriptor not found ParentID 52, ParentSchemaID 29: relation "user_promo_codes" (58): referenced database ID 52: referenced descriptor not found - ParentID 0, ParentSchemaID 0: namespace entry "movr" (52): descriptor not found + ParentID 0, ParentSchemaID 0: namespace entry "movr" (52): referenced descriptor not found Examining 2 jobs... job 587337426984566785: running schema change GC refers to missing table descriptor(s) [59]; existing descriptors that still need to be dropped []; job safe to delete: true. ERROR: validation failed diff --git a/pkg/cli/testdata/doctor/test_examine_zipdir_verbose b/pkg/cli/testdata/doctor/test_examine_zipdir_verbose index 90fdfb665e90..ba62a211e7ab 100644 --- a/pkg/cli/testdata/doctor/test_examine_zipdir_verbose +++ b/pkg/cli/testdata/doctor/test_examine_zipdir_verbose @@ -50,7 +50,7 @@ Examining 37 descriptors and 42 namespace entries... ParentID 52, ParentSchemaID 29: relation "user_promo_codes" (58): referenced database ID 52: referenced descriptor not found ParentID 52, ParentSchemaID 29: relation "user_promo_codes" (58): processed ParentID 0, ParentSchemaID 0: namespace entry "defaultdb" (50): processed - ParentID 0, ParentSchemaID 0: namespace entry "movr" (52): descriptor not found + ParentID 0, ParentSchemaID 0: namespace entry "movr" (52): referenced descriptor not found ParentID 0, ParentSchemaID 0: namespace entry "postgres" (51): processed ParentID 0, ParentSchemaID 0: namespace entry "system" (1): processed ParentID 1, ParentSchemaID 0: namespace entry "public" (29): processed diff --git a/pkg/sql/catalog/nstree/catalog.go b/pkg/sql/catalog/nstree/catalog.go index 459efadae6e4..6c932cd269d4 100644 --- a/pkg/sql/catalog/nstree/catalog.go +++ b/pkg/sql/catalog/nstree/catalog.go @@ -252,14 +252,14 @@ func (c Catalog) Validate( func (c Catalog) ValidateNamespaceEntry(key catalog.NameKey) error { ne := c.LookupNamespaceEntry(key) if ne == nil { - return errors.New("invalid descriptor ID") + return errors.AssertionFailedf("invalid namespace entry") } // Handle special cases. switch ne.GetID() { case descpb.InvalidID: return errors.New("invalid descriptor ID") - case keys.PublicSchemaID: - // The public schema doesn't have a descriptor. + case keys.SystemPublicSchemaID: + // The public schema for the system database doesn't have a descriptor. return nil default: isSchema := ne.GetParentID() != keys.RootNamespaceID && ne.GetParentSchemaID() == keys.RootNamespaceID @@ -271,19 +271,17 @@ func (c Catalog) ValidateNamespaceEntry(key catalog.NameKey) error { // Compare the namespace entry with the referenced descriptor. desc := c.LookupDescriptor(ne.GetID()) if desc == nil { - return catalog.ErrDescriptorNotFound + return catalog.ErrReferencedDescriptorNotFound } if desc.Dropped() { - return errors.Newf("no matching name info in draining names of dropped %s", - desc.DescriptorType()) + return catalog.ErrDescriptorDropped } if ne.GetParentID() == desc.GetParentID() && ne.GetParentSchemaID() == desc.GetParentSchemaID() && ne.GetName() == desc.GetName() { return nil } - return errors.Newf("no matching name info found in non-dropped %s %q", - desc.DescriptorType(), desc.GetName()) + return errors.Newf("mismatched name %q in %s descriptor", desc.GetName(), desc.DescriptorType()) } // ValidateWithRecover is like Validate but which recovers from panics. diff --git a/pkg/sql/crdb_internal_test.go b/pkg/sql/crdb_internal_test.go index 769151b23794..43d648dcaaf1 100644 --- a/pkg/sql/crdb_internal_test.go +++ b/pkg/sql/crdb_internal_test.go @@ -499,9 +499,9 @@ UPDATE system.namespace SET id = %d WHERE id = %d; {fmt.Sprintf("%d", schemaID), fmt.Sprintf("[%d]", databaseID), "public", "", fmt.Sprintf(`schema "public" (%d): referenced database ID %d: referenced descriptor not found`, schemaID, databaseID), }, - {fmt.Sprintf("%d", databaseID), "t", "", "", `descriptor not found`}, - {fmt.Sprintf("%d", tableFkTblID), "defaultdb", "public", "fktbl", `descriptor not found`}, - {fmt.Sprintf("%d", fakeID), fmt.Sprintf("[%d]", databaseID), "public", "test", `descriptor not found`}, + {fmt.Sprintf("%d", databaseID), "t", "", "", `referenced descriptor not found`}, + {fmt.Sprintf("%d", tableFkTblID), "defaultdb", "public", "fktbl", `referenced descriptor not found`}, + {fmt.Sprintf("%d", fakeID), fmt.Sprintf("[%d]", databaseID), "public", "test", `referenced descriptor not found`}, }) } diff --git a/pkg/sql/doctor/doctor_test.go b/pkg/sql/doctor/doctor_test.go index 8a8cc13d08f9..7be8951277bf 100644 --- a/pkg/sql/doctor/doctor_test.go +++ b/pkg/sql/doctor/doctor_test.go @@ -243,7 +243,7 @@ func TestExamineDescriptors(t *testing.T) { }, expected: `Examining 2 descriptors and 2 namespace entries... ParentID 52, ParentSchemaID 29: relation "t" (51): expected matching namespace entry, found none - ParentID 0, ParentSchemaID 29: namespace entry "t" (51): no matching name info found in non-dropped relation "t" + ParentID 0, ParentSchemaID 29: namespace entry "t" (51): mismatched name "t" in relation descriptor `, }, { // 8 @@ -372,7 +372,7 @@ func TestExamineDescriptors(t *testing.T) { {NameInfo: descpb.NameInfo{Name: "causes_error"}, ID: 2}, }, expected: `Examining 0 descriptors and 4 namespace entries... - ParentID 0, ParentSchemaID 0: namespace entry "causes_error" (2): descriptor not found + ParentID 0, ParentSchemaID 0: namespace entry "causes_error" (2): referenced descriptor not found `, }, { // 14 @@ -380,7 +380,7 @@ func TestExamineDescriptors(t *testing.T) { {NameInfo: descpb.NameInfo{Name: "null"}, ID: int64(descpb.InvalidID)}, }, expected: `Examining 0 descriptors and 1 namespace entries... - ParentID 0, ParentSchemaID 0: namespace entry "null" (0): invalid descriptor ID + ParentID 0, ParentSchemaID 0: namespace entry "null" (0): invalid namespace entry `, }, { // 15 @@ -415,7 +415,7 @@ func TestExamineDescriptors(t *testing.T) { {NameInfo: descpb.NameInfo{Name: "db"}, ID: 52}, }, expected: `Examining 2 descriptors and 2 namespace entries... - ParentID 52, ParentSchemaID 29: namespace entry "t" (51): no matching name info in draining names of dropped relation + ParentID 52, ParentSchemaID 29: namespace entry "t" (51): descriptor is being dropped `, }, { // 17 diff --git a/pkg/sql/logictest/testdata/logic_test/crdb_internal b/pkg/sql/logictest/testdata/logic_test/crdb_internal index 3c5e02d3224c..05c321061d14 100644 --- a/pkg/sql/logictest/testdata/logic_test/crdb_internal +++ b/pkg/sql/logictest/testdata/logic_test/crdb_internal @@ -1025,10 +1025,10 @@ query ITTTT colnames SELECT * FROM "".crdb_internal.invalid_objects ---- id database_name schema_name obj_name error -500 baddb · · descriptor not found -501 system badschema · descriptor not found -502 system public badobj descriptor not found -503 system [404] badobj descriptor not found +500 baddb · · referenced descriptor not found +501 system badschema · referenced descriptor not found +502 system public badobj referenced descriptor not found +503 system [404] badobj referenced descriptor not found statement ok SELECT crdb_internal.unsafe_delete_namespace_entry(0, 0, 'baddb', 500, true); diff --git a/pkg/upgrade/upgrades/precondition_before_starting_an_upgrade_external_test.go b/pkg/upgrade/upgrades/precondition_before_starting_an_upgrade_external_test.go index fdffa9cfad7f..80ecb46fab9f 100644 --- a/pkg/upgrade/upgrades/precondition_before_starting_an_upgrade_external_test.go +++ b/pkg/upgrade/upgrades/precondition_before_starting_an_upgrade_external_test.go @@ -119,7 +119,7 @@ func TestPreconditionBeforeStartingAnUpgrade(t *testing.T) { require.Error(t, err, "upgrade should be refused because precondition is violated.") require.Equal(t, "pq: verifying precondition for version 22.1-2: "+ "there exists invalid descriptors as listed below; fix these descriptors before attempting to upgrade again:\n"+ - "invalid descriptor: defaultdb.public.temp_tbl (104) because 'no matching name info found in non-dropped relation \"t\"'", + "invalid descriptor: defaultdb.public.temp_tbl (104) because 'mismatched name \"t\" in relation descriptor'", strings.ReplaceAll(err.Error(), "1000022", "22")) // The cluster version should remain at `v0`. ts.tdb.CheckQueryResults(t, "SHOW CLUSTER SETTING version", [][]string{{v0.String()}})