-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
sql/schemachanger: add compatibility with 22.2
Previously, when rules were relaxed on master we could have compatibility issues in mixed version environments. This was inadequate because, the relaxed rules could not generate plans that could pass assertions in a mixed version state. To address this, this patch adds the 22.2 rules on master, and uses them in mixed version scenarios. Release note: none Epic: none
- Loading branch information
Showing
25 changed files
with
4,682 additions
and
10 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
60 changes: 60 additions & 0 deletions
60
pkg/sql/schemachanger/scplan/internal/rules/release_22_2/BUILD.bazel
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| load("//build/bazelutil/unused_checker:unused.bzl", "get_x_data") | ||
| load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") | ||
|
|
||
| go_library( | ||
| name = "release_22_2", | ||
| srcs = [ | ||
| "dep_add_column.go", | ||
| "dep_add_constraint.go", | ||
| "dep_add_index.go", | ||
| "dep_add_index_and_column.go", | ||
| "dep_drop_column.go", | ||
| "dep_drop_constraint.go", | ||
| "dep_drop_index.go", | ||
| "dep_drop_index_and_column.go", | ||
| "dep_drop_object.go", | ||
| "dep_swap_index.go", | ||
| "dep_two_version.go", | ||
| "op_drop.go", | ||
| "op_index_and_column.go", | ||
| "registry.go", | ||
| ], | ||
| importpath = "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scplan/internal/rules/release_22_2", | ||
| visibility = ["//pkg/sql/schemachanger/scplan:__subpackages__"], | ||
| deps = [ | ||
| "//pkg/sql/schemachanger/rel", | ||
| "//pkg/sql/schemachanger/scpb", | ||
| "//pkg/sql/schemachanger/scplan/internal/opgen", | ||
| "//pkg/sql/schemachanger/scplan/internal/rules", | ||
| "//pkg/sql/schemachanger/scplan/internal/scgraph", | ||
| "//pkg/sql/schemachanger/screl", | ||
| ], | ||
| ) | ||
|
|
||
| go_test( | ||
| name = "release_22_2_test", | ||
| srcs = [ | ||
| "assertions_test.go", | ||
| "rules_test.go", | ||
| ], | ||
| args = ["-test.timeout=295s"], | ||
| data = glob(["testdata/**"]), | ||
| embed = [":release_22_2"], | ||
| deps = [ | ||
| "//pkg/clusterversion", | ||
| "//pkg/sql/catalog/catpb", | ||
| "//pkg/sql/schemachanger/rel", | ||
| "//pkg/sql/schemachanger/scpb", | ||
| "//pkg/sql/schemachanger/scplan/internal/opgen", | ||
| "//pkg/sql/schemachanger/scplan/internal/rules", | ||
| "//pkg/sql/schemachanger/screl", | ||
| "//pkg/sql/types", | ||
| "//pkg/testutils/datapathutils", | ||
| "@com_github_cockroachdb_datadriven//:datadriven", | ||
| "@com_github_cockroachdb_errors//:errors", | ||
| "@com_github_stretchr_testify//require", | ||
| "@in_gopkg_yaml_v3//:yaml_v3", | ||
| ], | ||
| ) | ||
|
|
||
| get_x_data(name = "get_x_data") |
190 changes: 190 additions & 0 deletions
190
pkg/sql/schemachanger/scplan/internal/rules/release_22_2/assertions_test.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,190 @@ | ||
| // Copyright 2022 The Cockroach Authors. | ||
| // | ||
| // Use of this software is governed by the Business Source License | ||
| // included in the file licenses/BSL.txt. | ||
| // | ||
| // As of the Change Date specified in that file, in accordance with | ||
| // the Business Source License, use of this software will be governed | ||
| // by the Apache License, Version 2.0, included in the file | ||
| // licenses/APL.txt. | ||
|
|
||
| package release_22_2 | ||
|
|
||
| import ( | ||
| "reflect" | ||
| "runtime" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/cockroachdb/cockroach/pkg/clusterversion" | ||
| "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" | ||
| "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" | ||
| "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scplan/internal/opgen" | ||
| . "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scplan/internal/rules" | ||
| "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/screl" | ||
| "github.com/cockroachdb/cockroach/pkg/sql/types" | ||
| "github.com/cockroachdb/errors" | ||
| ) | ||
|
|
||
| // TestRuleAssertions verifies that important helper functions verify certain | ||
| // properties that the rule definitions rely on. | ||
| func TestRuleAssertions(t *testing.T) { | ||
| for _, fn := range []func(e scpb.Element) error{ | ||
| checkSimpleDependentsReferenceDescID, | ||
| checkToAbsentCategories, | ||
| checkIsWithTypeT, | ||
| checkIsWithExpression, | ||
| checkIsColumnDependent, | ||
| checkIsIndexDependent, | ||
| checkIsConstraintDependent, | ||
| } { | ||
| version := clusterversion.ClusterVersion{Version: clusterversion.ByKey(clusterversion.V22_2)} | ||
| var fni interface{} = fn | ||
| fullName := runtime.FuncForPC(reflect.ValueOf(fni).Pointer()).Name() | ||
| nameParts := strings.Split(fullName, "rules.") | ||
| shortName := nameParts[len(nameParts)-1] | ||
| t.Run(shortName, func(t *testing.T) { | ||
| _ = ForEachElementInActiveVersion(func(e scpb.Element) error { | ||
| e = nonNilElement(e) | ||
| if err := fn(e); err != nil { | ||
| t.Errorf("%T: %+v", e, err) | ||
| } | ||
| return nil | ||
| }, version) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func nonNilElement(element scpb.Element) scpb.Element { | ||
| return reflect.New(reflect.ValueOf(element).Type().Elem()).Interface().(scpb.Element) | ||
| } | ||
|
|
||
| // Assert that only simple dependents (non-descriptor, non-index, non-column) | ||
| // have screl.ReferencedDescID attributes. | ||
| func checkSimpleDependentsReferenceDescID(e scpb.Element) error { | ||
| if IsSimpleDependent(e) { | ||
| return nil | ||
| } | ||
| if _, ok := e.(*scpb.ForeignKeyConstraint); ok { | ||
| return nil | ||
| } | ||
| if _, err := screl.Schema.GetAttribute(screl.ReferencedDescID, e); err == nil { | ||
| return errors.New("unexpected screl.ReferencedDescID attr") | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // Assert that elements can be grouped into three categories when transitioning | ||
| // from PUBLIC to ABSENT: | ||
| // - go via DROPPED iff they're descriptor elements | ||
| // - go via a non-read status iff they're indexes or columns, which are | ||
| // subject to the two-version invariant. | ||
| // - go direct to ABSENT in all other cases. | ||
| func checkToAbsentCategories(e scpb.Element) error { | ||
| s0 := opgen.InitialStatus(e, scpb.Status_ABSENT) | ||
| s1 := opgen.NextStatus(e, scpb.Status_ABSENT, s0) | ||
| switch s1 { | ||
| case scpb.Status_TXN_DROPPED, scpb.Status_DROPPED: | ||
| if IsDescriptor(e) { | ||
| return nil | ||
| } | ||
| case scpb.Status_VALIDATED, scpb.Status_WRITE_ONLY, scpb.Status_DELETE_ONLY: | ||
| if IsSubjectTo2VersionInvariant(e) { | ||
| return nil | ||
| } | ||
| case scpb.Status_ABSENT: | ||
| if IsSimpleDependent(e) { | ||
| return nil | ||
| } | ||
| } | ||
| return errors.Newf("unexpected transition %s -> %s in direction ABSENT", s0, s1) | ||
| } | ||
|
|
||
| // Assert that isWithTypeT covers all elements with embedded TypeTs. | ||
| func checkIsWithTypeT(e scpb.Element) error { | ||
| return screl.WalkTypes(e, func(t *types.T) error { | ||
| if IsWithTypeT(e) { | ||
| return nil | ||
| } | ||
| return errors.New("should verify isWithTypeT but doesn't") | ||
| }) | ||
| } | ||
|
|
||
| // Assert that isWithExpression covers all elements with embedded | ||
| // expressions. | ||
| func checkIsWithExpression(e scpb.Element) error { | ||
| return screl.WalkExpressions(e, func(t *catpb.Expression) error { | ||
| switch e.(type) { | ||
| // Ignore elements which have catpb.Expression fields but which don't | ||
| // have them within an scpb.Expression for valid reasons. | ||
| case *scpb.RowLevelTTL: | ||
| return nil | ||
| } | ||
| if IsWithExpression(e) { | ||
| return nil | ||
| } | ||
| return errors.New("should verify isWithExpression but doesn't") | ||
| }) | ||
| } | ||
|
|
||
| // Assert that rules.IsColumnDependent covers all dependent elements of a column | ||
| // element. | ||
| func checkIsColumnDependent(e scpb.Element) error { | ||
| // Exclude columns themselves. | ||
| if IsColumn(e) { | ||
| return nil | ||
| } | ||
| // A column dependent should have a ColumnID attribute. | ||
| _, err := screl.Schema.GetAttribute(screl.ColumnID, e) | ||
| if IsColumnDependent(e) { | ||
| if err != nil { | ||
| return errors.New("verifies rules.IsColumnDependent but doesn't have ColumnID attr") | ||
| } | ||
| } else if err == nil { | ||
| return errors.New("has ColumnID attr but doesn't verify rules.IsColumnDependent") | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // Assert that rules.IsIndexDependent covers all dependent elements of an index | ||
| // element. | ||
| func checkIsIndexDependent(e scpb.Element) error { | ||
| // Exclude indexes themselves. | ||
| if IsIndex(e) || IsSupportedNonIndexBackedConstraint(e) { | ||
| return nil | ||
| } | ||
| // Skip check constraints, in 22.2 these didn't have | ||
| // index IDs. | ||
| if _, ok := e.(*scpb.CheckConstraint); ok { | ||
| return nil | ||
| } | ||
| // An index dependent should have an IndexID attribute. | ||
| _, err := screl.Schema.GetAttribute(screl.IndexID, e) | ||
| if IsIndexDependent(e) { | ||
| if err != nil { | ||
| return errors.New("verifies rules.IsIndexDependent but doesn't have IndexID attr") | ||
| } | ||
| } else if err == nil { | ||
| return errors.New("has IndexID attr but doesn't verify rules.IsIndexDependent") | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // Assert that checkIsConstraintDependent covers all elements of a constraint | ||
| // element. | ||
| func checkIsConstraintDependent(e scpb.Element) error { | ||
| // Exclude constraints themselves. | ||
| if IsConstraint(e) { | ||
| return nil | ||
| } | ||
| // A constraint dependent should have a ConstraintID attribute. | ||
| _, err := screl.Schema.GetAttribute(screl.ConstraintID, e) | ||
| if IsConstraintDependent(e) { | ||
| if err != nil { | ||
| return errors.New("verifies rules.IsConstraintDependent but doesn't have ConstraintID attr") | ||
| } | ||
| } else if err == nil { | ||
| return errors.New("has ConstraintID attr but doesn't verify rules.IsConstraintDependent") | ||
| } | ||
| return nil | ||
| } |
Oops, something went wrong.