Fix capability checks

Signed-off-by: Dirkjan Bussink <[email protected]>

go/mysql/capabilities/capability.go

@@ -48,6 +48,8 @@ const (
 	PerformanceSchemaDataLocksTableCapability                           // supported in MySQL 8.0.1 and above: https://dev.mysql.com/doc/relnotes/mysql/8.0/en/news-8-0-1.html
 	InstantDDLXtrabackupCapability                                      // Supported in 8.0.32 and above, solving a MySQL-vs-Xtrabackup bug starting 8.0.29
 	ReplicaTerminologyCapability                                        // Supported in 8.0.26 and above, using SHOW REPLICA STATUS and all variations.
+	BinaryLogStatus                                                     // Supported in 8.2.0 and above, uses SHOW BINARY LOG STATUS
+	RestrictFKOnNonStandardKey                                          // Supported in 8.4.0 and above, restricts usage of non-standard indexes for foreign keys.
 )
 
 type CapableOf func(capability FlavorCapability) (bool, error)
@@ -119,6 +121,10 @@ func MySQLVersionHasCapability(serverVersion string, capability FlavorCapability
 		// So be conservative here, and only use the new syntax on newer versions,
 		// so we don't have to have too many different flavors.
 		return atLeast(8, 0, 26)
+	case BinaryLogStatus:
+		return atLeast(8, 2, 0)
+	case RestrictFKOnNonStandardKey:
+		return atLeast(8, 4, 0)
 	default:
 		return false, nil
 	}

go/mysql/flavor.go

