sql/types: reintroduce downgradeType for NAME compatibility in mixed clusters

This patch restores the `downgradeType` hook to maintain serialization
compatibility for the NAME type in mixed-version clusters (v25.3 and 25.4).

In previous releases, a NAME type could be misinterpreted by older nodes.
Specifically, a v25.3 node deserializing a NAME encoded by a v25.4 node would
mistakenly treat it as text (`oid.T_text`). This was due to the old
`upgradeType` logic overwriting the OID when no `VisibleType` was set, causing
`T_name` to degrade to `T_text`. PR #148694 addressed this issue for v25.4 by
changing `upgradeType` (to avoid clobbering a provided OID) and removing the
`downgradeType` function.

However, the follow-up fix for mixed-version serialization (PR #152633) relied
on the `InternalType.VisibleType` field to tag string types for older nodes.
Because the NAME type doesn’t use a `VisibleType` (it’s represented as a
STRING family with a special OID), that change didn’t cover NAME. As a
result, NAME remained broken in a 25.3/25.4 mixed cluster.

This commit explicitly handles NAME during serialization. When sending a NAME
type to the wire, we downgrade it to the legacy encoding: we set its type
family to `Family=name` (legacy enum value 11) so that v25.3 nodes recognize
it as a NAME. Additionally, if the locale is an empty string, we set it to
`nil` to ensure older versions don’t misclassify the NAME as a collated
string. Together, these steps preserve the correct type interpretation
on older nodes.

Once all nodes are on v25.4, this compatibility shim can be removed. At that
point, the improved upgrade logic in v25.4 (which no longer overrides a valid
OID) means we won’t need to downgrade types for backward compatibility. We may
also perform a one time descriptor migration to update any NAME descriptors to
the modern format after the cluster upgrade. In short, `downgradeType` for NAME
is a temporary measure to bridge the v25.3 gap.

Fixes #153322

Release note: None

Author: spilchen

pkg/sql/logictest/testdata/logic_test/crdb_internal_catalog

@@ -149,4 +149,4 @@ SELECT id, strip_volatile(descriptor) FROM crdb_internal.kv_catalog_descriptor W
 111         {"table": {"checks": [{"columnIds": [1], "constraintId": 2, "expr": "k > 0:::INT8", "name": "ck"}], "columns": [{"id": 1, "name": "k", "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 2, "name": "v", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}], "dependedOnBy": [{"columnIds": [1, 2], "id": 112}], "formatVersion": 3, "id": 111, "name": "kv", "nextColumnId": 3, "nextConstraintId": 3, "nextIndexId": 2, "nextMutationId": 1, "parentId": 106, "primaryIndex": {"constraintId": 1, "encodingType": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "keyColumnDirections": ["ASC"], "keyColumnIds": [1], "keyColumnNames": ["k"], "name": "kv_pkey", "partitioning": {}, "sharded": {}, "storeColumnIds": [2], "storeColumnNames": ["v"], "unique": true, "vecConfig": {}, "version": 4}, "privileges": {"ownerProto": "root", "users": [{"privileges": "2", "userProto": "admin", "withGrantOption": "2"}, {"privileges": "2", "userProto": "root", "withGrantOption": "2"}], "version": 3}, "replacementOf": {"time": {}}, "schemaLocked": true, "unexposedParentSchemaId": 107, "version": "7"}}
 112         {"table": {"columns": [{"id": 1, "name": "k", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 2, "name": "v", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"defaultExpr": "unique_rowid()", "hidden": true, "id": 3, "name": "rowid", "type": {"family": "IntFamily", "oid": 20, "width": 64}}], "dependsOn": [111], "formatVersion": 3, "id": 112, "indexes": [{"createdExplicitly": true, "foreignKey": {}, "geoConfig": {}, "id": 2, "interleave": {}, "keyColumnDirections": ["ASC"], "keyColumnIds": [2], "keyColumnNames": ["v"], "keySuffixColumnIds": [3], "name": "idx", "partitioning": {}, "sharded": {}, "vecConfig": {}, "version": 4}], "isMaterializedView": true, "name": "mv", "nextColumnId": 4, "nextConstraintId": 2, "nextIndexId": 4, "nextMutationId": 1, "parentId": 106, "primaryIndex": {"constraintId": 1, "encodingType": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "keyColumnDirections": ["ASC"], "keyColumnIds": [3], "keyColumnNames": ["rowid"], "name": "mv_pkey", "partitioning": {}, "sharded": {}, "storeColumnIds": [1, 2], "storeColumnNames": ["k", "v"], "unique": true, "vecConfig": {}, "version": 4}, "privileges": {"ownerProto": "root", "users": [{"privileges": "2", "userProto": "admin", "withGrantOption": "2"}, {"privileges": "2", "userProto": "root", "withGrantOption": "2"}], "version": 3}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 107, "version": "8", "viewQuery": "SELECT k, v FROM db.public.kv"}}
 113         {"function": {"functionBody": "SELECT json_remove_path(json_remove_path(json_remove_path(json_remove_path(json_remove_path(json_remove_path(json_remove_path(json_remove_path(json_remove_path(json_remove_path(json_remove_path(json_remove_path(d, ARRAY['table':::STRING, 'families':::STRING]:::STRING[]), ARRAY['table':::STRING, 'nextFamilyId':::STRING]:::STRING[]), ARRAY['table':::STRING, 'indexes':::STRING, '0':::STRING, 'createdAtNanos':::STRING]:::STRING[]), ARRAY['table':::STRING, 'indexes':::STRING, '1':::STRING, 'createdAtNanos':::STRING]:::STRING[]), ARRAY['table':::STRING, 'indexes':::STRING, '2':::STRING, 'createdAtNanos':::STRING]:::STRING[]), ARRAY['table':::STRING, 'primaryIndex':::STRING, 'createdAtNanos':::STRING]:::STRING[]), ARRAY['table':::STRING, 'createAsOfTime':::STRING]:::STRING[]), ARRAY['table':::STRING, 'modificationTime':::STRING]:::STRING[]), ARRAY['function':::STRING, 'modificationTime':::STRING]:::STRING[]), ARRAY['type':::STRING, 'modificationTime':::STRING]:::STRING[]), ARRAY['schema':::STRING, 'modificationTime':::STRING]:::STRING[]), ARRAY['database':::STRING, 'modificationTime':::STRING]:::STRING[]);", "id": 113, "lang": "SQL", "name": "strip_volatile", "nullInputBehavior": "CALLED_ON_NULL_INPUT", "params": [{"class": "IN", "name": "d", "type": {"family": "JsonFamily", "oid": 3802}}], "parentId": 104, "parentSchemaId": 105, "privileges": {"ownerProto": "root", "users": [{"privileges": "2", "userProto": "admin", "withGrantOption": "2"}, {"privileges": "1048576", "userProto": "public"}, {"privileges": "2", "userProto": "root", "withGrantOption": "2"}], "version": 3}, "returnType": {"type": {"family": "JsonFamily", "oid": 3802}}, "version": "1", "volatility": "STABLE"}}
-4294966962  {"table": {"columns": [{"id": 1, "name": "f_table_catalog", "nullable": true, "type": {"family": "StringFamily", "oid": 19}}, {"id": 2, "name": "f_table_schema", "nullable": true, "type": {"family": "StringFamily", "oid": 19}}, {"id": 3, "name": "f_table_name", "nullable": true, "type": {"family": "StringFamily", "oid": 19}}, {"id": 4, "name": "f_geography_column", "nullable": true, "type": {"family": "StringFamily", "oid": 19}}, {"id": 5, "name": "coord_dimension", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 6, "name": "srid", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 7, "name": "type", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}], "formatVersion": 3, "id": 4294966962, "name": "geography_columns", "nextColumnId": 8, "nextConstraintId": 2, "nextIndexId": 2, "nextMutationId": 1, "primaryIndex": {"constraintId": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "partitioning": {}, "sharded": {}, "vecConfig": {}}, "privileges": {"ownerProto": "node", "users": [{"privileges": "32", "userProto": "public"}], "version": 3}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 4294966963, "version": "1"}}
+4294966962  {"table": {"columns": [{"id": 1, "name": "f_table_catalog", "nullable": true, "type": {"family": 11, "oid": 19}}, {"id": 2, "name": "f_table_schema", "nullable": true, "type": {"family": 11, "oid": 19}}, {"id": 3, "name": "f_table_name", "nullable": true, "type": {"family": 11, "oid": 19}}, {"id": 4, "name": "f_geography_column", "nullable": true, "type": {"family": 11, "oid": 19}}, {"id": 5, "name": "coord_dimension", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 6, "name": "srid", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 7, "name": "type", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}], "formatVersion": 3, "id": 4294966962, "name": "geography_columns", "nextColumnId": 8, "nextConstraintId": 2, "nextIndexId": 2, "nextMutationId": 1, "primaryIndex": {"constraintId": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "partitioning": {}, "sharded": {}, "vecConfig": {}}, "privileges": {"ownerProto": "node", "users": [{"privileges": "32", "userProto": "public"}], "version": 3}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 4294966963, "version": "1"}}

pkg/sql/logictest/testdata/logic_test/mixed_version_char

@@ -1,5 +1,7 @@
 # LogicTest: cockroach-go-testserver-25.2
 
+subtest regression_152629
+
 # This test verifies that the "char" type is handled correctly in a mixed
 # version cluster. The reason it's being tested is because during 25.4
 # development we began removing the legacy code for serializing types in the
@@ -25,3 +27,48 @@ user root nodeidx=2
 
 statement ok
 insert into t values (ARRAY['abc'::STRING, ''::STRING], 'cat'::STRING);
+
+subtest end
+
+subtest regression_153322
+
+# This is a repro scenario for 153322. It involves the NAME data type, which is
+# a string type column type.
+
+# 1 is already upgraded but adding a redundant one here incase this subtest is
+# run in isolation.
+upgrade 1
+
+user root nodeidx=1
+
+# Create the function using the NAME data type in the upgraded version.
+statement ok
+CREATE FUNCTION test_name_func(p NAME) RETURNS TEXT LANGUAGE SQL AS 'SELECT p::TEXT'
+
+query T
+SELECT test_name_func('test_value')
+----
+test_value
+
+user root nodeidx=2
+
+# Attempt to access the function on the old non-upgraded versions. These had
+# previously failed to resolve the function.
+
+query T
+SELECT test_name_func('test_value')
+----
+test_value
+
+statement ok
+ALTER FUNCTION test_name_func(NAME) RENAME TO test_name_func_renamed
+
+query T
+SELECT test_name_func_renamed('renamed_test')
+----
+renamed_test
+
+statement ok
+DROP FUNCTION test_name_func_renamed(NAME);
+
+subtest end

pkg/sql/types/types.go

@@ -2526,7 +2526,18 @@ func (t *T) IsPseudoType() bool {
 //
 // Marshal is part of the protoutil.Message interface.
 func (t *T) Size() (n int) {
-	return t.InternalType.Size()
+	// non-NAME types don't need any serialization changes to be backwards compatible.
+	if t.InternalType.Oid != oid.T_name {
+		return t.InternalType.Size()
+	}
+	// Need to first downgrade the type before delegating to InternalType,
+	// because Marshal will downgrade.
+	temp := *t
+	err := temp.downgradeType()
+	if err != nil {
+		panic(errors.NewAssertionErrorWithWrappedErrf(err, "error during Size call"))
+	}
+	return temp.InternalType.Size()
 }
 
 // Identical is the internal implementation for T.Identical. See that comment
@@ -2832,12 +2843,50 @@ func (t *T) upgradeType() error {
 	return nil
 }
 
+// downgradeType transforms NAME types for backwards-compatibility with v25.3
+// and earlier versions during mixed-version clusters. For non-NAME types, this
+// is a no-op.
+//
+// NAME types are downgraded to use the legacy Family=name encoding so that v25.3
+// nodes can correctly interpret them without losing NAME semantics.
+//
+// TODO(sql-foundations): Remove this function entirely once v25.4 is the minimum
+// supported version and we no longer need to support mixed-version clusters with
+// v25.3 nodes.
+func (t *T) downgradeType() error {
+	// Set Family for NAME type for backwards-compatibility.
+	switch t.Family() {
+	case StringFamily, CollatedStringFamily:
+		t.InternalType.Family = name
+		// If locale is an empty string, we set it to nil to ensure older versions
+		// don’t misclassify the NAME as a collated string.
+		if t.InternalType.Locale != nil && len(*t.InternalType.Locale) == 0 {
+			t.InternalType.Locale = nil
+		}
+	}
+
+	return nil
+}
+
 // Marshal serializes a type into a byte representation using gogo protobuf
-// serialization rules. It returns the resulting bytes as a slice.
+// serialization rules. It returns the resulting bytes as a slice. The bytes
+// are serialized in a format that is backwards-compatible with the previous
+// version of CRDB so that clusters can run in mixed version mode during
+// upgrade.
 //
 //	bytes, err := protoutil.Marshal(&typ)
 func (t *T) Marshal() (data []byte, err error) {
-	return protoutil.Marshal(&t.InternalType)
+	// non-NAME types don't need any serialization changes to be backwards compatible.
+	if t.InternalType.Oid != oid.T_name {
+		return protoutil.Marshal(&t.InternalType)
+	}
+	// First downgrade to a struct that will be serialized in a backwards-
+	// compatible bytes format.
+	temp := *t
+	if err := temp.downgradeType(); err != nil {
+		return nil, err
+	}
+	return protoutil.Marshal(&temp.InternalType)
 }
 
 // MarshalToSizedBuffer is like Mashal, except that it deserializes to
@@ -2846,7 +2895,15 @@ func (t *T) Marshal() (data []byte, err error) {
 //
 // Marshal is part of the protoutil.Message interface.
 func (t *T) MarshalToSizedBuffer(data []byte) (int, error) {
-	return t.InternalType.MarshalToSizedBuffer(data)
+	// non-NAME types don't need any serialization changes to be backwards compatible.
+	if t.InternalType.Oid != oid.T_name {
+		return t.InternalType.MarshalToSizedBuffer(data)
+	}
+	temp := *t
+	if err := temp.downgradeType(); err != nil {
+		return 0, err
+	}
+	return temp.InternalType.MarshalToSizedBuffer(data)
 }
 
 // MarshalTo behaves like Marshal, except that it deserializes to an existing
@@ -2856,7 +2913,15 @@ func (t *T) MarshalToSizedBuffer(data []byte) (int, error) {
 //
 // Marshal is part of the protoutil.Message interface.
 func (t *T) MarshalTo(data []byte) (int, error) {
-	return t.InternalType.MarshalTo(data)
+	// non-NAME types don't need any serialization changes to be backwards compatible.
+	if t.InternalType.Oid != oid.T_name {
+		return t.InternalType.MarshalTo(data)
+	}
+	temp := *t
+	if err := temp.downgradeType(); err != nil {
+		return 0, err
+	}
+	return temp.InternalType.MarshalTo(data)
 }
 
 // String returns the name of the type, similar to the Name method. However, it

pkg/sql/types/types_jsonpb.go

@@ -19,11 +19,17 @@ import (
 // MarshalJSONPB marshals the T to json. This is necessary as otherwise
 // this field will be lost to the crdb_internal.pb_to_json and the likes.
 func (t *T) MarshalJSONPB(marshaler *jsonpb.Marshaler) ([]byte, error) {
+	temp := *t
+	// Only NAME types need changes to be backwards compatible.
+	if temp.InternalType.Oid == oid.T_name {
+		if err := temp.downgradeType(); err != nil {
+			return nil, err
+		}
+	}
 	// Map empty locale to nil so empty string does not appear in the JSON result.
 	// TODO(rafi): When we upgrade to go1.24, we can modify the proto definition
 	//  of the locale field to use `[(gogoproto.jsontag) = ",omitzero"]` instead of
 	//  this workaround.
-	temp := *t
 	if temp.InternalType.Locale != nil && len(*temp.InternalType.Locale) == 0 {
 		temp.InternalType.Locale = nil
 	}

pkg/sql/types/types_test.go

@@ -789,6 +789,7 @@ func TestUnmarshalCompat(t *testing.T) {
 		{InternalType{Family: StringFamily, VisibleType: visibleVARCHAR, Width: 20}, MakeVarChar(20)},
 		{InternalType{Family: StringFamily, VisibleType: visibleCHAR}, BPChar},
 		{InternalType{Family: StringFamily, VisibleType: visibleQCHAR, Width: 1}, QChar},
+		{InternalType{Family: name}, Name},
 	}
 
 	for _, tc := range testCases {
@@ -981,6 +982,17 @@ func TestUpgradeType(t *testing.T) {
 				Locale:      &emptyLocale,
 			}},
 		},
+		{
+			desc: "legacy NAME support",
+			input: &T{InternalType: InternalType{
+				Family: name,
+			}},
+			expected: &T{InternalType: InternalType{
+				Family: StringFamily,
+				Oid:    oid.T_name,
+				Locale: &emptyLocale,
+			}},
+		},
 	}
 
 	for _, tc := range testCases {