sql: Add unit tests for planning inside the new schema changer

pkg/BUILD.bazel

@@ -254,6 +254,7 @@ ALL_TESTS = [
     "//pkg/sql/schemachanger/scbuild:scbuild_test",
     "//pkg/sql/schemachanger/scexec:scexec_test",
     "//pkg/sql/schemachanger/scpb:scpb_test",
+    "//pkg/sql/schemachanger/scplan:scplan_test",
     "//pkg/sql/schemachanger:schemachanger_test",
     "//pkg/sql/sem/builtins:builtins_test",
     "//pkg/sql/sem/tree/eval_test:eval_test_test",

pkg/sql/schemachanger/scgraphviz/graphviz.go

@@ -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
 	}
@@ -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
 		}
 	}
@@ -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>

pkg/sql/schemachanger/scpb/attribute.go

@@ -61,7 +61,7 @@ func (id IndexID) String() string {
 
 // String implements AttributeValue.
 func (id ElementTypeID) String() string {
-	return strconv.Itoa(int(id))
+	return ElementIDToString(id)
 }
 
 // String implements AttributeValue.
@@ -146,16 +146,20 @@ func (a Attributes) Equal(other Attributes) bool {
 // String seralizes attribute into a string
 func (a Attributes) String() string {
 	result := strings.Builder{}
-	result.WriteString("[ ")
+	result.WriteString(a.Get(AttributeType).String())
+	result.WriteString(":{")
 	for attribIdx, attrib := range a.values {
+		if attrib.key == AttributeType {
+			continue
+		}
 		result.WriteString(attrib.key.String())
 		result.WriteString(": ")
 		result.WriteString(attrib.value.String())
 		if attribIdx < len(a.values)-1 {
 			result.WriteString(", ")
 		}
 	}
-	result.WriteString(" ]")
+	result.WriteString("}")
 	return result.String()
 }
 

pkg/sql/schemachanger/scpb/attribute_test.go

@@ -30,22 +30,22 @@ func TestGetAttribute(t *testing.T) {
 
 	// Sanity: Validate basic string conversion, equality,
 	// and inequality.
-	expectedStr := `[ Type: 4, DescID: 3, DepID: 1, ColumnID: 2 ]`
+	expectedStr := `SequenceDependency:{DescID: 3, DepID: 1, ColumnID: 2}`
 	require.Equal(t, expectedStr, seqElem.GetAttributes().String(), "Attribute string conversion is broken.")
 	require.True(t, seqElem.GetAttributes().Equal(seqElem.GetAttributes()))
 	require.False(t, seqElem.GetAttributes().Equal(seqElemDiff.GetAttributes()))
 
 	// Sanity: Validate type references, then check if type comparisons
 	// work.
 	typeBackRef := TypeReference{TypeID: 3, DescID: 1}
-	expectedStr = `[ Type: 10, DescID: 3, DepID: 1 ]`
+	expectedStr = `TypeReference:{DescID: 3, DepID: 1}`
 	require.Equal(t, expectedStr, typeBackRef.GetAttributes().String(), "Attribute string conversion is broken.")
 	require.False(t, seqElem.GetAttributes().Equal(typeBackRef.GetAttributes()))
 	require.False(t, typeBackRef.GetAttributes().Equal(seqElem.GetAttributes()))
 
 	// Sanity: Validate attribute fetching for both types.
 	require.Equal(t, "1", typeBackRef.GetAttributes().Get(AttributeDepID).String())
 	require.Equal(t, "3", typeBackRef.GetAttributes().Get(AttributeDescID).String())
-	require.Equal(t, "10", typeBackRef.GetAttributes().Get(AttributeType).String())
+	require.Equal(t, "TypeReference", typeBackRef.GetAttributes().Get(AttributeType).String())
 	require.Equal(t, "4", seqElemDiff.GetAttributes().Get(AttributeColumnID).String())
 }

pkg/sql/schemachanger/scpb/elements.go

@@ -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"), ",")
@@ -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)] = strings.TrimPrefix(f.Type.String(), "*scpb.")
 	}
 }
 
@@ -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 }
 

pkg/sql/schemachanger/scplan/BUILD.bazel

@@ -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",
@@ -21,3 +21,41 @@ 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_cockroachdb_errors//:errors",
+        "@com_github_stretchr_testify//require",
+        "@in_gopkg_yaml_v2//:yaml_v2",
+    ],
+)

pkg/sql/schemachanger/scplan/main_test.go

@@ -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())
+}

pkg/sql/schemachanger/scplan/plan.go

@@ -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
@@ -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,
@@ -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
@@ -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() {
@@ -251,6 +257,7 @@ func buildStages(init []*scpb.Node, g *scgraph.Graph) []Stage {
 		})
 		stages = append(stages, s)
 		cur = s.After
+
 	}
 	return stages
 }