Repository: pingcap/tidb-binlog
Pull request #836 (2019-11-29 12:45:10)

loader: don't refresh table info for some DDL (#613)
<!--
Thank you for contributing to TiDB! Please read TiDB's [CONTRIBUTING](https://github.com/pingcap/tidb/blob/master/CONTRIBUTING.md) document **BEFORE** filing this PR.
-->

### What problem does this PR solve? <!--add issue link with summary if exists-->
cherry-pick #613

### 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

Code changes

 - Has exported function/method change
 - Has exported variable/fields change
 - Has interface methods change
 - Has persistent data change

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
 - Need to update the `tidb-ansible` repository
 - Need to be included in the release note

suzaku: /run-all-tests

Patch:
diff --git a/pkg/loader/load.go b/pkg/loader/load.go
index c7f988ca..6e497f84 100644
--- a/pkg/loader/load.go
+++ b/pkg/loader/load.go
@@ -218,6 +218,27 @@ func (s *Loader) getTableInfo(schema string, table string) (info *tableInfo, err
 	return s.refreshTableInfo(schema, table)
 }
 
+func needRefreshTableInfo(sql string) bool {
+	stmt, err := parser.New().ParseOneStmt(sql, "", "")
+	if err != nil {
+		log.Errorf("parse sql failed, sql: %s, err: %v", sql, err)
+		return true
+	}
+
+	switch stmt.(type) {
+	case *ast.DropTableStmt:
+		return false
+	case *ast.DropDatabaseStmt:
+		return false
+	case *ast.TruncateTableStmt:
+		return false
+	case *ast.CreateDatabaseStmt:
+		return false
+	}
+
+	return true
+}
+
 func isCreateDatabaseDDL(sql string) bool {
 	stmt, err := parser.New().ParseOneStmt(sql, "", "")
 	if err != nil {
@@ -419,6 +440,9 @@ func (s *Loader) Run() error {
 		}
 
 		s.markSuccess(txn)
+		if !needRefreshTableInfo(txn.DDL.SQL) {
+			return nil
+		}
 		if _, err := s.refreshTableInfo(txn.DDL.Database, txn.DDL.Table); err != nil {
 			log.Errorf("refresh table info failed, database: %s, table: %s, error: %v", txn.DDL.Database, txn.DDL.Table, err)
 		}
diff --git a/reparo/syncer/translate.go b/reparo/syncer/translate.go
index d9ff56d7..4389346c 100644
--- a/reparo/syncer/translate.go
+++ b/reparo/syncer/translate.go
@@ -1,6 +1,8 @@
 package syncer
 
 import (
+	"strings"
+
 	"github.com/ngaut/log"
 	"github.com/pingcap/errors"
 	"github.com/pingcap/parser"
@@ -10,14 +12,25 @@ import (
 	"github.com/pingcap/tidb/util/codec"
 )
 
+func trimUse(sql string) string {
+	split := strings.Split(sql, ";")
+	if len(split) > 0 && strings.HasPrefix(split[0], "use ") {
+		return strings.Join(split[1:], ";")
+	}
+
+	return sql
+}
+
 func pbBinlogToTxn(binlog *pb.Binlog) (txn *loader.Txn, err error) {
 	txn = new(loader.Txn)
 	switch binlog.Tp {
 	case pb.BinlogType_DDL:
 		txn.DDL = new(loader.DDL)
-		// for table DDL, pb.Binlog.DdlQuery will be "use <db>; create..."
-		txn.DDL.SQL = string(binlog.DdlQuery)
-		txn.DDL.Database, txn.DDL.Table, err = parserSchemaTableFromDDL(txn.DDL.SQL)
+		// If it is not create database statement, pb.Binlog.DdlQuery will be "use <db>; create..."
+		// trim the "use..." to make it as expected by `loader.Txn`
+		// we should better just pass the database context into pb.Binlog
+		txn.DDL.SQL = trimUse(string(binlog.DdlQuery))
+		txn.DDL.Database, txn.DDL.Table, err = parserSchemaTableFromDDL(string(binlog.DdlQuery))
 		if err != nil {
 			return nil, errors.Trace(err)
 		}


Test patch:
diff --git a/pkg/loader/load_test.go b/pkg/loader/load_test.go
index d6f0564f..28397222 100644
--- a/pkg/loader/load_test.go
+++ b/pkg/loader/load_test.go
@@ -25,3 +25,21 @@ func (cs *LoadSuite) TestNewClose(c *check.C) {
 
 	loader.Close()
 }
+
+type needRefreshTableInfoSuite struct{}
+
+var _ = check.Suite(&needRefreshTableInfoSuite{})
+
+func (s *needRefreshTableInfoSuite) TestNeedRefreshTableInfo(c *check.C) {
+	cases := map[string]bool{
+		"DROP TABLE a":           false,
+		"DROP DATABASE a":        false,
+		"TRUNCATE TABLE a":       false,
+		"CREATE DATABASE a":      false,
+		"CREATE TABLE a(id int)": true,
+	}
+
+	for sql, res := range cases {
+		c.Assert(needRefreshTableInfo(sql), check.Equals, res)
+	}
+}
diff --git a/reparo/syncer/translate_test.go b/reparo/syncer/translate_test.go
index 6e53f18f..401e2288 100644
--- a/reparo/syncer/translate_test.go
+++ b/reparo/syncer/translate_test.go
@@ -20,10 +20,10 @@ func (s *testTranslateSuite) TestPBBinlogToTxn(c *check.C) {
 	tests := map[*pb.Binlog]*loader.Txn{
 		{
 			Tp:       pb.BinlogType_DDL,
-			DdlQuery: []byte("use db1; create table table1(id int)"),
+			DdlQuery: []byte("use db1;create table table1(id int)"),
 		}: {
 			DDL: &loader.DDL{
-				SQL:      "use db1; create table table1(id int)",
+				SQL:      "create table table1(id int)",
 				Database: "db1",
 				Table:    "table1",
 			},
@@ -74,6 +74,21 @@ func (s *testTranslateSuite) TestPBBinlogToTxn(c *check.C) {
 	}
 }
 
+func (s *testTranslateSuite) TestTrimUse(c *check.C) {
+	tests := []struct {
+		Origin string
+		Expect string
+	}{
+		{"use a;create table a(id int);", "create table a(id int);"},
+		{"create database a;", "create database a;"},
+	}
+
+	for _, test := range tests {
+		get := trimUse(test.Origin)
+		c.Assert(get, check.Equals, test.Expect)
+	}
+}
+
 func (s *testTranslateSuite) TestGenColsAndArgs(c *check.C) {
 	cols, args, err := genColsAndArgs(generateColumns(c))
 	c.Assert(err, check.IsNil)