Merge pull request #106217 from postamar/backport23.1-105832

docs/generated/sql/functions.md

@@ -3309,6 +3309,8 @@ table. Returns an error if validation fails.</p>
 <thead><tr><th>Function &rarr; Returns</th><th>Description</th><th>Volatility</th></tr></thead>
 <tbody>
 <tr><td><a name="crdb_internal.force_delete_table_data"></a><code>crdb_internal.force_delete_table_data(id: <a href="int.html">int</a>) &rarr; <a href="bool.html">bool</a></code></td><td><span class="funcdesc"><p>This function can be used to clear the data belonging to a table, when the table cannot be dropped.</p>
+</span></td><td>Volatile</td></tr>
+<tr><td><a name="crdb_internal.repair_catalog_corruption"></a><code>crdb_internal.repair_catalog_corruption(descriptor_id: <a href="int.html">int</a>, corruption: <a href="string.html">string</a>) &rarr; <a href="bool.html">bool</a></code></td><td><span class="funcdesc"><p>repair_catalog_corruption(descriptor_id,corruption) attempts to repair corrupt records in system tables associated with that descriptor id</p>
 </span></td><td>Volatile</td></tr></tbody>
 </table>
 

pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant

@@ -68,6 +68,7 @@ crdb_internal  kv_dropped_relations                    view   admin  NULL  NULL
 crdb_internal  kv_inherited_role_members               table  admin  NULL  NULL
 crdb_internal  kv_node_liveness                        table  admin  NULL  NULL
 crdb_internal  kv_node_status                          table  admin  NULL  NULL
+crdb_internal  kv_repairable_catalog_corruptions       view   admin  NULL  NULL
 crdb_internal  kv_store_status                         table  admin  NULL  NULL
 crdb_internal  kv_system_privileges                    view   admin  NULL  NULL
 crdb_internal  leases                                  table  admin  NULL  NULL

pkg/cli/zip_test.go

