From 089c3b24cef92278dce89318d167e2b030ff18a3 Mon Sep 17 00:00:00 2001
From: okJiang <819421878@qq.com>
Date: Thu, 24 Feb 2022 12:33:42 +0800
Subject: [PATCH] checker(dm): add auto increment id check in optimistic
 (#4636)

ref pingcap/tiflow#3608
---
 dm/pkg/checker/table_structure.go      | 23 +++++++++++++++++++----
 dm/pkg/checker/table_structure_test.go | 14 ++++++++------
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/dm/pkg/checker/table_structure.go b/dm/pkg/checker/table_structure.go
index 0c41f05eb3a..c2f976a4f61 100644
--- a/dm/pkg/checker/table_structure.go
+++ b/dm/pkg/checker/table_structure.go
@@ -438,14 +438,14 @@ func (c *ShardingTablesChecker) checkShardingTable(ctx context.Context, r *Resul
 				return err
 			}
 
-			if has := c.hasAutoIncrementKey(ctStmt); has {
+			if has := hasAutoIncrementKey(ctStmt); has {
 				c.reMu.Lock()
 				if r.State == StateSuccess {
 					r.State = StateWarning
-					r.Errors = append(r.Errors, NewError("sourceID %s table %v of sharding %s have auto-increment key, please make sure them don't conflict in target table!", sourceID, table, c.targetTableID))
 					r.Instruction = "If happen conflict, please handle it by yourself. You can refer to https://docs.pingcap.com/tidb-data-migration/stable/shard-merge-best-practices/#handle-conflicts-between-primary-keys-or-unique-indexes-across-multiple-sharded-tables"
 					r.Extra = AutoIncrementKeyChecking
 				}
+				r.Errors = append(r.Errors, NewError("sourceID %s table %v of sharding %s have auto-increment key, please make sure them don't conflict in target table!", sourceID, table, c.targetTableID))
 				c.reMu.Unlock()
 			}
 
@@ -462,7 +462,7 @@ func (c *ShardingTablesChecker) checkShardingTable(ctx context.Context, r *Resul
 	}
 }
 
-func (c *ShardingTablesChecker) hasAutoIncrementKey(stmt *ast.CreateTableStmt) bool {
+func hasAutoIncrementKey(stmt *ast.CreateTableStmt) bool {
 	for _, col := range stmt.Cols {
 		for _, opt := range col.Options {
 			if opt.Tp == ast.ColumnOptionAutoIncrement {
@@ -661,13 +661,28 @@ func (c *OptimisticShardingTablesChecker) checkTable(ctx context.Context, r *Res
 				return err
 			}
 
+			ctStmt, err := getCreateTableStmt(p, statement)
+			if err != nil {
+				return err
+			}
+
+			if has := hasAutoIncrementKey(ctStmt); has {
+				c.reMu.Lock()
+				if r.State == StateSuccess {
+					r.State = StateWarning
+					r.Instruction = "If happen conflict, please handle it by yourself. You can refer to https://docs.pingcap.com/tidb-data-migration/stable/shard-merge-best-practices/#handle-conflicts-between-primary-keys-or-unique-indexes-across-multiple-sharded-tables"
+					r.Extra = AutoIncrementKeyChecking
+				}
+				r.Errors = append(r.Errors, NewError("sourceID %s table %v of sharding %s have auto-increment key, please make sure them don't conflict in target table!", sourceID, table, c.targetTableID))
+				c.reMu.Unlock()
+			}
+
 			ti, err := dbutil.GetTableInfoBySQL(statement, p)
 			if err != nil {
 				return err
 			}
 			encodeTi := schemacmp.Encode(ti)
 			c.joinedMu.Lock()
-			log.L().Logger.Debug("get schemacmp", zap.Stringer("ti", encodeTi), zap.Stringer("joined", c.joined), zap.Bool("pk is handle", ti.PKIsHandle))
 			if c.joined == nil {
 				c.joined = &encodeTi
 				c.joinedMu.Unlock()
diff --git a/dm/pkg/checker/table_structure_test.go b/dm/pkg/checker/table_structure_test.go
index dd5eae11d0e..20c975f694e 100644
--- a/dm/pkg/checker/table_structure_test.go
+++ b/dm/pkg/checker/table_structure_test.go
@@ -216,7 +216,8 @@ func (t *testCheckSuite) TestOptimisticShardingTablesChecker(c *tc.C) {
 				"d" int(11) NOT NULL,
 				PRIMARY KEY ("c")
 				) ENGINE=InnoDB DEFAULT CHARSET=latin1`,
-			expectState: StateSuccess,
+			expectState: StateWarning,
+			errLen:      2, // 2 auto_increment warning
 		},
 		{
 			createTable1SQL: `CREATE TABLE "test-table-1" (
@@ -228,9 +229,10 @@ func (t *testCheckSuite) TestOptimisticShardingTablesChecker(c *tc.C) {
 				"d" int(11) NOT NULL,
 				PRIMARY KEY ("c")
 				) ENGINE=InnoDB DEFAULT CHARSET=latin1`,
-			expectState: StateSuccess,
+			expectState: StateWarning,
+			errLen:      1, // 1 auto_increment warning
 		},
-		// must set auto_increment with key
+		// must set auto_increment with key(failure)
 		{
 			createTable1SQL: `CREATE TABLE "test-table-1" (
 				"c" int(11) NOT NULL AUTO_INCREMENT
@@ -240,7 +242,7 @@ func (t *testCheckSuite) TestOptimisticShardingTablesChecker(c *tc.C) {
 				"d" int(11) NOT NULL
 				) ENGINE=InnoDB DEFAULT CHARSET=latin1`,
 			expectState: StateFailure,
-			errLen:      1,
+			errLen:      2, // 1 auto_increment warning
 		},
 		{
 			createTable1SQL: `CREATE TABLE "test-table-1" (
@@ -252,7 +254,7 @@ func (t *testCheckSuite) TestOptimisticShardingTablesChecker(c *tc.C) {
 				"d" int(11) NOT NULL
 				) ENGINE=InnoDB DEFAULT CHARSET=latin1`,
 			expectState: StateFailure,
-			errLen:      1,
+			errLen:      2, // 1 auto_increment warning
 		},
 		// different auto_increment
 		{
@@ -266,7 +268,7 @@ func (t *testCheckSuite) TestOptimisticShardingTablesChecker(c *tc.C) {
 				PRIMARY KEY ("d")
 				) ENGINE=InnoDB DEFAULT CHARSET=latin1`,
 			expectState: StateFailure,
-			errLen:      1,
+			errLen:      3, // 2 auto_increment warning
 		},
 	}