OnlineDDL to use schemadiff version capabilities; refactor some `flav…

…or` code. (#14883)

Signed-off-by: Shlomi Noach <[email protected]>

go/mysql/flavor.go

@@ -38,6 +38,8 @@ var (
 
 	// ErrNoPrimaryStatus means no status was returned by ShowPrimaryStatus().
 	ErrNoPrimaryStatus = errors.New("no master status")
+
+	ErrUnspecifiedServerVersion = vterrors.Errorf(vtrpc.Code_INTERNAL, "server version unspecified")
 )
 
 const (
@@ -136,7 +138,7 @@ type flavor interface {
 	baseShowTables() string
 	baseShowTablesWithSizes() string
 
-	supportsCapability(serverVersion string, capability capabilities.FlavorCapability) (bool, error)
+	supportsCapability(capability capabilities.FlavorCapability) (bool, error)
 }
 
 // flavors maps flavor names to their implementation.
@@ -148,6 +150,9 @@ var flavors = make(map[string]func() flavor)
 // Example: if input is []int{8, 0, 23}... the function returns 'true' if we're
 // on MySQL 8.0.23, 8.0.24, ...
 func ServerVersionAtLeast(serverVersion string, parts ...int) (bool, error) {
+	if serverVersion == "" {
+		return false, ErrUnspecifiedServerVersion
+	}
 	versionPrefix := strings.Split(serverVersion, "-")[0]
 	versionTokens := strings.Split(versionPrefix, ".")
 	for i, part := range parts {
@@ -168,6 +173,13 @@ func ServerVersionAtLeast(serverVersion string, parts ...int) (bool, error) {
 	return true, nil
 }
 
+// flavorCapableOf is a utility function that returns a CapableOf function for a given flavor
+func flavorCapableOf(f flavor) capabilities.CapableOf {
+	return func(capability capabilities.FlavorCapability) (bool, error) {
+		return f.supportsCapability(capability)
+	}
+}
+
 // GetFlavor fills in c.Flavor. If the params specify the flavor,
 // that is used. Otherwise, we auto-detect.
 //
@@ -187,25 +199,29 @@ func GetFlavor(serverVersion string, flavorFunc func() flavor) (f flavor, capabl
 		f = flavorFunc()
 	case strings.HasPrefix(serverVersion, mariaDBReplicationHackPrefix):
 		canonicalVersion = serverVersion[len(mariaDBReplicationHackPrefix):]
-		f = mariadbFlavor101{}
+		f = mariadbFlavor101{mariadbFlavor{serverVersion: canonicalVersion}}
 	case strings.Contains(serverVersion, mariaDBVersionString):
 		mariadbVersion, err := strconv.ParseFloat(serverVersion[:4], 64)
 		if err != nil || mariadbVersion < 10.2 {
-			f = mariadbFlavor101{}
+			f = mariadbFlavor101{mariadbFlavor{serverVersion: fmt.Sprintf("%f", mariadbVersion)}}
 		} else {
-			f = mariadbFlavor102{}
+			f = mariadbFlavor102{mariadbFlavor{serverVersion: fmt.Sprintf("%f", mariadbVersion)}}
 		}
 	case strings.HasPrefix(serverVersion, mysql57VersionPrefix):
-		f = mysqlFlavor57{}
+		f = mysqlFlavor57{mysqlFlavor{serverVersion: serverVersion}}
 	case strings.HasPrefix(serverVersion, mysql80VersionPrefix):
-		f = mysqlFlavor80{}
+		f = mysqlFlavor80{mysqlFlavor{serverVersion: serverVersion}}
 	default:
-		f = mysqlFlavor56{}
+		f = mysqlFlavor56{mysqlFlavor{serverVersion: serverVersion}}
 	}
-	return f,
-		func(capability capabilities.FlavorCapability) (bool, error) {
-			return f.supportsCapability(serverVersion, capability)
-		}, canonicalVersion
+	return f, flavorCapableOf(f), canonicalVersion
+}
+
+// ServerVersionCapableOf is a convenience function that returns a CapableOf function given a server version.
+// It is a shortcut for GetFlavor(serverVersion, nil).
+func ServerVersionCapableOf(serverVersion string) (capableOf capabilities.CapableOf) {
+	_, capableOf, _ = GetFlavor(serverVersion, nil)
+	return capableOf
 }
 
 // fillFlavor fills in c.Flavor. If the params specify the flavor,
@@ -454,7 +470,7 @@ func (c *Conn) BaseShowTablesWithSizes() string {
 
 // SupportsCapability checks if the database server supports the given capability
 func (c *Conn) SupportsCapability(capability capabilities.FlavorCapability) (bool, error) {
-	return c.flavor.supportsCapability(c.ServerVersion, capability)
+	return c.flavor.supportsCapability(capability)
 }
 
 func init() {

go/mysql/flavor_filepos.go

@@ -336,7 +336,7 @@ func (*filePosFlavor) baseShowTablesWithSizes() string {
 }
 
 // supportsCapability is part of the Flavor interface.
-func (*filePosFlavor) supportsCapability(serverVersion string, capability capabilities.FlavorCapability) (bool, error) {
+func (*filePosFlavor) supportsCapability(capability capabilities.FlavorCapability) (bool, error) {
 	switch capability {
 	default:
 		return false, nil

go/mysql/flavor_mariadb.go

@@ -32,7 +32,9 @@ import (
 )
 
 // mariadbFlavor implements the Flavor interface for MariaDB.
-type mariadbFlavor struct{}
+type mariadbFlavor struct {
+	serverVersion string
+}
 type mariadbFlavor101 struct {
 	mariadbFlavor
 }
@@ -287,7 +289,7 @@ func (mariadbFlavor) readBinlogEvent(c *Conn) (BinlogEvent, error) {
 }
 
 // supportsCapability is part of the Flavor interface.
-func (mariadbFlavor) supportsCapability(serverVersion string, capability capabilities.FlavorCapability) (bool, error) {
+func (mariadbFlavor) supportsCapability(capability capabilities.FlavorCapability) (bool, error) {
 	switch capability {
 	default:
 		return false, nil

go/mysql/flavor_mysql.go

@@ -31,7 +31,9 @@ import (
 )
 
 // mysqlFlavor implements the Flavor interface for Mysql.
-type mysqlFlavor struct{}
+type mysqlFlavor struct {
+	serverVersion string
+}
 type mysqlFlavor56 struct {
 	mysqlFlavor
 }
@@ -372,7 +374,7 @@ func (mysqlFlavor56) baseShowTablesWithSizes() string {
 }
 
 // supportsCapability is part of the Flavor interface.
-func (mysqlFlavor56) supportsCapability(serverVersion string, capability capabilities.FlavorCapability) (bool, error) {
+func (mysqlFlavor56) supportsCapability(capability capabilities.FlavorCapability) (bool, error) {
 	switch capability {
 	default:
 		return false, nil
@@ -385,7 +387,7 @@ func (mysqlFlavor57) baseShowTablesWithSizes() string {
 }
 
 // supportsCapability is part of the Flavor interface.
-func (mysqlFlavor57) supportsCapability(serverVersion string, capability capabilities.FlavorCapability) (bool, error) {
+func (mysqlFlavor57) supportsCapability(capability capabilities.FlavorCapability) (bool, error) {
 	switch capability {
 	case capabilities.MySQLJSONFlavorCapability:
 		return true, nil
@@ -400,7 +402,10 @@ func (mysqlFlavor80) baseShowTablesWithSizes() string {
 }
 
 // supportsCapability is part of the Flavor interface.
-func (mysqlFlavor80) supportsCapability(serverVersion string, capability capabilities.FlavorCapability) (bool, error) {
+func (f mysqlFlavor80) supportsCapability(capability capabilities.FlavorCapability) (bool, error) {
+	serverVersionAtLeast := func(parts ...int) (bool, error) {
+		return ServerVersionAtLeast(f.serverVersion, parts...)
+	}
 	switch capability {
 	case capabilities.InstantDDLFlavorCapability,
 		capabilities.InstantExpandEnumCapability,
@@ -409,21 +414,21 @@ func (mysqlFlavor80) supportsCapability(serverVersion string, capability capabil
 		capabilities.InstantChangeColumnDefaultFlavorCapability:
 		return true, nil
 	case capabilities.InstantAddDropColumnFlavorCapability:
-		return ServerVersionAtLeast(serverVersion, 8, 0, 29)
+		return serverVersionAtLeast(8, 0, 29)
 	case capabilities.TransactionalGtidExecutedFlavorCapability:
-		return ServerVersionAtLeast(serverVersion, 8, 0, 17)
+		return serverVersionAtLeast(8, 0, 17)
 	case capabilities.FastDropTableFlavorCapability:
-		return ServerVersionAtLeast(serverVersion, 8, 0, 23)
+		return serverVersionAtLeast(8, 0, 23)
 	case capabilities.MySQLJSONFlavorCapability:
 		return true, nil
 	case capabilities.MySQLUpgradeInServerFlavorCapability:
-		return ServerVersionAtLeast(serverVersion, 8, 0, 16)
+		return serverVersionAtLeast(8, 0, 16)
 	case capabilities.DynamicRedoLogCapacityFlavorCapability:
-		return ServerVersionAtLeast(serverVersion, 8, 0, 30)
+		return serverVersionAtLeast(8, 0, 30)
 	case capabilities.DisableRedoLogFlavorCapability:
-		return ServerVersionAtLeast(serverVersion, 8, 0, 21)
+		return serverVersionAtLeast(8, 0, 21)
 	case capabilities.CheckConstraintsCapability:
-		return ServerVersionAtLeast(serverVersion, 8, 0, 16)
+		return serverVersionAtLeast(8, 0, 16)
 	case capabilities.PerformanceSchemaDataLocksTableCapability:
 		return true, nil
 	default:

go/mysql/flavor_mysqlgr.go

@@ -249,26 +249,31 @@ func (mysqlGRFlavor) baseShowTablesWithSizes() string {
 }
 
 // supportsCapability is part of the Flavor interface.
-func (mysqlGRFlavor) supportsCapability(serverVersion string, capability capabilities.FlavorCapability) (bool, error) {
+func (f mysqlGRFlavor) supportsCapability(capability capabilities.FlavorCapability) (bool, error) {
+	serverVersionAtLeast := func(parts ...int) (bool, error) {
+		return ServerVersionAtLeast(f.serverVersion, parts...)
+	}
 	switch capability {
 	case capabilities.InstantDDLFlavorCapability,
 		capabilities.InstantExpandEnumCapability,
 		capabilities.InstantAddLastColumnFlavorCapability,
 		capabilities.InstantAddDropVirtualColumnFlavorCapability,
 		capabilities.InstantChangeColumnDefaultFlavorCapability:
-		return ServerVersionAtLeast(serverVersion, 8, 0, 0)
+		return serverVersionAtLeast(8, 0, 0)
 	case capabilities.InstantAddDropColumnFlavorCapability:
-		return ServerVersionAtLeast(serverVersion, 8, 0, 29)
+		return serverVersionAtLeast(8, 0, 29)
 	case capabilities.TransactionalGtidExecutedFlavorCapability:
-		return ServerVersionAtLeast(serverVersion, 8, 0, 17)
+		return serverVersionAtLeast(8, 0, 17)
 	case capabilities.FastDropTableFlavorCapability:
-		return ServerVersionAtLeast(serverVersion, 8, 0, 23)
+		return serverVersionAtLeast(8, 0, 23)
 	case capabilities.MySQLJSONFlavorCapability:
-		return ServerVersionAtLeast(serverVersion, 5, 7, 0)
+		return serverVersionAtLeast(5, 7, 0)
 	case capabilities.MySQLUpgradeInServerFlavorCapability:
-		return ServerVersionAtLeast(serverVersion, 8, 0, 16)
+		return serverVersionAtLeast(8, 0, 16)
 	case capabilities.DynamicRedoLogCapacityFlavorCapability:
-		return ServerVersionAtLeast(serverVersion, 8, 0, 30)
+		return serverVersionAtLeast(8, 0, 30)
+	case capabilities.CheckConstraintsCapability:
+		return serverVersionAtLeast(8, 0, 16)
 	default:
 		return false, nil
 	}

go/mysql/flavor_test.go

@@ -44,6 +44,21 @@ func TestServerVersionAtLeast(t *testing.T) {
 			parts:   []int{8, 0, 13},
 			expect:  true,
 		},
+		{
+			version: "8.0.14-log",
+			parts:   []int{8, 0, 13},
+			expect:  true,
+		},
+		{
+			version: "8.0.14",
+			parts:   []int{8, 0, 15},
+			expect:  false,
+		},
+		{
+			version: "8.0.14-log",
+			parts:   []int{8, 0, 15},
+			expect:  false,
+		},
 		{
 			version: "8.0.14",
 			parts:   []int{7, 5, 20},
@@ -79,6 +94,11 @@ func TestServerVersionAtLeast(t *testing.T) {
 			parts:       []int{8, 0, 14},
 			expectError: true,
 		},
+		{
+			version:     "",
+			parts:       []int{8, 0, 14},
+			expectError: true,
+		},
 	}
 	for _, tc := range testcases {
 		result, err := ServerVersionAtLeast(tc.version, tc.parts...)
@@ -172,6 +192,11 @@ func TestGetFlavor(t *testing.T) {
 			capability: capabilities.CheckConstraintsCapability,
 			isCapable:  true,
 		},
+		{
+			version:    "8.0.20-log",
+			capability: capabilities.CheckConstraintsCapability,
+			isCapable:  true,
+		},
 		{
 			version:    "5.7.38",
 			capability: capabilities.PerformanceSchemaDataLocksTableCapability,
@@ -182,11 +207,23 @@ func TestGetFlavor(t *testing.T) {
 			capability: capabilities.PerformanceSchemaDataLocksTableCapability,
 			isCapable:  true,
 		},
+		{
+			// What happens if server version is unspecified
+			version:    "",
+			capability: capabilities.CheckConstraintsCapability,
+			isCapable:  false,
+		},
+		{
+			// Some ridiculous version
+			version:    "5914.234.17",
+			capability: capabilities.CheckConstraintsCapability,
+			isCapable:  false,
+		},
 	}
 	for _, tc := range testcases {
 		name := fmt.Sprintf("%s %v", tc.version, tc.capability)
 		t.Run(name, func(t *testing.T) {
-			_, capableOf, _ := GetFlavor(tc.version, nil)
+			capableOf := ServerVersionCapableOf(tc.version)
 			isCapable, err := capableOf(tc.capability)
 			assert.NoError(t, err)
 			assert.Equal(t, tc.isCapable, isCapable)

go/test/endtoend/onlineddl/revert/onlineddl_revert_test.go

@@ -560,7 +560,7 @@ func testRevert(t *testing.T) {
 	mysqlVersion = onlineddl.GetMySQLVersion(t, clusterInstance.Keyspaces[0].Shards[0].PrimaryTablet())
 	require.NotEmpty(t, mysqlVersion)
 
-	_, capableOf, _ := mysql.GetFlavor(mysqlVersion, nil)
+	capableOf := mysql.ServerVersionCapableOf(mysqlVersion)
 
 	var uuids []string
 	ddlStrategy := "online"

go/test/endtoend/onlineddl/scheduler/onlineddl_scheduler_test.go

@@ -335,7 +335,7 @@ func testScheduler(t *testing.T) {
 
 	mysqlVersion := onlineddl.GetMySQLVersion(t, clusterInstance.Keyspaces[0].Shards[0].PrimaryTablet())
 	require.NotEmpty(t, mysqlVersion)
-	_, capableOf, _ := mysql.GetFlavor(mysqlVersion, nil)
+	capableOf := mysql.ServerVersionCapableOf(mysqlVersion)
 
 	var (
 		t1uuid string

go/test/endtoend/tabletmanager/tablegc/tablegc_test.go

@@ -291,7 +291,7 @@ func TestCapability(t *testing.T) {
 	mysqlVersion := onlineddl.GetMySQLVersion(t, clusterInstance.Keyspaces[0].Shards[0].PrimaryTablet())
 	require.NotEmpty(t, mysqlVersion)
 
-	_, capableOf, _ := mysql.GetFlavor(mysqlVersion, nil)
+	capableOf := mysql.ServerVersionCapableOf(mysqlVersion)
 	require.NotNil(t, capableOf)
 	var err error
 	fastDropTable, err = capableOf(capabilities.FastDropTableFlavorCapability)

go/vt/mysqlctl/backup_blackbox_test.go

@@ -602,7 +602,7 @@ func needInnoDBRedoLogSubdir() (needIt bool, err error) {
 		return needIt, err
 	}
 	versionStr := fmt.Sprintf("%d.%d.%d", sv.Major, sv.Minor, sv.Patch)
-	_, capableOf, _ := mysql.GetFlavor(versionStr, nil)
+	capableOf := mysql.ServerVersionCapableOf(versionStr)
 	if capableOf == nil {
 		return needIt, fmt.Errorf("cannot determine database flavor details for version %s", versionStr)
 	}