@@ -85,6 +85,7 @@ table_name NOT IN (
 	'kv_catalog_descriptor',
 	'kv_catalog_namespace',
 	'kv_catalog_zones',
+	'kv_repairable_catalog_corruptions',
 	'kv_dropped_relations',
 	'kv_inherited_role_members',
 	'lost_descriptors_with_data',

pkg/sql/catalog/desctestutils/descriptor_test_utils.go

@@ -165,7 +165,7 @@ func TestingGetPublicTypeDescriptor(
 	return TestingGetTypeDescriptor(kvDB, codec, database, "public", object)
 }
 
-func TestGetFunctionDescriptor(
+func TestingGetFunctionDescriptor(
 	kvDB *kv.DB, codec keys.SQLCodec, database string, schema string, fName string,
 ) catalog.FunctionDescriptor {
 	db := TestingGetDatabaseDescriptor(kvDB, codec, database)

pkg/sql/catalog/post_deserialization_changes.go

@@ -114,4 +114,9 @@ const (
 	// StrippedDanglingBackReferences indicates that at least one dangling
 	// back-reference was removed from the descriptor.
 	StrippedDanglingBackReferences
+
+	// StrippedDanglingSelfBackReferences indicates that at least one dangling
+	// back-reference to something within the descriptor itself was removed
+	// from the descriptor.
+	StrippedDanglingSelfBackReferences
 )

pkg/sql/catalog/redact/redact_test.go

@@ -65,7 +65,7 @@ $$`)
 	})
 
 	t.Run("create function", func(t *testing.T) {
-		fn := desctestutils.TestGetFunctionDescriptor(kvDB, keys.SystemSQLCodec, "defaultdb", "public", "f1")
+		fn := desctestutils.TestingGetFunctionDescriptor(kvDB, keys.SystemSQLCodec, "defaultdb", "public", "f1")
 		mut := funcdesc.NewBuilder(fn.FuncDesc()).BuildCreatedMutableFunction()
 		require.Empty(t, redact.Redact(mut.DescriptorProto()))
 		require.Equal(t, `SELECT k FROM defaultdb.public.kv WHERE v != '_'; SELECT k FROM defaultdb.public.kv WHERE v = '_';`, mut.FunctionBody)

pkg/sql/catalog/tabledesc/structured_test.go

@@ -856,6 +856,36 @@ func TestDefaultExprNil(t *testing.T) {
 	})
 }
 
+// TestStrippedDanglingSelfBackReferences checks the proper behavior of the
+// catalog.StrippedDanglingSelfBackReferences post-deserialization change.
+func TestStrippedDanglingSelfBackReferences(t *testing.T) {
+	ctx := context.Background()
+	s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
+	defer s.Stopper().Stop(ctx)
+	tdb := sqlutils.MakeSQLRunner(sqlDB)
+
+	// Create a table.
+	tdb.Exec(t, `CREATE DATABASE t`)
+	tdb.Exec(t, `CREATE TABLE t.tbl (a INT PRIMARY KEY)`)
+
+	// Get the descriptor for the table.
+	tbl := desctestutils.TestingGetPublicTableDescriptor(kvDB, keys.SystemSQLCodec, "t", "tbl")
+
+	// Inject some nonsense into the mutation_jobs slice.
+	mut := NewBuilder(tbl.TableDesc()).BuildExistingMutableTable()
+	mut.MutationJobs = append(mut.MutationJobs, descpb.TableDescriptor_MutationJob{
+		MutationID: 12345,
+		JobID:      6789,
+	})
+
+	// Check that it's properly removed.
+	b := NewBuilder(mut.TableDesc())
+	require.NoError(t, b.RunPostDeserializationChanges())
+	desc := b.BuildExistingMutableTable()
+	require.Empty(t, desc.MutationJobs)
+	require.True(t, desc.GetPostDeserializationChanges().Contains(catalog.StrippedDanglingSelfBackReferences))
+}
+
 // TestRemoveDefaultExprFromComputedColumn tests that default expressions are
 // correctly removed from descriptors of computed columns as part of the
 // RunPostDeserializationChanges suite.

pkg/sql/catalog/tabledesc/table_desc_builder.go

@@ -191,72 +191,68 @@ func (tdb *tableDescriptorBuilder) RunRestoreChanges(
 func (tdb *tableDescriptorBuilder) StripDanglingBackReferences(
 	descIDMightExist func(id descpb.ID) bool, nonTerminalJobIDMightExist func(id jobspb.JobID) bool,
 ) error {
+	tbl := tdb.maybeModified
 	// Strip dangling back-references in depended_on_by,
 	{
 		sliceIdx := 0
-		for _, backref := range tdb.maybeModified.DependedOnBy {
-			tdb.maybeModified.DependedOnBy[sliceIdx] = backref
+		for _, backref := range tbl.DependedOnBy {
+			tbl.DependedOnBy[sliceIdx] = backref
 			if descIDMightExist(backref.ID) {
 				sliceIdx++
 			}
 		}
-		if sliceIdx < len(tdb.maybeModified.DependedOnBy) {
-			tdb.maybeModified.DependedOnBy = tdb.maybeModified.DependedOnBy[:sliceIdx]
+		if sliceIdx < len(tbl.DependedOnBy) {
+			tbl.DependedOnBy = tbl.DependedOnBy[:sliceIdx]
 			tdb.changes.Add(catalog.StrippedDanglingBackReferences)
 		}
 	}
 	// ... in inbound foreign keys,
 	{
 		sliceIdx := 0
-		for _, backref := range tdb.maybeModified.InboundFKs {
-			tdb.maybeModified.InboundFKs[sliceIdx] = backref
+		for _, backref := range tbl.InboundFKs {
+			tbl.InboundFKs[sliceIdx] = backref
 			if descIDMightExist(backref.OriginTableID) {
 				sliceIdx++
 			}
 		}
-		if sliceIdx < len(tdb.maybeModified.InboundFKs) {
-			tdb.maybeModified.InboundFKs = tdb.maybeModified.InboundFKs[:sliceIdx]
+		if sliceIdx < len(tbl.InboundFKs) {
+			tbl.InboundFKs = tbl.InboundFKs[:sliceIdx]
 			tdb.changes.Add(catalog.StrippedDanglingBackReferences)
 		}
 	}
 	// ... in the replacement_of field,
-	if id := tdb.maybeModified.ReplacementOf.ID; id != descpb.InvalidID && !descIDMightExist(id) {
-		tdb.maybeModified.ReplacementOf.Reset()
+	if id := tbl.ReplacementOf.ID; id != descpb.InvalidID && !descIDMightExist(id) {
+		tbl.ReplacementOf.Reset()
 		tdb.changes.Add(catalog.StrippedDanglingBackReferences)
 	}
 	// ... in the drop_job field,
-	if id := tdb.maybeModified.DropJobID; id != jobspb.InvalidJobID && !nonTerminalJobIDMightExist(id) {
-		tdb.maybeModified.DropJobID = jobspb.InvalidJobID
+	if id := tbl.DropJobID; id != jobspb.InvalidJobID && !nonTerminalJobIDMightExist(id) {
+		tbl.DropJobID = jobspb.InvalidJobID
 		tdb.changes.Add(catalog.StrippedDanglingBackReferences)
 	}
 	// ... in the mutation_jobs slice,
 	{
-		var mutationIDs intsets.Fast
-		for _, m := range tdb.maybeModified.Mutations {
-			mutationIDs.Add(int(m.MutationID))
-		}
 		sliceIdx := 0
-		for _, backref := range tdb.maybeModified.MutationJobs {
-			tdb.maybeModified.MutationJobs[sliceIdx] = backref
-			if nonTerminalJobIDMightExist(backref.JobID) &&
-				mutationIDs.Contains(int(backref.MutationID)) {
+		for _, backref := range tbl.MutationJobs {
+			tbl.MutationJobs[sliceIdx] = backref
+			if nonTerminalJobIDMightExist(backref.JobID) {
 				sliceIdx++
 			}
 		}
-		if sliceIdx < len(tdb.maybeModified.MutationJobs) {
-			tdb.maybeModified.MutationJobs = tdb.maybeModified.MutationJobs[:sliceIdx]
+		if sliceIdx < len(tbl.MutationJobs) {
+			tbl.MutationJobs = tbl.MutationJobs[:sliceIdx]
 			tdb.changes.Add(catalog.StrippedDanglingBackReferences)
 		}
 	}
 	// ... in the row_level_ttl field,
-	if ttl := tdb.maybeModified.RowLevelTTL; ttl != nil {
+	if ttl := tbl.RowLevelTTL; ttl != nil {
 		if id := jobspb.JobID(ttl.ScheduleID); id != jobspb.InvalidJobID && !nonTerminalJobIDMightExist(id) {
-			tdb.maybeModified.RowLevelTTL = nil
+			tbl.RowLevelTTL = nil
 			tdb.changes.Add(catalog.StrippedDanglingBackReferences)
 		}
 	}
 	// ... in the sequence ownership field.
-	if seq := tdb.maybeModified.SequenceOpts; seq != nil {
+	if seq := tbl.SequenceOpts; seq != nil {
 		if id := seq.SequenceOwner.OwnerTableID; id != descpb.InvalidID && !descIDMightExist(id) {
 			seq.SequenceOwner.Reset()
 			tdb.changes.Add(catalog.StrippedDanglingBackReferences)
@@ -398,6 +394,7 @@ func maybeFillInDescriptor(
 	set(catalog.RemovedDuplicateIDsInRefs, maybeRemoveDuplicateIDsInRefs(desc))
 	set(catalog.AddedConstraintIDs, maybeAddConstraintIDs(desc))
 	set(catalog.SetCheckConstraintColumnIDs, maybeSetCheckConstraintColumnIDs(desc))
+	set(catalog.StrippedDanglingSelfBackReferences, maybeStripDanglingSelfBackReferences(desc))
 	return changes, nil
 }
 
@@ -978,6 +975,32 @@ func maybeSetCheckConstraintColumnIDs(desc *descpb.TableDescriptor) (hasChanged
 	return hasChanged
 }
 
+// maybeStripDanglingSelfBackReferences removes any references to things
+// within the table descriptor itself which don't exist.
+//
+// TODO(postamar): extend as needed to column references and whatnot
+func maybeStripDanglingSelfBackReferences(tbl *descpb.TableDescriptor) (hasChanged bool) {
+	var mutationIDs intsets.Fast
+	for _, m := range tbl.Mutations {
+		mutationIDs.Add(int(m.MutationID))
+	}
+	// Remove mutation_jobs entries which don't reference a valid mutation ID.
+	{
+		sliceIdx := 0
+		for _, backref := range tbl.MutationJobs {
+			tbl.MutationJobs[sliceIdx] = backref
+			if mutationIDs.Contains(int(backref.MutationID)) {
+				sliceIdx++
+			}
+		}
+		if sliceIdx < len(tbl.MutationJobs) {
+			tbl.MutationJobs = tbl.MutationJobs[:sliceIdx]
+			hasChanged = true
+		}
+	}
+	return hasChanged
+}
+
 // maybeSetCreateAsOfTime ensures that the CreateAsOfTime field is set.
 //
 // CreateAsOfTime is used for CREATE TABLE ... AS ... and was introduced in

pkg/sql/catalog/tabledesc/table_desc_test.go

@@ -109,10 +109,11 @@ func TestStripDanglingBackReferences(t *testing.T) {
 				MutationJobs: []descpb.TableDescriptor_MutationJob{
 					{JobID: 1, MutationID: 1},
 					{JobID: 111222333444, MutationID: 1},
-					{JobID: 2, MutationID: 1},
+					{JobID: 2, MutationID: 2},
 				},
 				Mutations: []descpb.DescriptorMutation{
 					{MutationID: 1},
+					{MutationID: 2},
 				},
 				DropJobID: 1,
 				RowLevelTTL: &catpb.RowLevelTTL{
@@ -127,36 +128,12 @@ func TestStripDanglingBackReferences(t *testing.T) {
 				},
 				Mutations: []descpb.DescriptorMutation{
 					{MutationID: 1},
+					{MutationID: 2},
 				},
 			},
 			validDescIDs: catalog.MakeDescriptorIDSet(100, 101, 104, 105),
 			validJobIDs:  map[jobspb.JobID]struct{}{111222333444: {}},
 		},
-		{
-			name: "mutation IDs",
-			input: descpb.TableDescriptor{
-				Name: "foo",
-				ID:   104,
-				MutationJobs: []descpb.TableDescriptor_MutationJob{
-					{JobID: 555666777888, MutationID: 123}, {JobID: 111222333444, MutationID: 1},
-				},
-				Mutations: []descpb.DescriptorMutation{
-					{MutationID: 1},
-				},
-			},
-			expectedOutput: descpb.TableDescriptor{
-				Name: "foo",
-				ID:   104,
-				MutationJobs: []descpb.TableDescriptor_MutationJob{
-					{JobID: 111222333444, MutationID: 1},
-				},
-				Mutations: []descpb.DescriptorMutation{
-					{MutationID: 1},
-				},
-			},
-			validDescIDs: catalog.MakeDescriptorIDSet(104),
-			validJobIDs:  map[jobspb.JobID]struct{}{111222333444: {}, 555666777888: {}},
-		},
 	}
 
 	for _, test := range testData {

pkg/sql/crdb_internal.go

@@ -210,6 +210,7 @@ var crdbInternal = virtualSchema{
 		catconstants.CrdbInternalShowTenantCapabilitiesCacheTableID: crdbInternalShowTenantCapabilitiesCache,
 		catconstants.CrdbInternalInheritedRoleMembersTableID:        crdbInternalInheritedRoleMembers,
 		catconstants.CrdbInternalKVSystemPrivilegesViewID:           crdbInternalKVSystemPrivileges,
+		catconstants.CrdbInternalRepairableCatalogCorruptionsViewID: crdbInternalRepairableCatalogCorruptions,
 	},
 	validWithNoDatabaseContext: true,
 }
@@ -5790,6 +5791,10 @@ CREATE TABLE crdb_internal.invalid_objects (
 					if dbContext != nil && d.GetParentID() != dbContext.GetID() {
 						return nil
 					}
+				case catalog.FunctionDescriptor:
+					if dbContext != nil && d.GetParentID() != dbContext.GetID() {
+						return nil
+					}
 				default:
 					return nil
 				}
@@ -6145,6 +6150,84 @@ CREATE TABLE crdb_internal.lost_descriptors_with_data (
 	},
 }
 
+// crdbInternalRepairableCatalogCorruptions lists all corruptions in the
+// catalog which can be automatically repaired. By type:
+//   - 'descriptor' corruptions denote corruptions in the system.descriptor
+//     table which can be repaired by applying
+//     crdb_internal.unsafe_upsert_descriptor to the output of
+//     crdb_internal.repaired_descriptor,
+//   - 'namespace' corruptions denote corruptions in the system.namespace
+//     table which can be repaired by removing the corresponding record with
+//     crdb_internal.unsafe_delete_namespace_entry.
+var crdbInternalRepairableCatalogCorruptions = virtualSchemaView{
+	comment: "known corruptions in the catalog which can be repaired using builtin functions like " +
+		"crdb_internal.repaired_descriptor",
+	resultColumns: colinfo.ResultColumns{
+		{Name: "parent_id", Typ: types.Int},
+		{Name: "parent_schema_id", Typ: types.Int},
+		{Name: "name", Typ: types.String},
+		{Name: "id", Typ: types.Int},
+		{Name: "corruption", Typ: types.String},
+	},
+	schema: `
+CREATE VIEW crdb_internal.kv_repairable_catalog_corruptions (
+	parent_id,
+	parent_schema_id,
+	name,
+	id,
+	corruption
+) AS
+	WITH
+		data
+			AS (
+				SELECT
+					ns."parentID" AS parent_id,
+					ns."parentSchemaID" AS parent_schema_id,
+					ns.name,
+					COALESCE(ns.id, d.id) AS id,
+					d.descriptor,
+					crdb_internal.descriptor_with_post_deserialization_changes(d.descriptor)
+						AS updated_descriptor,
+					crdb_internal.repaired_descriptor(
+						d.descriptor,
+						(SELECT array_agg(id) AS desc_id_array FROM system.descriptor),
+						(
+							SELECT
+								array_agg(id) AS job_id_array
+							FROM
+								system.jobs
+							WHERE
+								status NOT IN ('failed', 'succeeded', 'canceled', 'revert-failed')
+						)
+					)
+						AS repaired_descriptor
+				FROM
+					system.namespace AS ns FULL JOIN system.descriptor AS d ON ns.id = d.id
+			),
+		diag
+			AS (
+				SELECT
+					*,
+					CASE
+					WHEN descriptor IS NULL AND id != 29 THEN 'namespace'
+					WHEN updated_descriptor != repaired_descriptor THEN 'descriptor'
+					ELSE NULL
+					END
+						AS corruption
+				FROM
+					data
+			)
+	SELECT
+		parent_id, parent_schema_id, name, id, corruption
+	FROM
+		diag
+	WHERE
+		corruption IS NOT NULL
+	ORDER BY
+		parent_id, parent_schema_id, name, id
+`,
+}
+
 var crdbInternalDefaultPrivilegesTable = virtualSchemaTable{
 	comment: `virtual table with default privileges`,
 	schema: `