Incremental backup: accept GTID position without 'MySQL56/' flavor pr…

…efix (#13474)

* Incremental backup: accept GTID position without 'MySQL56/' flavor prefix

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* testing invalid cases

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* license year

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

---------

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

go/mysql/replication_position.go

@@ -145,6 +145,25 @@ func DecodePosition(s string) (rp Position, err error) {
 	return ParsePosition(flav, gtid)
 }
 
+// DecodePositionDefaultFlavor converts a string in the format returned by
+// EncodePosition back into a Position value with the
+// correct underlying flavor. If the string does not indicate a flavor, then the 'flavor' argument
+// is used. For example:
+// - DecodePositionDefaultFlavor("MySQL56/16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615", "foo"): "MySQL56" explicitly indicated, this is the flavor.
+// - DecodePositionDefaultFlavor("16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615", "MySQL56"): No flavor indicated in `s`, therefore using "MySQL56"
+func DecodePositionDefaultFlavor(s string, flavor string) (rp Position, err error) {
+	if s == "" {
+		return rp, nil
+	}
+
+	flav, gtid, ok := strings.Cut(s, "/")
+	if !ok {
+		gtid = s
+		flav = flavor
+	}
+	return ParsePosition(flav, gtid)
+}
+
 // ParsePosition calls the parser for the specified flavor.
 func ParsePosition(flavor, value string) (rp Position, err error) {
 	parser := gtidSetParsers[flavor]

go/mysql/replication_position_test.go

@@ -272,6 +272,24 @@ func TestDecodePosition(t *testing.T) {
 
 }
 
+func TestDecodePositionDefaultFlavor(t *testing.T) {
+	gtidSetParsers[Mysql56FlavorID] = func(s string) (GTIDSet, error) {
+		return ParseMysql56GTIDSet(s)
+	}
+	{
+		pos := "MySQL56/16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615"
+		rp, err := DecodePositionDefaultFlavor(pos, "foo")
+		assert.NoError(t, err)
+		assert.Equal(t, "16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615", rp.GTIDSet.String())
+	}
+	{
+		pos := "16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615"
+		rp, err := DecodePositionDefaultFlavor(pos, Mysql56FlavorID)
+		assert.NoError(t, err)
+		assert.Equal(t, "16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615", rp.GTIDSet.String())
+	}
+}
+
 func TestDecodePositionZero(t *testing.T) {
 	input := ""
 	want := Position{}

go/vt/mysqlctl/builtinbackupengine.go

@@ -208,6 +208,22 @@ func (be *BuiltinBackupEngine) ExecuteBackup(ctx context.Context, params BackupP
 	return be.executeFullBackup(ctx, params, bh)
 }
 
+// getIncrementalFromPosGTIDSet turns the given string into a valid Mysql56GTIDSet
+func getIncrementalFromPosGTIDSet(incrementalFromPos string) (mysql.Mysql56GTIDSet, error) {
+	pos, err := mysql.DecodePositionDefaultFlavor(incrementalFromPos, mysql.Mysql56FlavorID)
+	if err != nil {
+		return nil, vterrors.Wrapf(err, "cannot decode position in incremental backup: %v", incrementalFromPos)
+	}
+	if !pos.MatchesFlavor(mysql.Mysql56FlavorID) {
+		return nil, vterrors.Errorf(vtrpc.Code_FAILED_PRECONDITION, "incremental backup only supports MySQL GTID positions. Got: %v", incrementalFromPos)
+	}
+	ifPosGTIDSet, ok := pos.GTIDSet.(mysql.Mysql56GTIDSet)
+	if !ok {
+		return nil, vterrors.Errorf(vtrpc.Code_FAILED_PRECONDITION, "cannot get MySQL GTID value: %v", pos)
+	}
+	return ifPosGTIDSet, nil
+}
+
 // executeIncrementalBackup runs an incremental backup, based on given 'incremental_from_pos', which can be:
 // - A valid position
 // - "auto", indicating the incremental backup should begin with last successful backup end position.
@@ -264,21 +280,7 @@ func (be *BuiltinBackupEngine) executeIncrementalBackup(ctx context.Context, par
 	}
 
 	// params.IncrementalFromPos is a string. We want to turn that into a MySQL GTID
-	getIncrementalFromPosGTIDSet := func() (mysql.Mysql56GTIDSet, error) {
-		pos, err := mysql.DecodePosition(params.IncrementalFromPos)
-		if err != nil {
-			return nil, vterrors.Wrapf(err, "cannot decode position in incremental backup: %v", params.IncrementalFromPos)
-		}
-		if !pos.MatchesFlavor(mysql.Mysql56FlavorID) {
-			return nil, vterrors.Errorf(vtrpc.Code_FAILED_PRECONDITION, "incremental backup only supports MySQL GTID positions. Got: %v", params.IncrementalFromPos)
-		}
-		ifPosGTIDSet, ok := pos.GTIDSet.(mysql.Mysql56GTIDSet)
-		if !ok {
-			return nil, vterrors.Errorf(vtrpc.Code_FAILED_PRECONDITION, "cannot get MySQL GTID value: %v", pos)
-		}
-		return ifPosGTIDSet, nil
-	}
-	backupFromGTIDSet, err := getIncrementalFromPosGTIDSet()
+	backupFromGTIDSet, err := getIncrementalFromPosGTIDSet(params.IncrementalFromPos)
 	if err != nil {
 		return false, err
 	}

go/vt/mysqlctl/builtinbackupengine2_test.go

@@ -0,0 +1,69 @@
+/*
+Copyright 2023 The Vitess Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package mysqlctl_test is the blackbox tests for package mysqlctl.
+package mysqlctl
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetIncrementalFromPosGTIDSet(t *testing.T) {
+	tcases := []struct {
+		incrementalFromPos string
+		gtidSet            string
+		expctError         bool
+	}{
+		{
+			"MySQL56/16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615",
+			"16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615",
+			false,
+		},
+		{
+			"16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615",
+			"16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615",
+			false,
+		},
+		{
+			"MySQL56/16b1039f-22b6-11ed-b765-0a43f95f28a3",
+			"",
+			true,
+		},
+		{
+			"MySQL56/invalid",
+			"",
+			true,
+		},
+		{
+			"16b1039f-22b6-11ed-b765-0a43f95f28a3",
+			"",
+			true,
+		},
+	}
+	for _, tcase := range tcases {
+		t.Run(tcase.incrementalFromPos, func(t *testing.T) {
+			gtidSet, err := getIncrementalFromPosGTIDSet(tcase.incrementalFromPos)
+			if tcase.expctError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tcase.gtidSet, gtidSet.String())
+			}
+		})
+	}
+}