Repository: pingcap/dumpling
Pull request #381 (2021-11-05 07:40:19)

make version check more compatible
<!--
Thank you for contributing to Dumpling! Please read the [CONTRIBUTING](https://github.com/pingcap/dumpling/blob/master/CONTRIBUTING.md) document **BEFORE** filing this PR.

If you want to open the **Challenge Program** pull request, please use the following template:
https://raw.githubusercontent.com/pingcap/.github/master/.github/PULL_REQUEST_TEMPLATE/challenge-program.md

You can use it with query parameters: https://github.com/pingcap/dumpling/compare/master...${you branch}?template=challenge-program.md
-->

### What problem does this PR solve? <!--add issue link with summary if exists-->
cherry-pick pingcap/tidb#29500 to release-5.2

### What is changed and how it works?


### Check List <!--REMOVE the items that are not applicable-->

Tests <!-- At least one of them must be included. -->

 - Unit test
 - Integration test
 - Manual test (add detailed scripts or steps below)
 - No code

Side effects

 - Possible performance regression
 - Increased code complexity
 - Breaking backward compatibility

Related changes

 - Need to cherry-pick to the release branch
 - Need to update the documentation
 
### Release note

<!-- bugfixes or new feature need a release note, must in the form of a list, such as

- support -T/--tables-list argument

or if no need to be included in the release note, just add the following line

- No release note
-->


ti-chi-bot: [REVIEW NOTIFICATION]

This pull request has not been approved.


To complete the [pull request process](https://book.prow.tidb.io/#/en/workflows/pr), please ask the reviewers in the [list](https://prow.tidb.io/tichi/repos/pingcap/dumpling/pulls/381/owners) to review by filling `/cc @reviewer` in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the [list](https://prow.tidb.io/tichi/repos/pingcap/dumpling/pulls/381/owners) by filling  `/assign @committer` in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found [here](https://prow.tidb.io/command-help?repo=pingcap%2Fdumpling).

<details>

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.
</details>

<!--Review Notification Identifier-->
ti-chi-bot: @glorv: This cherry pick PR is for a release branch and has not yet been approved by release team.
Adding the `do-not-merge/cherry-pick-not-approved` label.

To merge this cherry pick, it must first be approved by the collaborators.

**AFTER** it has been approved by collaborators, please ping the release team in a comment to request a cherry pick review.


<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
lichunzhu: /merge
ti-chi-bot: This pull request has been accepted and is ready to merge. <details>Commit hash: 68f3bca97f45eaca36df8673c34be931353bba5b</details>
ti-chi-bot: Merge canceled because a new commit is pushed.
glorv: /merge
ti-chi-bot: This pull request has been accepted and is ready to merge. <details>Commit hash: 47a8ae69cb2a9f18287d34989af18296211fdaab</details>
ti-chi-bot: Merge canceled because a new commit is pushed.
glorv: /merge
ti-chi-bot: This pull request has been accepted and is ready to merge. <details>Commit hash: 0feb50a7d3b617eb9e99f76a5fd8779767b4b191</details>
ti-chi-bot: Merge canceled because a new commit is pushed.
glorv: /merge
ti-chi-bot: This pull request has been accepted and is ready to merge. <details>Commit hash: 518a227129bfa2ecf70fc85e3da29edbfa506726</details>
ti-chi-bot: Merge canceled because a new commit is pushed.
glorv: /merge
ti-chi-bot: This pull request has been accepted and is ready to merge. <details>Commit hash: 0e457cb4797f551eafb82e5745d031847289fb5a</details>

Patch:
diff --git a/v4/export/config.go b/v4/export/config.go
index 4458df2..3a28458 100644
--- a/v4/export/config.go
+++ b/v4/export/config.go
@@ -586,16 +586,23 @@ var ServerInfoUnknown = ServerInfo{
 }
 
 var (
-	versionRegex     = regexp.MustCompile(`^\d+\.\d+\.\d+([0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?`)
+	versionRegex = regexp.MustCompile(`^\d+\.\d+\.\d+([0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?`)
+	// `select version()` result
 	tidbVersionRegex = regexp.MustCompile(`-[v]?\d+\.\d+\.\d+([0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?`)
+	// `select tidb_version()` result
+	tidbReleaseVersionRegex = regexp.MustCompile(`v\d+\.\d+\.\d+([0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?`)
 )
 
 // ParseServerInfo parses exported server type and version info from version string
 func ParseServerInfo(tctx *tcontext.Context, src string) ServerInfo {
-	tctx.L().Debug("parse server info", zap.String("server info string", src))
 	lowerCase := strings.ToLower(src)
 	serverInfo := ServerInfo{}
+	isReleaseVersion := false
 	switch {
+	case strings.Contains(lowerCase, "release version:"):
+		// this version string is tidb release version
+		serverInfo.ServerType = ServerTypeTiDB
+		isReleaseVersion = true
 	case strings.Contains(lowerCase, "tidb"):
 		serverInfo.ServerType = ServerTypeTiDB
 	case strings.Contains(lowerCase, "mariadb"):
@@ -606,12 +613,13 @@ func ParseServerInfo(tctx *tcontext.Context, src string) ServerInfo {
 		serverInfo.ServerType = ServerTypeUnknown
 	}
 
-	tctx.L().Info("detect server type",
-		zap.String("type", serverInfo.ServerType.String()))
-
 	var versionStr string
 	if serverInfo.ServerType == ServerTypeTiDB {
-		versionStr = tidbVersionRegex.FindString(src)[1:]
+		if isReleaseVersion {
+			versionStr = tidbReleaseVersionRegex.FindString(src)
+		} else {
+			versionStr = tidbVersionRegex.FindString(src)[1:]
+		}
 		versionStr = strings.TrimPrefix(versionStr, "v")
 	} else {
 		versionStr = versionRegex.FindString(src)
@@ -622,11 +630,15 @@ func ParseServerInfo(tctx *tcontext.Context, src string) ServerInfo {
 	if err != nil {
 		tctx.L().Warn("fail to parse version",
 			zap.String("version", versionStr))
-		return serverInfo
 	}
-
+	var version string
+	if serverInfo.ServerVersion != nil {
+		version = serverInfo.ServerVersion.String()
+	}
 	tctx.L().Info("detect server version",
-		zap.String("version", serverInfo.ServerVersion.String()))
+		zap.String("type", serverInfo.ServerType.String()),
+		zap.String("version", version))
+
 	return serverInfo
 }
 
diff --git a/v4/export/dump.go b/v4/export/dump.go
index b2d51a7..9750dc6 100755
--- a/v4/export/dump.go
+++ b/v4/export/dump.go
@@ -1045,7 +1045,7 @@ func openSQLDB(d *Dumper) error {
 // detectServerInfo is an initialization step of Dumper.
 func detectServerInfo(d *Dumper) error {
 	db, conf := d.dbHandle, d.conf
-	versionStr, err := SelectVersion(db)
+	versionStr, err := SelectVersion(d.tctx, db)
 	if err != nil {
 		conf.ServerInfo = ServerInfoUnknown
 		return err
diff --git a/v4/export/sql.go b/v4/export/sql.go
index 0229501..1613a32 100644
--- a/v4/export/sql.go
+++ b/v4/export/sql.go
@@ -279,11 +279,18 @@ func ListAllDatabasesTables(tctx *tcontext.Context, db *sql.Conn, databaseNames
 }
 
 // SelectVersion gets the version information from the database server
-func SelectVersion(db *sql.DB) (string, error) {
+func SelectVersion(ctx *tcontext.Context, db *sql.DB) (string, error) {
 	var versionInfo string
-	const query = "SELECT version()"
+	const queryTiDB = "SELECT tidb_version();"
+	tidbRow := db.QueryRowContext(ctx, queryTiDB)
+	err := tidbRow.Scan(&versionInfo)
+	if err == nil {
+		return versionInfo, nil
+	}
+	ctx.L().Warn("select tidb_version() failed, will fallback to 'select version();'", zap.Error(err))
+	const query = "SELECT version();"
 	row := db.QueryRow(query)
-	err := row.Scan(&versionInfo)
+	err = row.Scan(&versionInfo)
 	if err != nil {
 		return "", errors.Annotatef(err, "sql: %s", query)
 	}


Test patch:
diff --git a/v4/export/sql_test.go b/v4/export/sql_test.go
index ba9ac58..179dfee 100644
--- a/v4/export/sql_test.go
+++ b/v4/export/sql_test.go
@@ -62,6 +62,8 @@ func TestDetectServerInfo(t *testing.T) {
 		{4, "5.7.25-TiDB-v3.0.7-58-g6adce2367", ServerTypeTiDB, mkVer(3, 0, 7, "58-g6adce2367")},
 		{5, "5.7.25-TiDB-3.0.6", ServerTypeTiDB, mkVer(3, 0, 6, "")},
 		{6, "invalid version", ServerTypeUnknown, (*semver.Version)(nil)},
+		{7, "Release Version: v5.2.1\nEdition: Community\nGit Commit Hash: cd8fb24c5f7ebd9d479ed228bb41848bd5e97445", ServerTypeTiDB, mkVer(5, 2, 1, "")},
+		{8, "Release Version: v5.4.0-alpha-21-g86caab907\nEdition: Community\nGit Commit Hash: 86caab907c481bbc4243b5a3346ec13907cc8721\nGit Branch: master", ServerTypeTiDB, mkVer(5, 4, 0, "alpha-21-g86caab907")},
 	}
 	dec := func(d []interface{}) (tag int, verStr string, tp ServerType, v *semver.Version) {
 		return d[0].(int), d[1].(string), ServerType(d[2].(int)), d[3].(*semver.Version)
@@ -70,10 +72,16 @@ func TestDetectServerInfo(t *testing.T) {
 	for _, datum := range data {
 		tag, r, serverTp, expectVer := dec(datum)
 		comment := fmt.Sprintf("test case number: %d", tag)
-		rows := sqlmock.NewRows([]string{"version"}).AddRow(r)
-		mock.ExpectQuery("SELECT version()").WillReturnRows(rows)
+		tidbVersionQuery := mock.ExpectQuery("SELECT tidb_version\\(\\);")
+		if strings.HasPrefix(r, "Release Version:") {
+			tidbVersionQuery.WillReturnRows(sqlmock.NewRows([]string{"tidb_version"}).AddRow(r))
+		} else {
+			tidbVersionQuery.WillReturnError(errors.New("mock error"))
+			rows := sqlmock.NewRows([]string{"version"}).AddRow(r)
+			mock.ExpectQuery("SELECT version\\(\\);").WillReturnRows(rows)
+		}
 
-		verStr, err := SelectVersion(db)
+		verStr, err := SelectVersion(tcontext.Background(), db)
 		require.NoError(t, err, comment)
 
 		info := ParseServerInfo(tcontext.Background(), verStr)