sql: clean up old version support and deprecated field in schemachanger

bghal · bghal · commit fb6d385dd936 · 2025-10-24T16:29:19.000-04:00
These versions are more than two versions back and don't need to be
supported. The deprecated computed column field is also removed.

Epic: none

Release note: None
diff --git a/pkg/sql/catalog/redact/redact.go b/pkg/sql/catalog/redact/redact.go
@@ -162,10 +162,6 @@ func redactElement(element scpb.Element) error {
 		return redactExpr(&e.Expression.Expr)
 	case *scpb.ColumnOnUpdateExpression:
 		return redactExpr(&e.Expression.Expr)
-	case *scpb.ColumnType:
-		if e.ComputeExpr != nil {
-			return redactExpr(&e.ComputeExpr.Expr)
-		}
 	case *scpb.ColumnComputeExpression:
 		return redactExpr(&e.Expression.Expr)
 	case *scpb.FunctionBody:
diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_add_column.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_add_column.go
@@ -169,14 +169,10 @@ func alterTableAddColumn(
 	if desc.IsComputed() {
 		validExpr, _ := b.ComputedColumnExpression(tbl, d, tree.ComputedColumnExprContext(d.IsVirtual()))
 		expr := b.WrapExpression(tbl.TableID, validExpr)
-		if spec.colType.ElementCreationMetadata.In_24_3OrLater {
-			spec.compute = &scpb.ColumnComputeExpression{
-				TableID:    tbl.TableID,
-				ColumnID:   spec.col.ColumnID,
-				Expression: *expr,
-			}
-		} else {
-			spec.colType.ComputeExpr = expr
+		spec.compute = &scpb.ColumnComputeExpression{
+			TableID:    tbl.TableID,
+			ColumnID:   spec.col.ColumnID,
+			Expression: *expr,
 		}
 		if desc.Virtual {
 			b.IncrementSchemaChangeAddColumnQualificationCounter("virtual")
@@ -483,7 +479,7 @@ func addColumn(b BuildCtx, spec addColumnSpec, n tree.NodeFormatter) (backing *s
 		}
 
 		inflatedChain := getInflatedPrimaryIndexChain(b, spec.tbl.TableID)
-		if spec.def == nil && spec.colType.ComputeExpr == nil && spec.compute == nil && spec.transientCompute == nil {
+		if spec.def == nil && spec.compute == nil && spec.transientCompute == nil {
 			// Optimization opportunity: if we were to add a new column without default
 			// value nor computed expression, then we can just add the column to existing
 			// non-nil primary indexes without actually backfilling any data. This is
diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_index.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_index.go
@@ -858,14 +858,10 @@ func maybeCreateAndAddShardCol(
 		notNull: true,
 	}
 	wexpr := b.WrapExpression(tbl.TableID, parsedExpr)
-	if spec.colType.ElementCreationMetadata.In_24_3OrLater {
-		spec.compute = &scpb.ColumnComputeExpression{
-			TableID:    tbl.TableID,
-			ColumnID:   shardColID,
-			Expression: *wexpr,
-		}
-	} else {
-		spec.colType.ComputeExpr = wexpr
+	spec.compute = &scpb.ColumnComputeExpression{
+		TableID:    tbl.TableID,
+		ColumnID:   shardColID,
+		Expression: *wexpr,
 	}
 
 	backing := addColumn(b, spec, n)
diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/helpers.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/helpers.go
@@ -2006,23 +2006,17 @@ func retrieveColumnTypeElem(
 }
 
 // retrieveColumnComputeExpression returns the compute expression of the column.
-// If no expression exists, then nil is returned. This will handle older
-// versions that may store the expression as part of the ColumnType.
+// If no expression exists, then nil is returned.
 func retrieveColumnComputeExpression(
 	b BuildCtx, tableID catid.DescID, columnID catid.ColumnID,
 ) (expr *scpb.Expression) {
-	// First try to retrieve the expression from the ColumnComputeExpression. This
-	// may be unavailable because the column doesn't have a compute expression, or
-	// it's an older version that stores the expression as part of the ColumnType.
-	colComputeExpression := b.QueryByID(tableID).FilterColumnComputeExpression().Filter(func(_ scpb.Status, _ scpb.TargetStatus, e *scpb.ColumnComputeExpression) bool {
+	if colComputeExpression := b.QueryByID(tableID).FilterColumnComputeExpression().Filter(func(_ scpb.Status, _ scpb.TargetStatus, e *scpb.ColumnComputeExpression) bool {
 		return e.ColumnID == columnID
-	}).MustGetZeroOrOneElement()
-	if colComputeExpression != nil {
+	}).MustGetZeroOrOneElement(); colComputeExpression != nil {
 		return &colComputeExpression.Expression
 	}
-	// Check the ColumnType in case this is an older version.
-	columnType := mustRetrieveColumnTypeElem(b, tableID, columnID)
-	return columnType.ComputeExpr
+
+	return nil
 }
 
 // mustRetrieveColumnTypeElem retrieves the index column elements associated
diff --git a/pkg/sql/schemachanger/scdecomp/decomp.go b/pkg/sql/schemachanger/scdecomp/decomp.go
@@ -580,15 +580,11 @@ func (w *walkCtx) walkColumn(tbl catalog.TableDescriptor, col catalog.Column) {
 			expr, err := w.newExpression(col.GetComputeExpr())
 			onErrPanic(err)
 
-			if columnType.ElementCreationMetadata.In_24_3OrLater {
-				w.ev(scpb.Status_PUBLIC, &scpb.ColumnComputeExpression{
-					TableID:    tbl.GetID(),
-					ColumnID:   col.GetID(),
-					Expression: *expr,
-				})
-			} else {
-				columnType.ComputeExpr = expr
-			}
+			w.ev(scpb.Status_PUBLIC, &scpb.ColumnComputeExpression{
+				TableID:    tbl.GetID(),
+				ColumnID:   col.GetID(),
+				Expression: *expr,
+			})
 		}
 		w.ev(scpb.Status_PUBLIC, columnType)
 	}
diff --git a/pkg/sql/schemachanger/scdecomp/helpers.go b/pkg/sql/schemachanger/scdecomp/helpers.go
@@ -134,13 +134,10 @@ func newTypeT(t *types.T) *scpb.TypeT {
 	}
 }
 
-// NewElementCreationMetadata construct a `*scpb.ElementCreationMetadata`
-// based on `clusterVersion`.
+// NewElementCreationMetadata construct a `*scpb.ElementCreationMetadata` based
+// on `clusterVersion`.
 func NewElementCreationMetadata(
 	clusterVersion clusterversion.ClusterVersion,
 ) *scpb.ElementCreationMetadata {
-	return &scpb.ElementCreationMetadata{
-		In_23_1OrLater: true,
-		In_24_3OrLater: true,
-	}
+	return &scpb.ElementCreationMetadata{}
 }
diff --git a/pkg/sql/schemachanger/scexec/scmutationexec/column.go b/pkg/sql/schemachanger/scexec/scmutationexec/column.go
@@ -78,16 +78,6 @@ func (i *immediateVisitor) addNewColumnType(
 	col.Type = op.ColumnType.Type
 	col.Nullable = true
 	col.Virtual = op.ColumnType.IsVirtual
-	// ComputeExpr is deprecated in favor of a separate element
-	// (ColumnComputeExpression). Any changes in this if block
-	// should also be made in the AddColumnComputeExpression function.
-	if !op.ColumnType.ElementCreationMetadata.In_24_3OrLater {
-		if ce := op.ColumnType.ComputeExpr; ce != nil {
-			expr := string(ce.Expr)
-			col.ComputeExpr = &expr
-			col.UsesSequenceIds = ce.UsesSequenceIDs
-		}
-	}
 	if !col.Virtual {
 		for i := range tbl.Families {
 			fam := &tbl.Families[i]
diff --git a/pkg/sql/schemachanger/scpb/elements.proto b/pkg/sql/schemachanger/scpb/elements.proto
@@ -241,10 +241,7 @@ message ColumnType {
   uint32 family_id = 2 [(gogoproto.customname) = "FamilyID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.FamilyID"];
   uint32 column_id = 3 [(gogoproto.customname) = "ColumnID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.ColumnID"];
   TypeT embedded_type_t = 4 [(gogoproto.nullable) = false, (gogoproto.embed) = true];
-  reserved 5;
-  // Deprecated
-  // compute expression is now handled as a separate element (see ColumnComputeExpression).
-  Expression compute_expr = 6 [deprecated = true];
+  reserved 5, 6;
   bool is_virtual = 7;
   reserved 8, 9, 10;
   // ElementCreationMetadata stores information about when this element is created.
@@ -1011,6 +1008,5 @@ message FunctionSecurity {
 }
 
 message ElementCreationMetadata {
-  bool in_23_1_or_later = 1;
-  bool in_24_3_or_later = 2;
+  reserved 1, 2;
 }
diff --git a/pkg/sql/schemachanger/scpb/migration.go b/pkg/sql/schemachanger/scpb/migration.go
@@ -58,23 +58,6 @@ func migrateDeprecatedFields(
 		}
 	}
 
-	// Migrate ComputeExpr field  to separate ColumnComputeExpression target.
-	if columnType := target.GetColumnType(); columnType != nil {
-		if columnType.ComputeExpr != nil {
-			newTarget := MakeTarget(
-				AsTargetStatus(target.TargetStatus),
-				&ColumnComputeExpression{
-					TableID:    columnType.TableID,
-					ColumnID:   columnType.ColumnID,
-					Expression: *columnType.ComputeExpr,
-				},
-				&target.Metadata,
-			)
-			newTargets = append(newTargets, newTarget)
-			columnType.ComputeExpr = nil
-			migrated = true
-		}
-	}
 	return
 }
 
diff --git a/pkg/sql/schemachanger/scpb/migration_test.go b/pkg/sql/schemachanger/scpb/migration_test.go
@@ -9,50 +9,11 @@ import (
 	"testing"
 
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
-	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/catid"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/idxtype"
 	"github.com/stretchr/testify/require"
 )
 
-// TestDeprecatedColumnComputeFieldMigration will ensure that ComputeExpr in
-// ColumnType is nulled out and a new ColumnComputeExpression is added.
-func TestDeprecatedColumnComputeFieldMigration(t *testing.T) {
-	state := DescriptorState{
-		Targets: []Target{
-			MakeTarget(ToPublic,
-				&ColumnType{
-					TableID: 100,
-					ComputeExpr: &Expression{
-						Expr: catpb.Expression("Hello"),
-					},
-				},
-				nil,
-			),
-		},
-		CurrentStatuses: []Status{Status_PUBLIC},
-		TargetRanks:     []uint32{1},
-	}
-	migrationOccurred := MigrateDescriptorState(
-		clusterversion.ClusterVersion{Version: clusterversion.Latest.Version()},
-		1,
-		&state,
-	)
-	require.True(t, migrationOccurred)
-	require.Len(t, state.CurrentStatuses, 2)
-	require.Len(t, state.Targets, 2)
-	require.NotNil(t, state.Targets[0].GetColumnType())
-	ct := state.Targets[0].GetColumnType()
-	require.Nil(t, ct.ComputeExpr)
-	require.NotNil(t, state.Targets[1].GetColumnComputeExpression())
-	cce := state.Targets[1].GetColumnComputeExpression()
-	require.Equal(t, cce.TableID, ct.TableID)
-	require.Equal(t, cce.ColumnID, ct.ColumnID)
-	require.Equal(t, cce.Expr, catpb.Expression("Hello"))
-	require.Equal(t, state.CurrentStatuses[1], Status_PUBLIC)
-	require.Equal(t, state.TargetRanks[1], uint32(2))
-}
-
 // TestDeprecatedIsInvertedMigration tests that the IsInverted field is migrated
 // to the new Type field.
 func TestDeprecatedIsInvertedMigration(t *testing.T) {
diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_type.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_type.go
@@ -71,11 +71,6 @@ func init() {
 
 func referencedTypeIDs(this *scpb.ColumnType) []catid.DescID {
 	var ids catalog.DescriptorIDSet
-	if this.ComputeExpr != nil {
-		for _, id := range this.ComputeExpr.UsesTypeIDs {
-			ids.Add(id)
-		}
-	}
 	for _, id := range this.ClosedTypeIDs {
 		ids.Add(id)
 	}
diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go b/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go
@@ -161,11 +161,6 @@ func isWithTypeTOrHasReferences(element scpb.Element) bool {
 
 func getExpression(element scpb.Element) (*scpb.Expression, error) {
 	switch e := element.(type) {
-	case *scpb.ColumnType:
-		if e == nil {
-			return nil, nil
-		}
-		return e.ComputeExpr, nil
 	case *scpb.ColumnComputeExpression:
 		if e == nil {
 			return nil, nil
diff --git a/pkg/sql/schemachanger/scplan/internal/rules/release_25_2/helpers.go b/pkg/sql/schemachanger/scplan/internal/rules/release_25_2/helpers.go
@@ -142,11 +142,6 @@ func isWithTypeT(element scpb.Element) bool {
 
 func getExpression(element scpb.Element) (*scpb.Expression, error) {
 	switch e := element.(type) {
-	case *scpb.ColumnType:
-		if e == nil {
-			return nil, nil
-		}
-		return e.ComputeExpr, nil
 	case *scpb.ColumnComputeExpression:
 		if e == nil {
 			return nil, nil
diff --git a/pkg/sql/schemachanger/scplan/internal/rules/release_25_3/helpers.go b/pkg/sql/schemachanger/scplan/internal/rules/release_25_3/helpers.go
@@ -148,11 +148,6 @@ func isWithTypeT(element scpb.Element) bool {
 
 func getExpression(element scpb.Element) (*scpb.Expression, error) {
 	switch e := element.(type) {
-	case *scpb.ColumnType:
-		if e == nil {
-			return nil, nil
-		}
-		return e.ComputeExpr, nil
 	case *scpb.ColumnComputeExpression:
 		if e == nil {
 			return nil, nil
diff --git a/pkg/sql/schemachanger/screl/scalars_test.go b/pkg/sql/schemachanger/screl/scalars_test.go
@@ -81,23 +81,6 @@ func TestAllDescIDsAndContainsDescID(t *testing.T) {
 			},
 			expected: []catid.DescID{1, 2, 3},
 		},
-		{
-			name: "computed column",
-			input: &scpb.ColumnType{
-				TableID:  1,
-				ColumnID: 10,
-				TypeT: scpb.TypeT{
-					Type:          types.AnyElement,
-					ClosedTypeIDs: []catid.DescID{2, 3},
-				},
-				ComputeExpr: &scpb.Expression{
-					Expr:            "foo",
-					UsesTypeIDs:     []catid.DescID{3, 4},
-					UsesSequenceIDs: []catid.DescID{5, 6},
-				},
-			},
-			expected: []catid.DescID{1, 2, 3, 4, 5, 6},
-		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			require.ElementsMatch(t, tc.expected, AllDescIDs(tc.input).Ordered())
diff --git a/pkg/sql/schemachanger/screl/walk_test.go b/pkg/sql/schemachanger/screl/walk_test.go
@@ -59,12 +59,11 @@ func TestWalk(t *testing.T) {
 			}
 		}(),
 		func() testCase {
-			v := scpb.ColumnType{ComputeExpr: &scpb.Expression{}, TypeT: scpb.TypeT{Type: types.Timestamp}}
+			v := scpb.ColumnType{TypeT: scpb.TypeT{Type: types.Timestamp}}
 			return testCase{
-				elem:           &v,
-				expIDs:         []*catid.DescID{&v.TableID},
-				expTypes:       []*types.T{types.Timestamp},
-				expExpressions: []*catpb.Expression{&v.ComputeExpr.Expr},
+				elem:     &v,
+				expIDs:   []*catid.DescID{&v.TableID},
+				expTypes: []*types.T{types.Timestamp},
 			}
 		}(),
 		func() testCase {

Original file line number	Diff line number	Diff line change
`@@ -134,13 +134,10 @@ func newTypeT(t types.T) scpb.TypeT {`
`134`	`134`	`}`
`135`	`135`	`}`
`136`	`136`
`137`		-// NewElementCreationMetadata construct a `*scpb.ElementCreationMetadata`
`138`		-// based on `clusterVersion`.
	`137`	+// NewElementCreationMetadata construct a `*scpb.ElementCreationMetadata` based
	`138`	+// on `clusterVersion`.
`139`	`139`	`func NewElementCreationMetadata(`
`140`	`140`	`clusterVersion clusterversion.ClusterVersion,`
`141`	`141`	`) *scpb.ElementCreationMetadata {`
`142`		`- return &scpb.ElementCreationMetadata{`
`143`		`- In_23_1OrLater: true,`
`144`		`- In_24_3OrLater: true,`
`145`		`- }`
	`142`	`+ return &scpb.ElementCreationMetadata{}`
`146`	`143`	`}`