playground: Only add --comments when needed (#2314)

components/playground/playground.go

@@ -22,6 +22,7 @@ import (
  "os"
  "os/exec"
  "path/filepath"
+ "regexp"
  "runtime"
  "strconv"
  "strings"
@@ -505,22 +506,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]))
  }
  }
 
@@ -1117,15 +1119,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])
  }
  }
 
@@ -1487,3 +1490,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)
+ }
+}