Fix bug in parsing of version comments

go/test/endtoend/vtgate/mysql80/main_test.go

@@ -55,7 +55,10 @@ func TestMain(m *testing.M) {
 			return 1
 		}
 
-		clusterInstance.VtGateExtraArgs = append(clusterInstance.VtGateExtraArgs, "-enable_system_settings=true")
+		clusterInstance.VtGateExtraArgs = append(clusterInstance.VtGateExtraArgs,
+			"-enable_system_settings=true",
+			"-mysql_server_version=8.0.16-7",
+		)
 		// Start vtgate
 		err = clusterInstance.StartVtgate()
 		if err != nil {

go/test/endtoend/vtgate/mysql80/misc_test.go

@@ -93,6 +93,13 @@ func TestCheckConstraint(t *testing.T) {
 	exec(t, conn, cleanup)
 }
 
+func TestVersionCommentWorks(t *testing.T) {
+	conn, err := mysql.Connect(context.Background(), &vtParams)
+	require.NoError(t, err)
+	defer conn.Close()
+	exec(t, conn, "/*!80000 SET SESSION information_schema_stats_expiry=0 */")
+}
+
 func assertMatches(t *testing.T, conn *mysql.Conn, query, expected string) {
 	t.Helper()
 	qr := exec(t, conn, query)

go/vt/servenv/buildinfo.go

@@ -23,22 +23,11 @@ import (
 	"strconv"
 	"time"
 
-	"vitess.io/vitess/go/vt/proto/vtrpc"
-	"vitess.io/vitess/go/vt/vterrors"
-
-	"vitess.io/vitess/go/vt/log"
-
-	"vitess.io/vitess/go/vt/sqlparser"
-
 	"vitess.io/vitess/go/stats"
+	"vitess.io/vitess/go/vt/sqlparser"
 )
 
 var (
-	// MySQLServerVersion is what Vitess will present as it's version during the connection handshake,
-	// and as the value to the @@version system variable. If nothing is provided, Vitess will report itself as
-	// a specific MySQL version with the vitess version appended to it
-	MySQLServerVersion = flag.String("mysql_server_version", "", "MySQL server version to advertise.")
-
 	buildHost             = ""
 	buildUser             = ""
 	buildTime             = ""
@@ -81,8 +70,8 @@ func (v *versionInfo) String() string {
 }
 
 func (v *versionInfo) MySQLVersion() string {
-	if *MySQLServerVersion != "" {
-		return *MySQLServerVersion
+	if *sqlparser.MySQLServerVersion != "" {
+		return *sqlparser.MySQLServerVersion
 	}
 	return "5.7.9-vitess-" + v.version
 }
@@ -111,13 +100,6 @@ func init() {
 		goArch:             runtime.GOARCH,
 		version:            versionName,
 	}
-	var convVersion string
-	convVersion, err = convertMySQLVersionToCommentVersion(AppVersion.MySQLVersion())
-	if err != nil {
-		log.Error(err)
-	} else {
-		sqlparser.MySQLVersion = convVersion
-	}
 	stats.NewString("BuildHost").Set(AppVersion.buildHost)
 	stats.NewString("BuildUser").Set(AppVersion.buildUser)
 	stats.NewGauge("BuildTimestamp", "build timestamp").Set(AppVersion.buildTime)
@@ -139,41 +121,3 @@ func init() {
 	}
 	stats.NewGaugesWithMultiLabels("BuildInformation", "build information exposed via label", buildLabels).Set(buildValues, 1)
 }
-
-// convertMySQLVersionToCommentVersion converts the MySQL version into comment version format.
-func convertMySQLVersionToCommentVersion(version string) (string, error) {
-	var res = make([]int, 3)
-	idx := 0
-	val := ""
-	for _, c := range version {
-		if c <= '9' && c >= '0' {
-			val += string(c)
-		} else if c == '.' {
-			v, err := strconv.Atoi(val)
-			if err != nil {
-				return "", err
-			}
-			val = ""
-			res[idx] = v
-			idx++
-			if idx == 3 {
-				break
-			}
-		} else {
-			break
-		}
-	}
-	if val != "" {
-		v, err := strconv.Atoi(val)
-		if err != nil {
-			return "", err
-		}
-		res[idx] = v
-		idx++
-	}
-	if idx == 0 {
-		return "", vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "MySQL version not correctly setup - %s.", version)
-	}
-
-	return fmt.Sprintf("%01d%02d%02d", res[0], res[1], res[2]), nil
-}

go/vt/servenv/buildinfo_test.go

@@ -20,8 +20,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/require"
-
 	"github.com/stretchr/testify/assert"
 )
 
@@ -48,45 +46,4 @@ func TestVersionString(t *testing.T) {
 	assert.Equal(t, "Version: v1.2.3-SNAPSHOT (Jenkins build 422) (Git revision d54b87c branch 'gitBranch') built on time is now by user@host using 1.17 amiga/amd64", v.String())
 
 	assert.Equal(t, "5.7.9-vitess-v1.2.3-SNAPSHOT", v.MySQLVersion())
-	newVersion := "test!"
-	MySQLServerVersion = &newVersion
-	assert.Equal(t, newVersion, v.MySQLVersion())
-}
-
-func TestConvertMySQLVersion(t *testing.T) {
-	testcases := []struct {
-		version        string
-		commentVersion string
-		error          string
-	}{{
-		version:        "5.7.9",
-		commentVersion: "50709",
-	}, {
-		version:        "0008.08.9",
-		commentVersion: "80809",
-	}, {
-		version:        "5.7.9, Vitess - 10.0.1",
-		commentVersion: "50709",
-	}, {
-		version:        "8.1 Vitess - 10.0.1",
-		commentVersion: "80100",
-	}, {
-		version: "Vitess - 10.0.1",
-		error:   "MySQL version not correctly setup - Vitess - 10.0.1.",
-	}, {
-		version:        "5.7.9.22",
-		commentVersion: "50709",
-	}}
-
-	for _, tcase := range testcases {
-		t.Run(tcase.version, func(t *testing.T) {
-			output, err := convertMySQLVersionToCommentVersion(tcase.version)
-			if tcase.error != "" {
-				require.EqualError(t, err, tcase.error)
-			} else {
-				require.NoError(t, err)
-				require.Equal(t, tcase.commentVersion, output)
-			}
-		})
-	}
 }

go/vt/sqlparser/parser.go

@@ -17,8 +17,10 @@ limitations under the License.
 package sqlparser
 
 import (
+	"flag"
 	"fmt"
 	"io"
+	"strconv"
 	"strings"
 	"sync"
 
@@ -28,6 +30,12 @@ import (
 	vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
 )
 
+// MySQLServerVersion is what Vitess will present as it's version during the connection handshake,
+// and as the value to the @@version system variable. If nothing is provided, Vitess will report itself as
+// a specific MySQL version with the vitess version appended to it
+var MySQLServerVersion = flag.String("mysql_server_version", "", "MySQL server version to advertise.")
+var versionFlagSync sync.Once
+
 // parserPool is a pool for parser objects.
 var parserPool = sync.Pool{
 	New: func() interface{} {
@@ -39,7 +47,7 @@ var parserPool = sync.Pool{
 var zeroParser yyParserImpl
 
 // MySQLVersion is the version of MySQL that the parser would emulate
-var MySQLVersion string = "50709"
+var MySQLVersion string
 
 // yyParsePooled is a wrapper around yyParse that pools the parser objects. There isn't a
 // particularly good reason to use yyParse directly, since it immediately discards its parser.
@@ -99,6 +107,58 @@ func Parse2(sql string) (Statement, BindVars, error) {
 	return tokenizer.ParseTree, tokenizer.BindVars, nil
 }
 
+func checkParserVersionFlag() {
+	if flag.Parsed() {
+		versionFlagSync.Do(func() {
+			convVersion, err := convertMySQLVersionToCommentVersion(*MySQLServerVersion)
+			if err != nil {
+				log.Error(err)
+				MySQLVersion = "50709" // default version if nothing else is stated
+			} else {
+				MySQLVersion = convVersion
+			}
+		})
+	}
+}
+
+// convertMySQLVersionToCommentVersion converts the MySQL version into comment version format.
+func convertMySQLVersionToCommentVersion(version string) (string, error) {
+	var res = make([]int, 3)
+	idx := 0
+	val := ""
+	for _, c := range version {
+		if c <= '9' && c >= '0' {
+			val += string(c)
+		} else if c == '.' {
+			v, err := strconv.Atoi(val)
+			if err != nil {
+				return "", err
+			}
+			val = ""
+			res[idx] = v
+			idx++
+			if idx == 3 {
+				break
+			}
+		} else {
+			break
+		}
+	}
+	if val != "" {
+		v, err := strconv.Atoi(val)
+		if err != nil {
+			return "", err
+		}
+		res[idx] = v
+		idx++
+	}
+	if idx == 0 {
+		return "", vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "MySQL version not correctly setup - %s.", version)
+	}
+
+	return fmt.Sprintf("%01d%02d%02d", res[0], res[1], res[2]), nil
+}
+
 // Parse behaves like Parse2 but does not return a set of bind variables
 func Parse(sql string) (Statement, error) {
 	stmt, _, err := Parse2(sql)

go/vt/sqlparser/token.go

@@ -52,6 +52,8 @@ type Tokenizer struct {
 // NewStringTokenizer creates a new Tokenizer for the
 // sql string.
 func NewStringTokenizer(sql string) *Tokenizer {
+	checkParserVersionFlag()
+
 	return &Tokenizer{
 		buf:      sql,
 		BindVars: make(map[string]struct{}),

go/vt/sqlparser/version_test.go

@@ -0,0 +1,61 @@
+/*
+Copyright 2021 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 sqlparser
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestConvertMySQLVersion(t *testing.T) {
+	testcases := []struct {
+		version        string
+		commentVersion string
+		error          string
+	}{{
+		version:        "5.7.9",
+		commentVersion: "50709",
+	}, {
+		version:        "0008.08.9",
+		commentVersion: "80809",
+	}, {
+		version:        "5.7.9, Vitess - 10.0.1",
+		commentVersion: "50709",
+	}, {
+		version:        "8.1 Vitess - 10.0.1",
+		commentVersion: "80100",
+	}, {
+		version: "Vitess - 10.0.1",
+		error:   "MySQL version not correctly setup - Vitess - 10.0.1.",
+	}, {
+		version:        "5.7.9.22",
+		commentVersion: "50709",
+	}}
+
+	for _, tcase := range testcases {
+		t.Run(tcase.version, func(t *testing.T) {
+			output, err := convertMySQLVersionToCommentVersion(tcase.version)
+			if tcase.error != "" {
+				require.EqualError(t, err, tcase.error)
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, tcase.commentVersion, output)
+			}
+		})
+	}
+}

go/vt/vtgate/plugin_mysql_server.go

@@ -428,8 +428,8 @@ func initMySQLProtocol() {
 		if err != nil {
 			log.Exitf("mysql.NewListener failed: %v", err)
 		}
-		if *servenv.MySQLServerVersion != "" {
-			mysqlListener.ServerVersion = *servenv.MySQLServerVersion
+		if *sqlparser.MySQLServerVersion != "" {
+			mysqlListener.ServerVersion = *sqlparser.MySQLServerVersion
 		}
 		if *mysqlSslCert != "" && *mysqlSslKey != "" {
 			tlsVersion, err := vttls.TLSVersionToNumber(*mysqlTLSMinVersion)

go/vt/vttest/vtprocess.go

@@ -27,6 +27,8 @@ import (
 	"syscall"
 	"time"
 
+	"vitess.io/vitess/go/vt/sqlparser"
+
 	"google.golang.org/protobuf/encoding/prototext"
 
 	"vitess.io/vitess/go/vt/log"
@@ -253,8 +255,8 @@ func VtcomboProcess(env Environment, args *Config, mysql MySQLManager) *VtProces
 	if args.VSchemaDDLAuthorizedUsers != "" {
 		vt.ExtraArgs = append(vt.ExtraArgs, []string{"-vschema_ddl_authorized_users", args.VSchemaDDLAuthorizedUsers}...)
 	}
-	if *servenv.MySQLServerVersion != "" {
-		vt.ExtraArgs = append(vt.ExtraArgs, "-mysql_server_version", *servenv.MySQLServerVersion)
+	if *sqlparser.MySQLServerVersion != "" {
+		vt.ExtraArgs = append(vt.ExtraArgs, "-mysql_server_version", *sqlparser.MySQLServerVersion)
 	}
 
 	if socket != "" {