From d3fb44ba42e72c659781e29fec7c2cc7f6084241 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 27 Sep 2022 23:08:48 +0000 Subject: [PATCH] backupccl: remove non-remapped system table rows During restore, we need to remap the values in some system table rows to point to the new IDs of tables, i.e. we need to change the ID of a zone config that applied to the foo table when it had ID 104 to apply to the restored foo table that now has ID 108. However when we do so, we would sometimes see that a row in the restored system table would have an ID which we are not restoring and thus for which we have no remapping. Such rows might correspond to zones for dropped tables or similar. In these cases, the remapping process could fail due to a collision between these old, non-remapped IDs and new, reampped IDs if they were assigned an ID matching the old one. These rows corresponding to IDs not being restored should generally also not be restored, so we can avoid this conflict by simply deleting them during the remapping. The exception to this is rows mentioning IDs below 50, as these are fixed ID system tables that are never remapped; even though they do not appear in the remapping table, rows which mention them should be restored, as-is, as they still mention the same system table in the restored cluster, as these are fixed static IDs. Release note: none. Fixes #88008. --- pkg/ccl/backupccl/system_schema.go | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/pkg/ccl/backupccl/system_schema.go b/pkg/ccl/backupccl/system_schema.go index 54d3191b3f64..da45c9c116e7 100644 --- a/pkg/ccl/backupccl/system_schema.go +++ b/pkg/ccl/backupccl/system_schema.go @@ -586,25 +586,38 @@ func rekeySystemTable( typ = "oid" } - // We'll build one big UPDATE that moves all the IDs to their intermediate - // new values, which is new+offset, or just +offset for IDs which do not - // have a remapping, since we will unconditionally slide everything down by - // the offset at the end. + // We'll build one big UPDATE that moves all remapped the IDs temporary IDs + // consisting of new+fixed-offset; later we can slide everything down by + // fixed-offset to put them in their correct, final places. q := strings.Builder{} fmt.Fprintf(&q, "UPDATE %s SET %s = (CASE\n", tempTableName, colName) for _, old := range toRekey { fmt.Fprintf(&q, "WHEN %s = %d THEN %d\n", colName, old, rekeys[old].ID+offset) } - fmt.Fprintf(&q, "ELSE %s::int + %d END)::%s", colName, offset, typ) + fmt.Fprintf(&q, "ELSE %s END)::%s", colName, typ) if _, err := executor.Exec(ctx, fmt.Sprintf("remap-%s", tempTableName), txn, q.String()); err != nil { return errors.Wrapf(err, "remapping IDs %s", tempTableName) } - // Now slide all the IDs back down by the offset to their intended values. + // Now that the rows mentioning remapped IDs are remapped to be shifted to + // be above offset, we can clean out anything that did not get remapped and + // might be in the way when we shift back down such as a zone config for a + // dropped table that isn't being restored and thus has no remapping. Rows + // that mention IDs below 50 can stay though: these are mentioning old fixed + // ID system tables that we do not restore directly, and thus have no entry + // in our remapping, but the configuration of them (comments, zones, etc) is + // expected to be restored. + if _, err := executor.Exec(ctx, fmt.Sprintf("remap-remove-%s", tempTableName), txn, + fmt.Sprintf("DELETE FROM %s WHERE %s >= 50 AND %s < %d", tempTableName, colName, colName, offset), + ); err != nil { + return errors.Wrapf(err, "remapping IDs %s", tempTableName) + } + + // Now slide remapped the IDs back down by offset, to their intended values. if _, err := executor.Exec(ctx, fmt.Sprintf("remap-%s-deoffset", tempTableName), txn, - fmt.Sprintf("UPDATE %s SET %s = (%s::int - %d)::%s", tempTableName, colName, colName, offset, typ), + fmt.Sprintf("UPDATE %s SET %s = (%s::int - %d)::%s WHERE %s::int >= %d", tempTableName, colName, colName, offset, typ, colName, offset), ); err != nil { return errors.Wrapf(err, "remapping %s; removing offset", tempTableName) }