sql: reference sequences used in views by their IDs

pkg/ccl/backupccl/backup_test.go

@@ -5189,14 +5189,32 @@ func TestBackupRestoreSequence(t *testing.T) {
 			{"3"},
 		})
 
-		// Verify that we can kkeep inserting into the table, without violating a uniqueness constraint.
+		// Verify that we can keep inserting into the table, without violating a uniqueness constraint.
 		newDB.Exec(t, `INSERT INTO data.t (v) VALUES ('bar')`)
 
 		// Verify that sequence <=> table dependencies are still in place.
 		newDB.ExpectErr(
 			t, "pq: cannot drop sequence t_id_seq because other objects depend on it",
 			`DROP SEQUENCE t_id_seq`,
 		)
+
+		// Check that we can rename the sequence, and the table is fine.
+		newDB.Exec(t, `ALTER SEQUENCE t_id_seq RENAME TO t_id_seq2`)
+		newDB.Exec(t, `INSERT INTO data.t (v) VALUES ('qux')`)
+		newDB.CheckQueryResults(t, `SELECT last_value FROM t_id_seq2`, [][]string{{"5"}})
+		newDB.CheckQueryResults(t, `SELECT * FROM t`, [][]string{
+			{"1", "foo"},
+			{"2", "bar"},
+			{"3", "baz"},
+			{"4", "bar"},
+			{"5", "qux"},
+		})
+
+		// Verify that sequence <=> table dependencies are still in place.
+		newDB.ExpectErr(
+			t, "pq: cannot drop sequence t_id_seq2 because other objects depend on it",
+			`DROP SEQUENCE t_id_seq2`,
+		)
 	})
 
 	t.Run("restore just the table to a new cluster", func(t *testing.T) {
@@ -5254,6 +5272,99 @@ func TestBackupRestoreSequence(t *testing.T) {
 	})
 }
 
+func TestBackupRestoreSequencesInViews(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	// Test backing up and restoring a database with views referencing sequences.
+	t.Run("database", func(t *testing.T) {
+		_, _, sqlDB, _, cleanupFn := BackupRestoreTestSetup(t, singleNode, 0, InitManualReplication)
+		defer cleanupFn()
+
+		sqlDB.Exec(t, `CREATE DATABASE d`)
+		sqlDB.Exec(t, `USE d`)
+		sqlDB.Exec(t, `CREATE SEQUENCE s`)
+		sqlDB.Exec(t, `CREATE VIEW v AS SELECT k FROM (SELECT nextval('s') AS k)`)
+
+		// Backup the database.
+		sqlDB.Exec(t, `BACKUP DATABASE d TO 'nodelocal://0/test/'`)
+
+		// Drop the database and restore into it.
+		sqlDB.Exec(t, `DROP DATABASE d`)
+		sqlDB.Exec(t, `RESTORE DATABASE d FROM 'nodelocal://0/test/'`)
+
+		// Check that the view is not corrupted.
+		sqlDB.CheckQueryResults(t, `SELECT * FROM d.v`, [][]string{{"1"}})
+
+		// Check that the sequence can still be renamed.
+		sqlDB.Exec(t, `ALTER SEQUENCE d.s RENAME TO d.s2`)
+
+		// Check that after renaming, the view is still fine, and reflects the rename.
+		sqlDB.CheckQueryResults(t, `SELECT * FROM d.v`, [][]string{{"2"}})
+		sqlDB.CheckQueryResults(t, `SHOW CREATE VIEW d.v`, [][]string{{
+			"d.public.v", `CREATE VIEW public.v (k) AS SELECT k FROM (SELECT nextval('public.s2'::REGCLASS) AS k)`,
+		}})
+
+		// Test that references are still tracked.
+		sqlDB.ExpectErr(t, `pq: cannot drop sequence s2 because other objects depend on it`, `DROP SEQUENCE d.s2`)
+	})
+
+	// Test backing up and restoring both view and sequence.
+	t.Run("restore view and sequence", func(t *testing.T) {
+		_, _, sqlDB, _, cleanupFn := BackupRestoreTestSetup(t, singleNode, 0, InitManualReplication)
+		defer cleanupFn()
+
+		sqlDB.Exec(t, `CREATE DATABASE d`)
+		sqlDB.Exec(t, `USE d`)
+		sqlDB.Exec(t, `CREATE SEQUENCE s`)
+		sqlDB.Exec(t, `CREATE VIEW v AS (SELECT k FROM (SELECT nextval('s') AS k))`)
+
+		// Backup v and s.
+		sqlDB.Exec(t, `BACKUP TABLE v, s TO 'nodelocal://0/test/'`)
+		// Drop v and s.
+		sqlDB.Exec(t, `DROP VIEW v`)
+		sqlDB.Exec(t, `DROP SEQUENCE s`)
+		// Restore v and s.
+		sqlDB.Exec(t, `RESTORE TABLE s, v FROM 'nodelocal://0/test/'`)
+		sqlDB.CheckQueryResults(t, `SHOW CREATE VIEW d.v`, [][]string{{
+			"d.public.v", `CREATE VIEW public.v (k) AS (SELECT k FROM (SELECT nextval('public.s'::REGCLASS) AS k))`,
+		}})
+
+		// Check that v is not corrupted.
+		sqlDB.CheckQueryResults(t, `SELECT * FROM v`, [][]string{{"1"}})
+
+		// Check that s can be renamed and v reflects the change.
+		sqlDB.Exec(t, `ALTER SEQUENCE s RENAME TO s2`)
+		sqlDB.CheckQueryResults(t, `SHOW CREATE VIEW d.v`, [][]string{{
+			"d.public.v", `CREATE VIEW public.v (k) AS (SELECT k FROM (SELECT nextval('public.s2'::REGCLASS) AS k))`,
+		}})
+		sqlDB.CheckQueryResults(t, `SELECT * FROM v`, [][]string{{"2"}})
+
+		// Test that references are still tracked.
+		sqlDB.ExpectErr(t, `pq: cannot drop sequence s2 because other objects depend on it`, `DROP SEQUENCE s2`)
+	})
+
+	// Test backing up and restoring just the view.
+	t.Run("restore just the view", func(t *testing.T) {
+		_, _, sqlDB, _, cleanupFn := BackupRestoreTestSetup(t, singleNode, 0, InitManualReplication)
+		defer cleanupFn()
+
+		sqlDB.Exec(t, `CREATE DATABASE d`)
+		sqlDB.Exec(t, `USE d`)
+		sqlDB.Exec(t, `CREATE SEQUENCE s`)
+		sqlDB.Exec(t, `CREATE VIEW v AS (SELECT k FROM (SELECT nextval('s') AS k))`)
+
+		// Backup v and drop.
+		sqlDB.Exec(t, `BACKUP TABLE v TO 'nodelocal://0/test/'`)
+		sqlDB.Exec(t, `DROP VIEW v`)
+		// Restore v.
+		sqlDB.ExpectErr(
+			t, "pq: cannot restore view \"v\" without restoring referenced table \\(or \"skip_missing_views\" option\\)",
+			`RESTORE TABLE v FROM 'nodelocal://0/test/'`,
+		)
+	})
+}
+
 func TestBackupRestoreSequenceOwnership(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)

