Changes from self review

Signed-off-by: Matt Lord <[email protected]>

go/cmd/vtctldclient/command/vreplication/common/update.go

@@ -143,7 +143,7 @@ func commandUpdateState(cmd *cobra.Command, args []string) error {
 		TabletRequest: &tabletmanagerdatapb.UpdateVReplicationWorkflowRequest{
 			Workflow:    workflowUpdateOptions.Workflow,
 			Cells:       textutil.SimulatedNullStringSlice,
-			TabletTypes: []topodatapb.TabletType{topodatapb.TabletType(textutil.SimulatedNullInt)},
+			TabletTypes: textutil.SimulatedNullTabletTypeSlice,
 			State:       &state,
 		},
 	}

go/cmd/vtctldclient/command/vreplication/workflow/state.go

@@ -29,7 +29,6 @@ import (
 
 	binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
 	tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata"
-	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
 	vtctldatapb "vitess.io/vitess/go/vt/proto/vtctldata"
 )
 
@@ -79,7 +78,7 @@ func commandUpdateState(cmd *cobra.Command, args []string) error {
 		TabletRequest: &tabletmanagerdatapb.UpdateVReplicationWorkflowRequest{
 			Workflow:    baseOptions.Workflow,
 			Cells:       textutil.SimulatedNullStringSlice,
-			TabletTypes: []topodatapb.TabletType{topodatapb.TabletType(textutil.SimulatedNullInt)},
+			TabletTypes: textutil.SimulatedNullTabletTypeSlice,
 			State:       &state,
 		},
 	}

go/cmd/vtctldclient/command/vreplication/workflow/update.go

