Merge cockroachdb#153724

153724: backup: restore must skip temporary tables r=jeffswenson a=jeffswenson

There is a bug in backup. We backup temporary table descriptors but not temporary schemas. When we restore the temporary tables they end up getting restored into the public schema. This is wrong in the sense the temporary table is placed in the wrong schema and can cause restores to fail if there are multiple temp tables with the same name.

Release note: fixes a bug where the presence of duplicate temporary tables in a backup caused the restore to fail with an error containing the text 'restoring table desc and namespace
entries: table already exists'.
Informs: cockroachdb#153722

Co-authored-by: Jeff Swenson <[email protected]>

Author: craig[bot]

pkg/backup/restore_test.go

@@ -7,6 +7,7 @@ package backup
 
 import (
 	"context"
+	gosql "database/sql"
 	"syscall"
 	"testing"
 	"time"
@@ -271,3 +272,49 @@ func TestRestoreJobMessages(t *testing.T) {
 		return nil
 	})
 }
+
+func TestRestoreDuplicateTempTables(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	// This is a regression test for #153722. It verifies that restoring a backup
+	// that contains two temporary tables with the same name does not cause the
+	// restore to fail with an error of the form: "restoring 17 TableDescriptors
+	// from 4 databases: restoring table desc and namespace entries: table
+	// already exists"
+
+	clusterSize := 1
+	tc, sqlDB, _, cleanupFn := backuptestutils.StartBackupRestoreTestCluster(t, clusterSize)
+	defer cleanupFn()
+
+	sqlDB.Exec(t, `SET experimental_enable_temp_tables=true`)
+	sqlDB.Exec(t, `CREATE DATABASE test_db`)
+	sqlDB.Exec(t, `USE test_db`)
+	sqlDB.Exec(t, `CREATE TABLE permanent_table (id INT PRIMARY KEY, name TEXT)`)
+
+	sessions := make([]*gosql.DB, 2)
+	for i := range sessions {
+		sessions[i] = tc.Servers[0].SQLConn(t)
+		sql := sqlutils.MakeSQLRunner(sessions[i])
+		sql.Exec(t, `SET experimental_enable_temp_tables=true`)
+		sql.Exec(t, `USE test_db`)
+		sql.Exec(t, `CREATE TEMP TABLE duplicate_temp (id INT PRIMARY KEY, value TEXT)`)
+		sql.Exec(t, `INSERT INTO duplicate_temp VALUES (1, 'value')`)
+	}
+
+	sqlDB.Exec(t, `BACKUP INTO 'nodelocal://1/duplicate_temp_backup'`)
+
+	for _, session := range sessions {
+		require.NoError(t, session.Close())
+	}
+
+	// The cluster must be empty for a full cluster restore.
+	sqlDB.Exec(t, `DROP DATABASE test_db CASCADE`)
+	sqlDB.Exec(t, `RESTORE FROM LATEST IN 'nodelocal://1/duplicate_temp_backup'`)
+
+	sqlDB.Exec(t, `DROP DATABASE test_db CASCADE`)
+	sqlDB.Exec(t, `RESTORE DATABASE test_db FROM LATEST IN 'nodelocal://1/duplicate_temp_backup'`)
+
+	result := sqlDB.QueryStr(t, `SELECT table_name FROM [SHOW TABLES] ORDER BY table_name`)
+	require.Equal(t, [][]string{{"permanent_table"}}, result)
+}

pkg/backup/targets.go

@@ -305,6 +305,10 @@ func fullClusterTargetsRestore(
 	var filteredDescs []catalog.Descriptor
 	var filteredDBs []catalog.DatabaseDescriptor
 	for _, desc := range fullClusterDescs {
+		if table, ok := desc.(catalog.TableDescriptor); ok && table.IsTemporary() {
+			// TODO(jeffswenson): We should move this filtering into backup.
+			continue
+		}
 		if desc.GetID() != keys.SystemDatabaseID {
 			filteredDescs = append(filteredDescs, desc)
 		}
@@ -381,7 +385,6 @@ func selectTargets(
 	}
 
 	if descriptorCoverage == tree.AllDescriptors {
-
 		tables, dbs, patterns, err := fullClusterTargetsRestore(ctx, allDescs, lastBackupManifest)
 		return tables, dbs, patterns, nil, err
 	}