pkg/ccl/backupccl/restore_old_sequences_test.go

@@ -20,8 +20,8 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// TestRestoreOldVersions ensures that we can successfully restore tables
-// and databases exported by old version.
+// TestRestoreOldSequences ensures that we can successfully restore tables
+// and views created in older versions into a newer version.
 //
 // The files being restored live in testdata and are all made from the same
 // input SQL which lives in <testdataBase>/create.sql.
@@ -73,7 +73,7 @@ func restoreOldSequencesTest(exportDir string) func(t *testing.T) {
 		sqlDB.QueryRow(t, `RESTORE test.* FROM $1`, LocalFoo).Scan(
 			&unused, &unused, &unused, &importedRows, &unused, &unused,
 		)
-		const totalRows = 3
+		const totalRows = 4
 		if importedRows != totalRows {
 			t.Fatalf("expected %d rows, got %d", totalRows, importedRows)
 		}
@@ -96,9 +96,17 @@ func restoreOldSequencesTest(exportDir string) func(t *testing.T) {
 		}
 
 		// Verify that tables with old sequences aren't corrupted.
-		//sqlDB.Exec(t, `SET database = test`)
-		//sqlDB.Exec(t, `USE test`)
 		sqlDB.Exec(t, `SET database = test; INSERT INTO test.t1 VALUES (default, default)`)
 		sqlDB.CheckQueryResults(t, `SELECT * FROM test.t1 ORDER BY i`, sequenceResults)
+
+		// Verify that the views are okay, and the sequences it depends on cannot be renamed.
+		sqlDB.CheckQueryResults(t, `SET database = test; SELECT * FROM test.v`, [][]string{{"1"}})
+		sqlDB.CheckQueryResults(t, `SET database = test; SELECT * FROM test.v2`, [][]string{{"2"}})
+		sqlDB.ExpectErr(t,
+			`pq: cannot rename relation "s2" because view "v" depends on it`,
+			`ALTER SEQUENCE s2 RENAME TO s3`)
+		sqlDB.CheckQueryResults(t, `SET database = test; SHOW CREATE VIEW test.v`, [][]string{{
+			"test.public.v", `CREATE VIEW public.v (nextval) AS (SELECT nextval('s2':::STRING))`,
+		}})
 	}
 }

