VReplication: Replace most usage of SimulatedNulls (#16734)

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

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

@@ -21,13 +21,12 @@ import (
 	"sort"
 	"strings"
 
-	"vitess.io/vitess/go/vt/proto/vtrpc"
-	"vitess.io/vitess/go/vt/vterrors"
-
 	"github.com/spf13/cobra"
 
 	"vitess.io/vitess/go/cmd/vtctldclient/cli"
 	"vitess.io/vitess/go/textutil"
+	"vitess.io/vitess/go/vt/proto/vtrpc"
+	"vitess.io/vitess/go/vt/vterrors"
 
 	binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
 	tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata"
@@ -143,9 +142,8 @@ func commandUpdateState(cmd *cobra.Command, args []string) error {
 		TabletRequest: &tabletmanagerdatapb.UpdateVReplicationWorkflowRequest{
 			Workflow:    workflowUpdateOptions.Workflow,
 			Cells:       textutil.SimulatedNullStringSlice,
-			TabletTypes: []topodatapb.TabletType{topodatapb.TabletType(textutil.SimulatedNullInt)},
-			OnDdl:       binlogdatapb.OnDDLAction(textutil.SimulatedNullInt),
-			State:       state,
+			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,9 +78,8 @@ func commandUpdateState(cmd *cobra.Command, args []string) error {
 		TabletRequest: &tabletmanagerdatapb.UpdateVReplicationWorkflowRequest{
 			Workflow:    baseOptions.Workflow,
 			Cells:       textutil.SimulatedNullStringSlice,
-			TabletTypes: []topodatapb.TabletType{topodatapb.TabletType(textutil.SimulatedNullInt)},
-			OnDdl:       binlogdatapb.OnDDLAction(textutil.SimulatedNullInt),
-			State:       state,
+			TabletTypes: textutil.SimulatedNullTabletTypeSlice,
+			State:       &state,
 		},
 	}
 

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

@@ -25,6 +25,7 @@ import (
 
 	"vitess.io/vitess/go/cmd/vtctldclient/cli"
 	"vitess.io/vitess/go/cmd/vtctldclient/command/vreplication/common"
+	"vitess.io/vitess/go/ptr"
 	"vitess.io/vitess/go/textutil"
 
 	binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
@@ -65,14 +66,14 @@ 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
 				if _, ok := binlogdatapb.OnDDLAction_value[strings.ToUpper(updateOptions.OnDDL)]; !ok {
 					return fmt.Errorf("invalid on-ddl value: %s", updateOptions.OnDDL)
 				}
-			} // Simulated NULL will need to be handled in command
+			}
 			if !changes {
 				return fmt.Errorf("no configuration options specified to update")
 			}
@@ -85,15 +86,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) // Simulated NULL when no value provided
-	if val, ok := binlogdatapb.OnDDLAction_value[strings.ToUpper(updateOptions.OnDDL)]; ok {
-		onddl = val
-	}
-
-	// Simulated NULL when no value is provided.
 	tsp := tabletmanagerdatapb.TabletSelectionPreference_UNKNOWN
 	if cmd.Flags().Lookup("tablet-types-in-order").Changed {
 		if updateOptions.TabletTypesInPreferenceOrder {
@@ -109,12 +101,17 @@ func commandUpdate(cmd *cobra.Command, args []string) error {
 			Workflow:                  baseOptions.Workflow,
 			Cells:                     updateOptions.Cells,
 			TabletTypes:               updateOptions.TabletTypes,
-			TabletSelectionPreference: tsp,
-			OnDdl:                     binlogdatapb.OnDDLAction(onddl),
-			State:                     binlogdatapb.VReplicationWorkflowState(textutil.SimulatedNullInt), // We don't allow changing this in the client command
+			TabletSelectionPreference: &tsp,
 		},
 	}
 
+	// 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 {
+		req.TabletRequest.OnDdl = ptr.Of(binlogdatapb.OnDDLAction(val))
+	}
+
 	resp, err := common.GetClient().WorkflowUpdate(common.GetCommandCtx(), req)
 	if err != nil {
 		return err

go/test/endtoend/vreplication/vdiff2_test.go

@@ -32,6 +32,7 @@ import (
 	"google.golang.org/protobuf/encoding/protojson"
 	"google.golang.org/protobuf/proto"
 
+	"vitess.io/vitess/go/ptr"
 	"vitess.io/vitess/go/test/endtoend/cluster"
 	"vitess.io/vitess/go/vt/log"
 	"vitess.io/vitess/go/vt/sqlparser"
@@ -370,7 +371,6 @@ func testCLIErrors(t *testing.T, ksWorkflow, cells string) {
 // testCLIFlagHandling tests that the vtctldclient CLI flags are handled correctly
 // from vtctldclient->vtctld->vttablet->mysqld.
 func testCLIFlagHandling(t *testing.T, targetKs, workflowName string, cell *Cell) {
-	false := false
 	expectedOptions := &tabletmanagerdatapb.VDiffOptions{
 		CoreOptions: &tabletmanagerdatapb.VDiffCoreOptions{
 			MaxRows:               999,
@@ -379,7 +379,7 @@ func testCLIFlagHandling(t *testing.T, targetKs, workflowName string, cell *Cell
 			UpdateTableStats:      true,
 			TimeoutSeconds:        60,
 			MaxDiffSeconds:        333,
-			AutoStart:             &false,
+			AutoStart:             ptr.Of(false),
 		},
 		PickerOptions: &tabletmanagerdatapb.VDiffPickerOptions{
 			SourceCell:  "zone1,zone2,zone3,zonefoosource",

go/textutil/strings.go

@@ -25,7 +25,6 @@ import (
 
 	"vitess.io/vitess/go/sqltypes"
 
-	binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
 	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
 )
 
@@ -37,10 +36,10 @@ const (
 )
 
 var (
-	delimitedListRegexp      = regexp.MustCompile(`[ ,;]+`)
-	SimulatedNullString      = sqltypes.NULL.String()
-	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
@@ -91,29 +90,16 @@ func SingleWordCamel(w string) string {
 	return strings.ToUpper(w[0:1]) + strings.ToLower(w[1:])
 }
 
-// ValueIsSimulatedNull returns true if the value represents
-// a NULL or unknown/unspecified value. This is used to
-// distinguish between a zero value / default and a user
-// provided value that is equivalent (e.g. an empty string
-// or slice).
+// ValueIsSimulatedNull returns true if the slice value represents
+// a NULL or unknown/unspecified value. This is used to distinguish
+// between a zero value empty slice and a user provided value of an
+// empty slice.
 func ValueIsSimulatedNull(val any) bool {
 	switch cval := val.(type) {
-	case string:
-		return cval == SimulatedNullString
 	case []string:
 		return len(cval) == 1 && cval[0] == sqltypes.NULL.String()
-	case binlogdatapb.OnDDLAction:
-		return int32(cval) == int32(SimulatedNullInt)
-	case int:
-		return cval == SimulatedNullInt
-	case int32:
-		return int32(cval) == int32(SimulatedNullInt)
-	case int64:
-		return int64(cval) == int64(SimulatedNullInt)
 	case []topodatapb.TabletType:
 		return len(cval) == 1 && cval[0] == topodatapb.TabletType(SimulatedNullInt)
-	case binlogdatapb.VReplicationWorkflowState:
-		return int32(cval) == int32(SimulatedNullInt)
 	default:
 		return false
 	}

go/textutil/strings_test.go

@@ -23,6 +23,8 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"vitess.io/vitess/go/sqltypes"
+
 	binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
 	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
 )
@@ -131,53 +133,38 @@ func TestValueIsSimulatedNull(t *testing.T) {
 			val:    "test",
 			isNull: false,
 		},
-		{
-			name:   "case string true",
-			val:    SimulatedNullString,
-			isNull: true,
-		},
 		{
 			name:   "case []string true",
-			val:    []string{SimulatedNullString},
+			val:    []string{sqltypes.NULL.String()},
 			isNull: true,
 		},
 		{
 			name:   "case []string false",
-			val:    []string{SimulatedNullString, SimulatedNullString},
+			val:    []string{sqltypes.NULL.String(), sqltypes.NULL.String()},
 			isNull: false,
 		},
 		{
-			name:   "case binlogdatapb.OnDDLAction true",
-			val:    binlogdatapb.OnDDLAction(SimulatedNullInt),
-			isNull: true,
-		},
-		{
-			name:   "case int true",
-			val:    SimulatedNullInt,
-			isNull: true,
-		},
-		{
-			name:   "case int32 true",
-			val:    int32(SimulatedNullInt),
-			isNull: true,
+			name:   "case binlogdatapb.OnDDLAction exec",
+			val:    binlogdatapb.OnDDLAction_EXEC,
+			isNull: false,
 		},
 		{
-			name:   "case int64 true",
-			val:    int64(SimulatedNullInt),
-			isNull: true,
+			name:   "case int false",
+			val:    1,
+			isNull: false,
 		},
 		{
 			name:   "case []topodatapb.TabletType true",
 			val:    []topodatapb.TabletType{topodatapb.TabletType(SimulatedNullInt)},
 			isNull: true,
 		},
 		{
-			name:   "case binlogdatapb.VReplicationWorkflowState true",
-			val:    binlogdatapb.VReplicationWorkflowState(SimulatedNullInt),
-			isNull: true,
+			name:   "case binlogdatapb.VReplicationWorkflowState running",
+			val:    binlogdatapb.VReplicationWorkflowState_Running,
+			isNull: false,
 		},
 		{
-			name:   "case default",
+			name:   "case float false",
 			val:    float64(1),
 			isNull: false,
 		},