Skip to content

Commit

Permalink
sql/schemachanger: version gate element creation
Browse files Browse the repository at this point in the history
We cannot create elements the old version of the code does not know about.

Release justification: fixed mixed version incompatibility
Release note: None
  • Loading branch information
ajwerner authored and Xiang-Gu committed Sep 8, 2022
1 parent 1133a0e commit 7dbf545
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 3 deletions.
6 changes: 6 additions & 0 deletions pkg/sql/schemachanger/scbuild/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,18 @@ func Build(
Authorization: els.authorization,
}
current := make([]scpb.Status, 0, len(bs.output))
version := dependencies.ClusterSettings().Version.ActiveVersion(ctx)
for _, e := range bs.output {
if e.metadata.Size() == 0 {
// Exclude targets which weren't explicitly set.
// Explicitly-set targets have non-zero values in the target metadata.
continue
}
// Exclude targets which are not yet usable in the currently active
// cluster version.
if !version.IsActive(screl.MinVersion(e.element)) {
continue
}
ts.Targets = append(ts.Targets, scpb.MakeTarget(e.target, e.element, &e.metadata))
current = append(current, e.current)
}
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/schemachanger/screl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ go_library(
importpath = "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/screl",
visibility = ["//visibility:public"],
deps = [
"//pkg/clusterversion",
"//pkg/sql/catalog",
"//pkg/sql/catalog/catpb",
"//pkg/sql/catalog/descpb",
Expand Down
27 changes: 27 additions & 0 deletions pkg/sql/schemachanger/screl/scalars.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
package screl

import (
"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catid"
Expand Down Expand Up @@ -91,3 +92,29 @@ func ContainsDescID(haystack scpb.Element, needle catid.DescID) (contains bool)
})
return contains
}

// MinVersion returns the minimum cluster version at which an element may
// be used.
func MinVersion(el scpb.Element) clusterversion.Key {
switch el.(type) {
case *scpb.Database, *scpb.Schema, *scpb.View, *scpb.Sequence, *scpb.Table,
*scpb.AliasType, *scpb.ColumnFamily, *scpb.Column, *scpb.PrimaryIndex,
*scpb.SecondaryIndex, *scpb.TemporaryIndex, *scpb.EnumType,
*scpb.UniqueWithoutIndexConstraint, *scpb.CheckConstraint,
*scpb.ForeignKeyConstraint, *scpb.TableComment, *scpb.RowLevelTTL,
*scpb.TableLocalityGlobal, *scpb.TableLocalityPrimaryRegion,
*scpb.TableLocalitySecondaryRegion, *scpb.TableLocalityRegionalByRow,
*scpb.ColumnName, *scpb.ColumnType, *scpb.ColumnDefaultExpression,
*scpb.ColumnOnUpdateExpression, *scpb.SequenceOwner, *scpb.ColumnComment,
*scpb.IndexName, *scpb.IndexPartitioning, *scpb.SecondaryIndexPartial,
*scpb.IndexComment, *scpb.ConstraintName, *scpb.ConstraintComment,
*scpb.Namespace, *scpb.Owner, *scpb.UserPrivileges,
*scpb.DatabaseRegionConfig, *scpb.DatabaseRoleSetting, *scpb.DatabaseComment,
*scpb.SchemaParent, *scpb.SchemaComment, *scpb.ObjectParent:
return clusterversion.V22_1
case *scpb.IndexColumn, *scpb.EnumTypeValue, *scpb.TableZoneConfig:
return clusterversion.UseDelRangeInGCJob
default:
panic(errors.AssertionFailedf("unknown element %T", el))
}
}
19 changes: 16 additions & 3 deletions pkg/sql/schemachanger/screl/scalars_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,24 @@ import (

// TestAllElementsHaveDescID ensures that all element types have a DescID.
func TestAllElementsHaveDescID(t *testing.T) {
forEachElementType(func(elem scpb.Element) {
require.Equalf(t, descpb.ID(0), GetDescID(elem), "elem %T", elem)
})
}

func TestAllElementsHaveMinVersion(t *testing.T) {
forEachElementType(func(elem scpb.Element) {
// If `elem` does not have a min version, the following function call will panic.
MinVersion(elem)
})
}

func forEachElementType(f func(element scpb.Element)) {
typ := reflect.TypeOf((*scpb.ElementProto)(nil)).Elem()
for i := 0; i < typ.NumField(); i++ {
f := typ.Field(i)
elem := reflect.New(f.Type.Elem()).Interface().(scpb.Element)
require.Equal(t, descpb.ID(0), GetDescID(elem))
field := typ.Field(i)
elem := reflect.New(field.Type.Elem()).Interface().(scpb.Element)
f(elem)
}
}

Expand Down

0 comments on commit 7dbf545

Please sign in to comment.