pkg/ccl/backupccl/restore_planning.go

@@ -131,7 +131,11 @@ func rewriteSequencesInExpr(expr string, rewrites DescRewriteMap) (string, error
 		return "", err
 	}
 	rewriteFunc := func(expr tree.Expr) (recurse bool, newExpr tree.Expr, err error) {
-		annotateTypeExpr, id, ok := schemaexpr.GetTypeExprAndSeqID(expr)
+		id, ok := schemaexpr.GetSeqIDFromExpr(expr)
+		if !ok {
+			return true, expr, nil
+		}
+		annotateTypeExpr, ok := expr.(*tree.AnnotateTypeExpr)
 		if !ok {
 			return true, expr, nil
 		}
@@ -155,6 +159,41 @@ func rewriteSequencesInExpr(expr string, rewrites DescRewriteMap) (string, error
 	return newExpr.String(), nil
 }
 
+// rewriteSequencesInView walks the given viewQuery and
+// rewrites all sequence IDs in it according to rewrites.
+func rewriteSequencesInView(viewQuery string, rewrites DescRewriteMap) (string, error) {
+	rewriteFunc := func(expr tree.Expr) (recurse bool, newExpr tree.Expr, err error) {
+		id, ok := schemaexpr.GetSeqIDFromExpr(expr)
+		if !ok {
+			return true, expr, nil
+		}
+		annotateTypeExpr, ok := expr.(*tree.AnnotateTypeExpr)
+		if !ok {
+			return true, expr, nil
+		}
+		rewrite, ok := rewrites[descpb.ID(id)]
+		if !ok {
+			return true, expr, nil
+		}
+		annotateTypeExpr.Expr = tree.NewNumVal(
+			constant.MakeInt64(int64(rewrite.ID)),
+			strconv.Itoa(int(rewrite.ID)),
+			false, /* negative */
+		)
+		return false, annotateTypeExpr, nil
+	}
+
+	stmt, err := parser.ParseOne(viewQuery)
+	if err != nil {
+		return "", err
+	}
+	newStmt, err := tree.SimpleStmtVisit(stmt.AST, rewriteFunc)
+	if err != nil {
+		return "", err
+	}
+	return newStmt.String(), nil
+}
+
 // maybeFilterMissingViews filters the set of tables to restore to exclude views
 // whose dependencies are either missing or are themselves unrestorable due to
 // missing dependencies, and returns the resulting set of tables. If the
@@ -1079,6 +1118,15 @@ func RewriteTableDescs(
 			return err
 		}
 
+		// Walk view query and remap sequence IDs.
+		if table.IsView() {
+			viewQuery, err := rewriteSequencesInView(table.ViewQuery, descriptorRewrites)
+			if err != nil {
+				return err
+			}
+			table.ViewQuery = viewQuery
+		}
+
 		if err := catalog.ForEachNonDropIndex(table, func(indexI catalog.Index) error {
 			index := indexI.IndexDesc()
 			// Verify that for any interleaved index being restored, the interleave

pkg/ccl/backupccl/testdata/restore_old_sequences/create.sql

@@ -15,3 +15,8 @@ CREATE TABLE t1 (i SERIAL PRIMARY KEY, j INT NOT NULL DEFAULT nextval('test.publ
 
 INSERT INTO t1 VALUES (default, default);
 
+CREATE SEQUENCE s2;
+
+CREATE VIEW v AS (SELECT nextval('s2'));
+
+CREATE VIEW v2 AS (SELECT k FROM (SELECT nextval('s2') AS k));

pkg/ccl/importccl/import_stmt_test.go

@@ -5668,7 +5668,7 @@ func TestImportPgDump(t *testing.T) {
 			if c.expected == expectAll {
 				sqlDB.CheckQueryResults(t, `SHOW CREATE TABLE seqtable`, [][]string{{
 					"seqtable", `CREATE TABLE public.seqtable (
-	a INT8 NULL DEFAULT nextval('public.a_seq':::STRING::REGCLASS),
+	a INT8 NULL DEFAULT nextval('public.a_seq'::REGCLASS),
 	b INT8 NULL,
 	rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
 	CONSTRAINT "primary" PRIMARY KEY (rowid ASC),

pkg/sql/catalog/schemaexpr/column.go

@@ -286,14 +286,14 @@ func columnDescriptorsToPtrs(cols []descpb.ColumnDescriptor) []*descpb.ColumnDes
 	return ptrs
 }
 
-// replaceIDsWithFQNames walks the given expr and replaces occurrences
+// ReplaceIDsWithFQNames walks the given expr and replaces occurrences
 // of regclass IDs in the expr with the descriptor's fully qualified name.
 // For example, nextval(12345::REGCLASS) => nextval('foo.public.seq').
-func replaceIDsWithFQNames(
+func ReplaceIDsWithFQNames(
 	ctx context.Context, rootExpr tree.Expr, semaCtx *tree.SemaContext,
 ) (tree.Expr, error) {
 	replaceFn := func(expr tree.Expr) (recurse bool, newExpr tree.Expr, err error) {
-		_, id, ok := GetTypeExprAndSeqID(expr)
+		id, ok := GetSeqIDFromExpr(expr)
 		if !ok {
 			return true, expr, nil
 		}
@@ -322,32 +322,29 @@ func replaceIDsWithFQNames(
 	return newExpr, err
 }
 
-// GetTypeExprAndSeqID takes an expr and looks for a sequence ID in
-// this expr. If it finds one, it will return that ID as well as the
-// AnnotateTypeExpr that contains it.
-func GetTypeExprAndSeqID(expr tree.Expr) (*tree.AnnotateTypeExpr, int64, bool) {
-	// If it's not an AnnotateTypeExpr, skip this node.
-	annotateTypeExpr, ok := expr.(*tree.AnnotateTypeExpr)
-	if !ok {
-		return nil, 0, false
-	}
-
-	// If it's not a regclass, skip this node.
-	if typ, safe := tree.GetStaticallyKnownType(
-		annotateTypeExpr.Type,
-	); !safe || typ.Oid() != oid.T_regclass {
-		return nil, 0, false
-	}
-
-	// If it's not a constant int, skip this node.
-	numVal, ok := annotateTypeExpr.Expr.(*tree.NumVal)
-	if !ok {
-		return nil, 0, false
-	}
-	id, err := numVal.AsInt64()
-	if err != nil {
-		return nil, 0, false
+// GetSeqIDFromExpr takes an expr and looks for a sequence ID in
+// this expr. If it finds one, it will return that ID.
+func GetSeqIDFromExpr(expr tree.Expr) (int64, bool) {
+	// Depending on if the given expr is typed checked, we're looking
+	// for either a *tree.AnnotateTypeExpr (if not typed checked), or
+	// a *tree.DOid (if type checked).
+	switch n := expr.(type) {
+	case *tree.AnnotateTypeExpr:
+		if typ, safe := tree.GetStaticallyKnownType(n.Type); !safe || typ.Oid() != oid.T_regclass {
+			return 0, false
+		}
+		numVal, ok := n.Expr.(*tree.NumVal)
+		if !ok {
+			return 0, false
+		}
+		id, err := numVal.AsInt64()
+		if err != nil {
+			return 0, false
+		}
+		return id, true
+	case *tree.DOid:
+		return int64(n.DInt), true
+	default:
+		return 0, false
 	}
-
-	return annotateTypeExpr, id, true
 }

pkg/sql/catalog/schemaexpr/expr.go

@@ -174,7 +174,12 @@ func FormatExprForDisplay(
 	if err != nil {
 		return "", err
 	}
-	return tree.AsStringWithFlags(expr, fmtFlags), nil
+	// Replace any IDs in the expr with their fully qualified names.
+	replacedExpr, err := ReplaceIDsWithFQNames(ctx, expr, semaCtx)
+	if err != nil {
+		return "", err
+	}
+	return tree.AsStringWithFlags(replacedExpr, fmtFlags), nil
 }
 
 func deserializeExprForFormatting(
@@ -189,15 +194,9 @@ func deserializeExprForFormatting(
 		return nil, err
 	}
 
-	// Replace any IDs in the expr with their fully qualified names.
-	seqReplacedExpr, err := replaceIDsWithFQNames(ctx, expr, semaCtx)
-	if err != nil {
-		return nil, err
-	}
-
 	// Replace the column variables with dummyColumns so that they can be
 	// type-checked.
-	replacedExpr, _, err := replaceColumnVars(desc, seqReplacedExpr)
+	replacedExpr, _, err := replaceColumnVars(desc, expr)
 	if err != nil {
 		return nil, err
 	}

pkg/sql/crdb_internal.go

@@ -2043,7 +2043,7 @@ CREATE TABLE crdb_internal.create_statements (
 		var err error
 		if table.IsView() {
 			descType = typeView
-			stmt, err = ShowCreateView(ctx, &name, table)
+			stmt, err = ShowCreateView(ctx, &p.semaCtx, &name, table)
 		} else if table.IsSequence() {
 			descType = typeSequence
 			stmt, err = ShowCreateSequence(ctx, &name, table)