playground: Only add --comments when needed

components/playground/playground.go

@@ -22,6 +22,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"runtime"
 	"strconv"
 	"strings"
@@ -493,22 +494,23 @@ func (p *Playground) handleScaleOut(w io.Writer, cmd *Command) error {
 		return err
 	}
 
+	mysql := mysqlCommand()
 	if cmd.ComponentID == "tidb" {
 		addr := p.tidbs[len(p.tidbs)-1].Addr()
 		if checkDB(addr, cmd.UpTimeout) {
 			ss := strings.Split(addr, ":")
-			connectMsg := "To connect new added TiDB: mysql --comments --host %s --port %s -u root -p (no password)"
-			fmt.Println(color.GreenString(connectMsg, ss[0], ss[1]))
-			fmt.Fprintln(w, color.GreenString(connectMsg, ss[0], ss[1]))
+			connectMsg := "To connect new added TiDB: %s --host %s --port %s -u root -p (no password)"
+			fmt.Println(color.GreenString(connectMsg, mysql, ss[0], ss[1]))
+			fmt.Fprintln(w, color.GreenString(connectMsg, mysql, ss[0], ss[1]))
 		}
 	}
 	if cmd.ComponentID == "tiproxy" {
 		addr := p.tiproxys[len(p.tidbs)-1].Addr()
 		if checkDB(addr, cmd.UpTimeout) {
 			ss := strings.Split(addr, ":")
-			connectMsg := "To connect to the newly added TiProxy: mysql --comments --host %s --port %s -u root -p (no password)"
-			fmt.Println(color.GreenString(connectMsg, ss[0], ss[1]))
-			fmt.Fprintln(w, color.GreenString(connectMsg, ss[0], ss[1]))
+			connectMsg := "To connect to the newly added TiProxy: %s --host %s --port %s -u root -p (no password)"
+			fmt.Println(color.GreenString(connectMsg, mysql, ss[0], ss[1]))
+			fmt.Fprintln(w, color.GreenString(connectMsg, mysql, ss[0], ss[1]))
 		}
 	}
 
@@ -1092,15 +1094,16 @@ func (p *Playground) bootCluster(ctx context.Context, env *environment.Environme
 		fmt.Println()
 		color.New(color.FgGreen, color.Bold).Println("🎉 TiDB Playground Cluster is started, enjoy!")
 		fmt.Println()
+		mysql := mysqlCommand()
 		for _, dbAddr := range tidbSucc {
 			ss := strings.Split(dbAddr, ":")
 			fmt.Printf("Connect TiDB:    ")
-			colorCmd.Printf("mysql --comments --host %s --port %s -u root\n", ss[0], ss[1])
+			colorCmd.Printf("%s --host %s --port %s -u root\n", mysql, ss[0], ss[1])
 		}
 		for _, dbAddr := range tiproxySucc {
 			ss := strings.Split(dbAddr, ":")
 			fmt.Printf("Connect TiProxy: ")
-			colorCmd.Printf("mysql --comments --host %s --port %s -u root\n", ss[0], ss[1])
+			colorCmd.Printf("%s --host %s --port %s -u root\n", mysql, ss[0], ss[1])
 		}
 	}
 
@@ -1473,3 +1476,61 @@ func logIfErr(err error) {
 		fmt.Println(err)
 	}
 }
