Merge pull request #112757 from cockroachdb/blathers/backport-release…

…-22.2-111820

release-22.2: catalog: don't set modification time for offline descriptors

pkg/ccl/backupccl/backup_test.go

@@ -7505,6 +7505,7 @@ USE d;
 CREATE SCHEMA sc;
 CREATE TABLE sc.tb (x INT);
 CREATE TYPE sc.typ AS ENUM ('hello');
+CREATE FUNCTION f() RETURNS INT AS $$ SELECT 1 $$ LANGUAGE SQL;
 `)
 
 		// Back up the database.
@@ -7524,19 +7525,30 @@ CREATE TYPE sc.typ AS ENUM ('hello');
 
 		<-beforePublishingNotif
 
-		// Verify that the descriptors are offline.
+		// Verify that the descriptors are offline. Also check that they don't have
+		// any PostDeserializationChanges, since that would cause the version to
+		// get bumped during the cluster upgrade that rewrites all descriptors
+		// with PostDeserializationChanges.
 
 		dbDesc := desctestutils.TestingGetDatabaseDescriptor(kvDB, keys.SystemSQLCodec, "d")
 		require.Equal(t, descpb.DescriptorState_OFFLINE, dbDesc.DatabaseDesc().State)
+		require.Equal(t, descpb.DescriptorVersion(1), dbDesc.DatabaseDesc().Version)
+		require.Empty(t, dbDesc.GetPostDeserializationChanges())
 
 		schemaDesc := desctestutils.TestingGetSchemaDescriptor(kvDB, keys.SystemSQLCodec, dbDesc.GetID(), "sc")
 		require.Equal(t, descpb.DescriptorState_OFFLINE, schemaDesc.SchemaDesc().State)
+		require.Equal(t, descpb.DescriptorVersion(1), schemaDesc.SchemaDesc().Version)
+		require.Empty(t, schemaDesc.GetPostDeserializationChanges())
 
 		tableDesc := desctestutils.TestingGetTableDescriptor(kvDB, keys.SystemSQLCodec, "d", "sc", "tb")
 		require.Equal(t, descpb.DescriptorState_OFFLINE, tableDesc.GetState())
+		require.Equal(t, descpb.DescriptorVersion(1), tableDesc.GetVersion())
+		require.Empty(t, tableDesc.GetPostDeserializationChanges())
 
 		typeDesc := desctestutils.TestingGetTypeDescriptor(kvDB, keys.SystemSQLCodec, "d", "sc", "typ")
 		require.Equal(t, descpb.DescriptorState_OFFLINE, typeDesc.TypeDesc().State)
+		require.Equal(t, descpb.DescriptorVersion(1), typeDesc.TypeDesc().Version)
+		require.Empty(t, typeDesc.GetPostDeserializationChanges())
 
 		// Verify that the descriptors are not visible.
 		// TODO (lucy): Arguably there should be a SQL test where we manually create
@@ -7571,6 +7583,7 @@ CREATE TYPE sc.typ AS ENUM ('hello');
 
 		close(continueNotif)
 		require.NoError(t, g.Wait())
+
 	})
 
 	t.Run("restore-into-existing-database", func(t *testing.T) {

pkg/sql/catalog/dbdesc/database_desc_builder.go

@@ -94,7 +94,7 @@ func (ddb *databaseDescriptorBuilder) RunPostDeserializationChanges() (err error
 	// Set the ModificationTime field before doing anything else.
 	// Other changes may depend on it.
 	mustSetModTime, err := descpb.MustSetModificationTime(
-		ddb.original.ModificationTime, ddb.mvccTimestamp, ddb.original.Version,
+		ddb.original.ModificationTime, ddb.mvccTimestamp, ddb.original.Version, ddb.original.State,
 	)
 	if err != nil {
 		return err

pkg/sql/catalog/descpb/descriptor.go

@@ -86,8 +86,14 @@ func GetDescriptors(
 // field must be set to the given MVCC timestamp. An error is returned if the
 // argument values are inconsistent.
 func MustSetModificationTime(
-	modTime hlc.Timestamp, mvccTimestamp hlc.Timestamp, version DescriptorVersion,
+	modTime hlc.Timestamp,
+	mvccTimestamp hlc.Timestamp,
+	version DescriptorVersion,
+	state DescriptorState,
 ) (bool, error) {
+	if state == DescriptorState_OFFLINE {
+		return false, nil
+	}
 	// Set the ModificationTime based on the passed mvccTimestamp if we should.
 	// Table descriptors can be updated in place after their version has been
 	// incremented (e.g. to include a schema change lease).

pkg/sql/catalog/funcdesc/func_desc_builder.go

@@ -87,7 +87,7 @@ func (fdb functionDescriptorBuilder) RunPostDeserializationChanges() (err error)
 	// Set the ModificationTime field before doing anything else.
 	// Other changes may depend on it.
 	mustSetModTime, err := descpb.MustSetModificationTime(
-		fdb.original.ModificationTime, fdb.mvccTimestamp, fdb.original.Version,
+		fdb.original.ModificationTime, fdb.mvccTimestamp, fdb.original.Version, fdb.original.State,
 	)
 	if err != nil {
 		return err

pkg/sql/catalog/schemadesc/schema_desc_builder.go

@@ -89,7 +89,7 @@ func (sdb *schemaDescriptorBuilder) RunPostDeserializationChanges() (err error)
 	// Set the ModificationTime field before doing anything else.
 	// Other changes may depend on it.
 	mustSetModTime, err := descpb.MustSetModificationTime(
-		sdb.original.ModificationTime, sdb.mvccTimestamp, sdb.original.Version,
+		sdb.original.ModificationTime, sdb.mvccTimestamp, sdb.original.Version, sdb.original.State,
 	)
 	if err != nil {
 		return err

pkg/sql/catalog/tabledesc/table_desc_builder.go

@@ -129,7 +129,7 @@ func (tdb *tableDescriptorBuilder) RunPostDeserializationChanges() (err error) {
 	// Set the ModificationTime field before doing anything else.
 	// Other changes may depend on it.
 	mustSetModTime, err := descpb.MustSetModificationTime(
-		tdb.original.ModificationTime, tdb.mvccTimestamp, tdb.original.Version,
+		tdb.original.ModificationTime, tdb.mvccTimestamp, tdb.original.Version, tdb.original.State,
 	)
 	if err != nil {
 		return err

pkg/sql/catalog/typedesc/type_desc_builder.go

@@ -90,7 +90,7 @@ func (tdb *typeDescriptorBuilder) RunPostDeserializationChanges() (err error) {
 	// Set the ModificationTime field before doing anything else.
 	// Other changes may depend on it.
 	mustSetModTime, err := descpb.MustSetModificationTime(
-		tdb.original.ModificationTime, tdb.mvccTimestamp, tdb.original.Version,
+		tdb.original.ModificationTime, tdb.mvccTimestamp, tdb.original.Version, tdb.original.State,
 	)
 	if err != nil {
 		return err