@@ -65,7 +65,7 @@ var (
 				}
 				changes = true
 			} else {
-				updateOptions.TabletTypes = []topodatapb.TabletType{topodatapb.TabletType(textutil.SimulatedNullInt)}
+				updateOptions.TabletTypes = textutil.SimulatedNullTabletTypeSlice
 			}
 			if cmd.Flags().Lookup("on-ddl").Changed { // Validate the provided value
 				changes = true
@@ -85,14 +85,6 @@ var (
 func commandUpdate(cmd *cobra.Command, args []string) error {
 	cli.FinishedParsing(cmd)
 
-	// We've already validated any provided value, if one WAS provided.
-	// Now we need to do the mapping from the string representation to
-	// the enum value.
-	onddl := int32(textutil.SimulatedNullInt)
-	if val, ok := binlogdatapb.OnDDLAction_value[strings.ToUpper(updateOptions.OnDDL)]; ok {
-		onddl = val
-	}
-
 	tsp := tabletmanagerdatapb.TabletSelectionPreference_UNKNOWN
 	if cmd.Flags().Lookup("tablet-types-in-order").Changed {
 		if updateOptions.TabletTypesInPreferenceOrder {
@@ -111,8 +103,12 @@ func commandUpdate(cmd *cobra.Command, args []string) error {
 			TabletSelectionPreference: &tsp,
 		},
 	}
-	if onddl != int32(textutil.SimulatedNullInt) {
-		v := binlogdatapb.OnDDLAction(onddl)
+
+	// We've already validated any provided value, if one WAS provided.
+	// Now we need to do the mapping from the string representation to
+	// the enum value.
+	if val, ok := binlogdatapb.OnDDLAction_value[strings.ToUpper(updateOptions.OnDDL)]; ok {
+		v := binlogdatapb.OnDDLAction(val)
 		req.TabletRequest.OnDdl = &v
 	}
 

go/textutil/strings.go

@@ -36,9 +36,10 @@ const (
 )
 
 var (
-	delimitedListRegexp      = regexp.MustCompile(`[ ,;]+`)
-	SimulatedNullStringSlice = []string{sqltypes.NULL.String()}
-	SimulatedNullInt         = -1
+	delimitedListRegexp          = regexp.MustCompile(`[ ,;]+`)
+	SimulatedNullStringSlice     = []string{sqltypes.NULL.String()}
+	SimulatedNullTabletTypeSlice = []topodatapb.TabletType{topodatapb.TabletType(SimulatedNullInt)}
+	SimulatedNullInt             = -1
 )
 
 // SplitDelimitedList splits a given string by comma, semi-colon or space, and returns non-empty strings

go/vt/vtctl/vtctl.go

@@ -3805,7 +3805,7 @@ func commandWorkflow(ctx context.Context, wr *wrangler.Wrangler, subFlags *pflag
 					}
 				}
 			} else {
-				tabletTypes = []topodatapb.TabletType{topodatapb.TabletType(textutil.SimulatedNullInt)}
+				tabletTypes = textutil.SimulatedNullTabletTypeSlice
 			}
 			onddl := int32(textutil.SimulatedNullInt) // To signify no value has been provided
 			if subFlags.Lookup("on-ddl").Changed {    // Validate the provided value

go/vt/vtctl/workflow/materializer.go

@@ -42,7 +42,6 @@ import (
 
 	binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
 	tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata"
-	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
 	vschemapb "vitess.io/vitess/go/vt/proto/vschema"
 	vtctldatapb "vitess.io/vitess/go/vt/proto/vtctldata"
 )
@@ -500,10 +499,8 @@ func (mz *materializer) startStreams(ctx context.Context) error {
 			Workflow: mz.ms.Workflow,
 			State:    &running,
 			// Don't change anything else, so pass simulated NULLs.
-			Cells: textutil.SimulatedNullStringSlice,
-			TabletTypes: []topodatapb.TabletType{
-				topodatapb.TabletType(textutil.SimulatedNullInt),
-			},
+			Cells:       textutil.SimulatedNullStringSlice,
+			TabletTypes: textutil.SimulatedNullTabletTypeSlice,
 		}); err != nil {
 			return vterrors.Wrap(err, "failed to update workflow")
 		}

go/vt/vttablet/tabletmanager/rpc_vreplication_test.go

@@ -621,7 +621,7 @@ func TestUpdateVReplicationWorkflow(t *testing.T) {
 			request: &tabletmanagerdatapb.UpdateVReplicationWorkflowRequest{
 				Workflow:    workflow,
 				Cells:       []string{"zone3"},
-				TabletTypes: []topodatapb.TabletType{topodatapb.TabletType(textutil.SimulatedNullInt)}, // So keep the current value of replica
+				TabletTypes: textutil.SimulatedNullTabletTypeSlice, // So keep the current value of replica
 			},
 			query: fmt.Sprintf(`update _vt.vreplication set state = 'Running', source = 'keyspace:"%s" shard:"%s" filter:{rules:{match:"corder" filter:"select * from corder"} rules:{match:"customer" filter:"select * from customer"}}', cell = '%s', tablet_types = '%s' where id in (%d)`,
 				keyspace, shard, "zone3", tabletTypes[0], vreplID),
@@ -672,7 +672,7 @@ func TestUpdateVReplicationWorkflow(t *testing.T) {
 				Workflow:    workflow,
 				State:       &stopped,
 				Cells:       textutil.SimulatedNullStringSlice,
-				TabletTypes: []topodatapb.TabletType{topodatapb.TabletType(textutil.SimulatedNullInt)},
+				TabletTypes: textutil.SimulatedNullTabletTypeSlice,
 			},
 			query: fmt.Sprintf(`update _vt.vreplication set state = '%s', source = 'keyspace:"%s" shard:"%s" filter:{rules:{match:"corder" filter:"select * from corder"} rules:{match:"customer" filter:"select * from customer"}}', cell = '%s', tablet_types = '%s' where id in (%d)`,
 				binlogdatapb.VReplicationWorkflowState_Stopped.String(), keyspace, shard, cells[0], tabletTypes[0], vreplID),
@@ -683,7 +683,7 @@ func TestUpdateVReplicationWorkflow(t *testing.T) {
 				Workflow:    workflow,
 				State:       &running,
 				Cells:       textutil.SimulatedNullStringSlice,
-				TabletTypes: []topodatapb.TabletType{topodatapb.TabletType(textutil.SimulatedNullInt)},
+				TabletTypes: textutil.SimulatedNullTabletTypeSlice,
 			},
 			isCopying: true,
 			query: fmt.Sprintf(`update _vt.vreplication set state = 'Copying', source = 'keyspace:"%s" shard:"%s" filter:{rules:{match:"corder" filter:"select * from corder"} rules:{match:"customer" filter:"select * from customer"}}', cell = '%s', tablet_types = '%s' where id in (%d)`,

go/vt/wrangler/vexec_test.go

@@ -409,13 +409,13 @@ func TestWorkflowUpdate(t *testing.T) {
 		{
 			name:        "no flags",
 			cells:       nullSlice,
-			tabletTypes: []topodatapb.TabletType{topodatapb.TabletType(textutil.SimulatedNullInt)},
+			tabletTypes: textutil.SimulatedNullTabletTypeSlice,
 			wantErr:     "no updates were provided; use --cells, --tablet-types, or --on-ddl to specify new values",
 		},
 		{
 			name:        "only cells",
 			cells:       []string{"zone1"},
-			tabletTypes: []topodatapb.TabletType{topodatapb.TabletType(textutil.SimulatedNullInt)},
+			tabletTypes: textutil.SimulatedNullTabletTypeSlice,
 			output:      "The following workflow fields will be updated:\n  cells=\"zone1\"\nOn the following tablets in the target keyspace for workflow wrWorkflow:\n  zone1-0000000200 (target/-80)\n  zone1-0000000210 (target/80-)\n",
 		},
 		{
@@ -427,7 +427,7 @@ func TestWorkflowUpdate(t *testing.T) {
 		{
 			name:        "only on-ddl",
 			cells:       nullSlice,
-			tabletTypes: []topodatapb.TabletType{topodatapb.TabletType(textutil.SimulatedNullInt)},
+			tabletTypes: textutil.SimulatedNullTabletTypeSlice,
 			onDDL:       binlogdatapb.OnDDLAction_EXEC_IGNORE,
 			output:      "The following workflow fields will be updated:\n  on_ddl=\"EXEC_IGNORE\"\nOn the following tablets in the target keyspace for workflow wrWorkflow:\n  zone1-0000000200 (target/-80)\n  zone1-0000000210 (target/80-)\n",
 		},