+
+// Check the MySQL Client version
+//
+// Since v8.1.0 `--comments` is the default, so we don't need to specify it.
+// Without `--comments` the MySQL client strips TiDB specific comments
+// like `/*T![clustered_index] CLUSTERED */`
+//
+// This returns `mysql --comments` for older versions or in case we failed to check
+// the version for any reason as `mysql --comments` is the safe option.
+// For newer MySQL versions it returns just `mysql`.
+//
+// For MariaDB versions of the MySQL Client it is expected to return `mysql --comments`.
+func mysqlCommand() (cmd string) {
+	cmd = "mysql --comments"
+	mysqlVerOutput, err := exec.Command("mysql", "--version").Output()
+	if err != nil {
+		return
+	}
+	vMaj, vMin, _, err := parseMysqlVersion(string(mysqlVerOutput))
+	if err == nil {
+		if vMaj == 8 && vMin >= 1 { // 8.1.0 and newer
+			return "mysql"
+		}
+	}
+	return
+}
+
+// parseMysqlVersion parses the output from `mysql --version` that is in `versionOutput`
+// and returns the major, minor and patch version.
+//
+// New format example: `mysql  Ver 8.2.0 for Linux on x86_64 (MySQL Community Server - GPL)`
+// Old format example: `mysql  Ver 14.14 Distrib 5.7.36, for linux-glibc2.12 (x86_64) using  EditLine wrapper`
+// MariaDB 11.2 format: `/usr/bin/mysql from 11.2.2-MariaDB, client 15.2 for linux-systemd (x86_64) using readline 5.1`
+//
+// Note that MariaDB has `bin/mysql` (deprecated) and `bin/mariadb`. This is to parse the version from `bin/mysql`.
+// As TiDB is a MySQL compatible database we recommend `bin/mysql` from MySQL.
+// If we ever want to auto-detect other clients like `bin/mariadb`, `bin/mysqlsh`, `bin/mycli`, etc then
+// each of them needs their own version detection and adjust for the right commandline options.
+func parseMysqlVersion(versionOutput string) (vMaj int, vMin int, vPatch int, err error) {
+	mysqlVerRegexp := regexp.MustCompile(`(Ver|Distrib|from) ([0-9]+)\.([0-9]+)\.([0-9]+)`)
+	mysqlVerMatch := mysqlVerRegexp.FindStringSubmatch(versionOutput)
+	if mysqlVerMatch == nil {
+		return 0, 0, 0, errors.New("No match")
+	}
+	vMaj, err = strconv.Atoi(mysqlVerMatch[2])
+	if err != nil {
+		return 0, 0, 0, err
+	}
+	vMin, err = strconv.Atoi(mysqlVerMatch[3])
+	if err != nil {
+		return 0, 0, 0, err
+	}
+	vPatch, err = strconv.Atoi(mysqlVerMatch[4])
+	if err != nil {
+		return 0, 0, 0, err
+	}
+	return
+}

components/playground/playground_test.go

@@ -40,3 +40,75 @@ func TestPlaygroundAbsDir(t *testing.T) {
 	assert.Nil(t, err)
 	assert.Equal(t, filepath.Join(u.HomeDir, "c/d/e"), c)
 }
+
+func TestParseMysqlCommand(t *testing.T) {
+	cases := []struct {
+		version string
+		vMaj    int
+		vMin    int
+		vPatch  int
+		err     bool
+	}{
+		{
+			"mysql  Ver 8.2.0 for Linux on x86_64 (MySQL Community Server - GPL)",
+			8,
+			2,
+			0,
+			false,
+		},
+		{
+			"mysql  Ver 8.0.34 for Linux on x86_64 (MySQL Community Server - GPL)",
+			8,
+			0,
+			34,
+			false,
+		},
+		{
+			"mysql  Ver 8.0.34-foobar for Linux on x86_64 (MySQL Community Server - GPL)",
+			8,
+			0,
+			34,
+			false,
+		},
+		{
+			"foobar",
+			0,
+			0,
+			0,
+			true,
+		},
+		{
+			"mysql  Ver 14.14 Distrib 5.7.36, for linux-glibc2.12 (x86_64) using  EditLine wrapper",
+			5,
+			7,
+			36,
+			false,
+		},
+		{
+			"mysql  Ver 15.1 Distrib 10.3.37-MariaDB, for Linux (x86_64) using readline 5.1",
+			10,
+			3,
+			37,
+			false,
+		},
+		{
+			"/bin/mysql from 11.2.2-MariaDB, client 15.2 for linux-systemd (x86_64) using readline 5.1",
+			11,
+			2,
+			2,
+			false,
+		},
+	}
+
+	for _, tc := range cases {
+		vMaj, vMin, vPatch, err := parseMysqlVersion(tc.version)
+		if tc.err {
+			assert.NotNil(t, err)
+		} else {
+			assert.Nil(t, err)
+		}
+		assert.Equal(t, vMaj, tc.vMaj)
+		assert.Equal(t, vMin, tc.vMin)
+		assert.Equal(t, vPatch, tc.vPatch)
+	}
+}