Skip to content

Commit

Permalink
sql: add unit tests for planning inside the new schema changer
Browse files Browse the repository at this point in the history
Previously, the new schema changer did not have any unit
tests covering the planning capability. This was inadequate,
because we had no way of detect if plans were regressing with
changes or enhancements. To address this, this patch adds
basic tests to see if the operators/dependencies for a given
command are sane.
Release note: None
  • Loading branch information
fqazi committed Jun 3, 2021
1 parent 3b368d1 commit a150367
Show file tree
Hide file tree
Showing 9 changed files with 1,015 additions and 13 deletions.
8 changes: 4 additions & 4 deletions pkg/sql/schemachanger/scgraphviz/graphviz.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,11 @@ func htmlLabel(o interface{}) dot.HTML {
return dot.HTML(buf.String())
}

// toMap converts a struct to a map, field by field. If at any point a protobuf
// ToMap converts a struct to a map, field by field. If at any point a protobuf
// message is encountered, it is converted to a map using jsonpb to marshal it
// to json and then marshaling it back to a map. This approach allows zero
// values to be effectively omitted.
func toMap(v interface{}) (interface{}, error) {
func ToMap(v interface{}) (interface{}, error) {
if v == nil {
return nil, nil
}
Expand Down Expand Up @@ -202,7 +202,7 @@ func toMap(v interface{}) (interface{}, error) {
continue
}
var err error
if m[vt.Field(i).Name], err = toMap(vvf.Interface()); err != nil {
if m[vt.Field(i).Name], err = ToMap(vvf.Interface()); err != nil {
return nil, err
}
}
Expand Down Expand Up @@ -235,7 +235,7 @@ var objectTemplate = template.Must(template.New("obj").Funcs(template.FuncMap{
"isStruct": func(v interface{}) bool {
return reflect.Indirect(reflect.ValueOf(v)).Kind() == reflect.Struct
},
"toMap": toMap,
"toMap": ToMap,
}).Parse(`
{{- define "key" -}}
<td>
Expand Down
8 changes: 8 additions & 0 deletions pkg/sql/schemachanger/scpb/elements.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,12 @@ func NewTarget(dir Target_Direction, elem Element) *Target {
type ElementTypeID int

var typeToElementID map[reflect.Type]ElementTypeID
var elementIDToString map[ElementTypeID]string

func init() {
typ := reflect.TypeOf((*ElementProto)(nil)).Elem()
typeToElementID = make(map[reflect.Type]ElementTypeID, typ.NumField())
elementIDToString = make(map[ElementTypeID]string, typ.NumField())
for i := 0; i < typ.NumField(); i++ {
f := typ.Field(i)
protoFlags := strings.Split(f.Tag.Get("protobuf"), ",")
Expand All @@ -75,6 +77,7 @@ func init() {
panic(errors.Wrapf(err, "failed to extract ID from protobuf tag: %q", protoFlags))
}
typeToElementID[f.Type] = ElementTypeID(id)
elementIDToString[ElementTypeID(id)] = f.Type.String()
}
}

Expand All @@ -83,6 +86,11 @@ func ElementType(el Element) ElementTypeID {
return typeToElementID[reflect.TypeOf(el)]
}

// ElementIDToString determines the type ID of a element
func ElementIDToString(id ElementTypeID) string {
return elementIDToString[id]
}

// DescriptorID implements the Element interface.
func (e *Column) DescriptorID() descpb.ID { return e.TableID }

Expand Down
39 changes: 38 additions & 1 deletion pkg/sql/schemachanger/scplan/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "scplan",
Expand All @@ -21,3 +21,40 @@ go_library(
"@com_github_cockroachdb_errors//:errors",
],
)

go_test(
name = "scplan_test",
srcs = [
"main_test.go",
"plan_test.go",
],
data = glob(["testdata/**"]),
deps = [
":scplan",
"//pkg/base",
"//pkg/kv",
"//pkg/security",
"//pkg/security/securitytest",
"//pkg/server",
"//pkg/sql",
"//pkg/sql/catalog/descpb",
"//pkg/sql/catalog/descs",
"//pkg/sql/catalog/resolver",
"//pkg/sql/parser",
"//pkg/sql/schemachanger/scbuild",
"//pkg/sql/schemachanger/scgraph",
"//pkg/sql/schemachanger/scgraphviz",
"//pkg/sql/schemachanger/scpb",
"//pkg/sql/sem/tree",
"//pkg/sql/sessiondatapb",
"//pkg/testutils/serverutils",
"//pkg/testutils/sqlutils",
"//pkg/testutils/testcluster",
"//pkg/util/leaktest",
"//pkg/util/log",
"//pkg/util/randutil",
"@com_github_cockroachdb_datadriven//:datadriven",
"@com_github_stretchr_testify//require",
"@in_gopkg_yaml_v2//:yaml_v2",
],
)
31 changes: 31 additions & 0 deletions pkg/sql/schemachanger/scplan/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2021 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 scplan_test

import (
"os"
"testing"

"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/security/securitytest"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
"github.com/cockroachdb/cockroach/pkg/util/randutil"
)

func TestMain(m *testing.M) {
security.SetAssetLoader(securitytest.EmbeddedAssets)
randutil.SeedForTests()
serverutils.InitTestServerFactory(server.TestServerFactory)
serverutils.InitTestClusterFactory(testcluster.TestClusterFactory)
os.Exit(m.Run())
}
23 changes: 15 additions & 8 deletions pkg/sql/schemachanger/scplan/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ type Params struct {
//
// This doesn't do anything right now.
CreatedDescriptorIDs catalog.DescriptorIDSet
// DisableOpRandomization disables randomization for the final set of
// operations.
DisableOpRandomization bool
}

// A Plan is a schema change plan, primarily containing ops to be executed that
Expand Down Expand Up @@ -110,7 +113,7 @@ func MakePlan(initialStates []*scpb.Node, params Params) (_ Plan, err error) {
}); err != nil {
return Plan{}, err
}
stages := buildStages(initialStates, g)
stages := buildStages(initialStates, g, params)
return Plan{
Params: params,
InitialNodes: initialStates,
Expand All @@ -119,7 +122,7 @@ func MakePlan(initialStates []*scpb.Node, params Params) (_ Plan, err error) {
}, nil
}

func buildStages(init []*scpb.Node, g *scgraph.Graph) []Stage {
func buildStages(init []*scpb.Node, g *scgraph.Graph, params Params) []Stage {
// TODO(ajwerner): deal with the case where the target state was
// fulfilled by something that preceded the initial state.
cur := init
Expand Down Expand Up @@ -236,12 +239,15 @@ func buildStages(init []*scpb.Node, g *scgraph.Graph) []Stage {
// be order independent, however we will
// try to execute non-failing ones first.
opsSlice := s.Ops.Slice()
rand.Seed(timeutil.Now().UnixNano())
rand.Shuffle(len(opsSlice), func(i, j int) {
tmp := opsSlice[i]
opsSlice[i] = opsSlice[j]
opsSlice[j] = tmp
})
if !params.DisableOpRandomization {

rand.Seed(timeutil.Now().UnixNano())
rand.Shuffle(len(opsSlice), func(i, j int) {
tmp := opsSlice[i]
opsSlice[i] = opsSlice[j]
opsSlice[j] = tmp
})
}
// Place non-revertible operations at the end
sort.SliceStable(opsSlice, func(i, j int) bool {
if opsSlice[i].Revertible() == opsSlice[j].Revertible() {
Expand All @@ -251,6 +257,7 @@ func buildStages(init []*scpb.Node, g *scgraph.Graph) []Stage {
})
stages = append(stages, s)
cur = s.After

}
return stages
}
Loading

0 comments on commit a150367

Please sign in to comment.