@@ -158,7 +158,7 @@ type flavor interface {
 // flavorFuncs maps flavor names to their implementation.
 // Flavors need to register only if they support being specified in the
 // connection parameters.
-var flavorFuncs = make(map[string]func() flavor)
+var flavorFuncs = make(map[string]func(serverVersion string) flavor)
 
 // GetFlavor fills in c.Flavor. If the params specify the flavor,
 // that is used. Otherwise, we auto-detect.
@@ -172,11 +172,11 @@ var flavorFuncs = make(map[string]func() flavor)
 // Note on such servers, 'select version()' would return 10.0.21-MariaDB-...
 // as well (not matching what c.ServerVersion is, but matching after we remove
 // the prefix).
-func GetFlavor(serverVersion string, flavorFunc func() flavor) (f flavor, capableOf capabilities.CapableOf, canonicalVersion string) {
+func GetFlavor(serverVersion string, flavorFunc func(serverVersion string) flavor) (f flavor, capableOf capabilities.CapableOf, canonicalVersion string) {
 	canonicalVersion = serverVersion
 	switch {
 	case flavorFunc != nil:
-		f = flavorFunc()
+		f = flavorFunc(serverVersion)
 	case strings.HasPrefix(serverVersion, mariaDBReplicationHackPrefix):
 		canonicalVersion = serverVersion[len(mariaDBReplicationHackPrefix):]
 		f = mariadbFlavor101{mariadbFlavor{serverVersion: canonicalVersion}}
@@ -282,7 +282,7 @@ func (c *Conn) GetServerUUID() (string, error) {
 
 // PrimaryFilePosition returns the current primary's file based replication position.
 func (c *Conn) PrimaryFilePosition() (replication.Position, error) {
-	filePosFlavor := filePosFlavor{}
+	filePosFlavor := filePosFlavor{serverVersion: c.ServerVersion}
 	gtidSet, err := filePosFlavor.primaryGTIDSet(c)
 	if err != nil {
 		return replication.Position{}, err
@@ -440,7 +440,7 @@ func (c *Conn) CatchupToGTIDCommands(params *ConnParams, pos replication.Positio
 // the context expires for the file position flavor. It returns an error if
 // we did not succeed.
 func (c *Conn) WaitUntilFilePosition(ctx context.Context, pos replication.Position) error {
-	filePosFlavor := filePosFlavor{}
+	filePosFlavor := filePosFlavor{serverVersion: c.ServerVersion}
 	return filePosFlavor.waitUntilPosition(ctx, c, pos)
 }
 

go/mysql/flavor_filepos.go

@@ -32,21 +32,21 @@ import (
 )
 
 type filePosFlavor struct {
-	format     BinlogFormat
-	file       string
-	savedEvent BinlogEvent
+	format        BinlogFormat
+	file          string
+	savedEvent    BinlogEvent
+	serverVersion string
 }
 
 // newFilePosFlavor creates a new filePos flavor.
-func newFilePosFlavor() flavor {
-	return &filePosFlavor{}
+func newFilePosFlavor(serverVersion string) flavor {
+	return &filePosFlavor{serverVersion: serverVersion}
 }
 
 // primaryGTIDSet is part of the Flavor interface.
 func (flv *filePosFlavor) primaryGTIDSet(c *Conn) (replication.GTIDSet, error) {
 	query := "SHOW MASTER STATUS"
-	capability, _ := capabilities.ServerVersionAtLeast(c.ServerVersion, 8, 4, 0)
-	if capability {
+	if ok, err := c.SupportsCapability(capabilities.BinaryLogStatus); err == nil && ok {
 		query = "SHOW BINARY LOG STATUS"
 	}
 
@@ -300,11 +300,9 @@ func (flv *filePosFlavor) waitUntilPosition(ctx context.Context, c *Conn, pos re
 	queryPos := "SELECT MASTER_POS_WAIT('%s', %d)"
 	queryPosSub := "SELECT MASTER_POS_WAIT('%s', %d, %.6f)"
 
-	if capableOf := capabilities.MySQLVersionCapableOf(c.ServerVersion); capableOf != nil {
-		if ok, _ := capableOf(capabilities.ReplicaTerminologyCapability); ok {
-			queryPos = "SELECT SOURCE_POS_WAIT('%s', %d)"
-			queryPosSub = "SELECT SOURCE_POS_WAIT('%s', %d, %.6f)"
-		}
+	if ok, err := c.SupportsCapability(capabilities.ReplicaTerminologyCapability); err == nil && ok {
+		queryPos = "SELECT SOURCE_POS_WAIT('%s', %d)"
+		queryPosSub = "SELECT SOURCE_POS_WAIT('%s', %d, %.6f)"
 	}
 
 	query := fmt.Sprintf(queryPos, filePosPos.File, filePosPos.Pos)
@@ -369,8 +367,10 @@ func (*filePosFlavor) baseShowTablesWithSizes() string {
 }
 
 // supportsCapability is part of the Flavor interface.
-func (*filePosFlavor) supportsCapability(capability capabilities.FlavorCapability) (bool, error) {
+func (f *filePosFlavor) supportsCapability(capability capabilities.FlavorCapability) (bool, error) {
 	switch capability {
+	case capabilities.BinaryLogStatus:
+		return capabilities.ServerVersionAtLeast(f.serverVersion, 8, 2, 0)
 	default:
 		return false, nil
 	}

go/mysql/flavor_mysqlgr.go

@@ -42,8 +42,8 @@ type mysqlGRFlavor struct {
 }
 
 // newMysqlGRFlavor creates a new mysqlGR flavor.
-func newMysqlGRFlavor() flavor {
-	return &mysqlGRFlavor{}
+func newMysqlGRFlavor(serverVersion string) flavor {
+	return &mysqlGRFlavor{mysqlFlavor{serverVersion: serverVersion}}
 }
 
 // startReplicationCommand returns the command to start the replication.

go/vt/binlog/binlogplayer/dbclient.go

@@ -23,6 +23,7 @@ import (
 
 	"vitess.io/vitess/go/constants/sidecar"
 	"vitess.io/vitess/go/mysql"
+	"vitess.io/vitess/go/mysql/capabilities"
 	"vitess.io/vitess/go/mysql/sqlerror"
 	"vitess.io/vitess/go/sqltypes"
 	"vitess.io/vitess/go/textutil"
@@ -41,7 +42,7 @@ type DBClient interface {
 	Close()
 	ExecuteFetch(query string, maxrows int) (qr *sqltypes.Result, err error)
 	ExecuteFetchMulti(query string, maxrows int) (qrs []*sqltypes.Result, err error)
-	ServerVersion() string
+	SupportsCapability(capability capabilities.FlavorCapability) (bool, error)
 }
 
 // dbClientImpl is a real DBClient backed by a mysql connection.
@@ -124,8 +125,8 @@ func (dc *dbClientImpl) Close() {
 	dc.dbConn.Close()
 }
 
-func (dc *dbClientImpl) ServerVersion() string {
-	return dc.dbConn.ServerVersion
+func (dc *dbClientImpl) SupportsCapability(capability capabilities.FlavorCapability) (bool, error) {
+	return dc.dbConn.SupportsCapability(capability)
 }
 
 // LogError logs a message after truncating it to avoid spamming logs

go/vt/binlog/binlogplayer/fake_dbclient.go

@@ -20,7 +20,7 @@ import (
 	"fmt"
 	"strings"
 
-	"vitess.io/vitess/go/mysql/config"
+	"vitess.io/vitess/go/mysql/capabilities"
 	"vitess.io/vitess/go/sqltypes"
 )
 
@@ -86,6 +86,6 @@ func (dc *fakeDBClient) ExecuteFetchMulti(query string, maxrows int) ([]*sqltype
 	return make([]*sqltypes.Result, 0), nil
 }
 
-func (dc *fakeDBClient) ServerVersion() string {
-	return config.DefaultMySQLVersion
+func (dc *fakeDBClient) SupportsCapability(capability capabilities.FlavorCapability) (bool, error) {
+	return false, nil
 }

go/vt/binlog/binlogplayer/mock_dbclient.go

@@ -24,7 +24,7 @@ import (
 	"testing"
 	"time"
 
-	"vitess.io/vitess/go/mysql/config"
+	"vitess.io/vitess/go/mysql/capabilities"
 	"vitess.io/vitess/go/sqltypes"
 	"vitess.io/vitess/go/vt/sqlparser"
 )
@@ -262,6 +262,6 @@ func (dc *MockDBClient) RemoveInvariant(query string) {
 	delete(dc.invariants, query)
 }
 
-func (dc *MockDBClient) ServerVersion() string {
-	return config.DefaultMySQLVersion
+func (dc *MockDBClient) SupportsCapability(capability capabilities.FlavorCapability) (bool, error) {
+	return false, nil
 }

go/vt/vttablet/tabletmanager/vdiff/framework_test.go

@@ -30,6 +30,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"vitess.io/vitess/go/mysql"
+	"vitess.io/vitess/go/mysql/capabilities"
 	"vitess.io/vitess/go/sqltypes"
 	"vitess.io/vitess/go/vt/binlog/binlogplayer"
 	"vitess.io/vitess/go/vt/grpcclient"
@@ -422,8 +423,8 @@ func (dbc *realDBClient) ExecuteFetchMulti(query string, maxrows int) ([]*sqltyp
 	return results, nil
 }
 
-func (dbc *realDBClient) ServerVersion() string {
-	return dbc.conn.ServerVersion
+func (dbc *realDBClient) SupportsCapability(capability capabilities.FlavorCapability) (bool, error) {
+	return dbc.conn.SupportsCapability(capability)
 }
 
 //----------------------------------------------

go/vt/vttablet/tabletmanager/vreplication/framework_test.go

@@ -34,6 +34,7 @@ import (
 
 	_flag "vitess.io/vitess/go/internal/flag"
 	"vitess.io/vitess/go/mysql"
+	"vitess.io/vitess/go/mysql/capabilities"
 	"vitess.io/vitess/go/mysql/replication"
 	"vitess.io/vitess/go/sqltypes"
 	"vitess.io/vitess/go/test/utils"
@@ -509,8 +510,8 @@ func (dbc *realDBClient) ExecuteFetchMulti(query string, maxrows int) ([]*sqltyp
 	return results, nil
 }
 
-func (dbc *realDBClient) ServerVersion() string {
-	return dbc.conn.ServerVersion
+func (dbc *realDBClient) SupportsCapability(capability capabilities.FlavorCapability) (bool, error) {
+	return dbc.conn.SupportsCapability(capability)
 }
 
 func expectDeleteQueries(t *testing.T) {

go/vt/vttablet/tabletmanager/vreplication/vreplicator.go

@@ -527,7 +527,10 @@ func (vr *vreplicator) getSettingFKCheck() error {
 }
 
 func (vr *vreplicator) needFKRestrict() bool {
-	ok, _ := capabilities.ServerVersionAtLeast(vr.dbClient.ServerVersion(), 8, 4, 0)
+	ok, err := vr.dbClient.SupportsCapability(capabilities.RestrictFKOnNonStandardKey)
+	if err != nil {
+		return false
+	}
 	return ok
 }
 

go/vt/vttablet/tabletmanager/vreplication/vreplicator_test.go

@@ -31,7 +31,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"vitess.io/vitess/go/mysql/capabilities"
 	"vitess.io/vitess/go/vt/binlog/binlogplayer"
 	"vitess.io/vitess/go/vt/dbconfigs"
 	"vitess.io/vitess/go/vt/mysqlctl"
@@ -482,9 +481,7 @@ func TestDeferSecondaryKeys(t *testing.T) {
 			}
 
 			// Create the table.
-			capability, err := capabilities.ServerVersionAtLeast(dbClient.ServerVersion(), 8, 4, 0)
-			require.NoError(t, err)
-			if capability {
+			if vr.needFKRestrict() {
 				_, err := dbClient.ExecuteFetch("set @@session.restrict_fk_on_non_standard_key=0", 1)
 				require.NoError(t, err)
 				defer func() {

go/vt/wrangler/fake_dbclient_test.go

@@ -25,11 +25,10 @@ import (
 
 	"github.com/stretchr/testify/assert"
 
-	"vitess.io/vitess/go/mysql/config"
+	"vitess.io/vitess/go/mysql/capabilities"
+	"vitess.io/vitess/go/sqltypes"
 	"vitess.io/vitess/go/vt/log"
 	"vitess.io/vitess/go/vt/sqlparser"
-
-	"vitess.io/vitess/go/sqltypes"
 )
 
 func verifyQueries(t *testing.T, dcs []*fakeDBClient) {
@@ -178,8 +177,8 @@ func (dc *fakeDBClient) ExecuteFetchMulti(query string, maxrows int) ([]*sqltype
 	return results, nil
 }
 
-func (dc *fakeDBClient) ServerVersion() string {
-	return config.DefaultMySQLVersion
+func (dc *fakeDBClient) SupportsCapability(capability capabilities.FlavorCapability) (bool, error) {
+	return false, nil
 }
 
 // ExecuteFetch is part of the DBClient interface