cherry pick #1335 to release-2.0 (#1344)

_utils/terror_gen/errors_release.txt

@@ -9,7 +9,7 @@ ErrGetFileSize,[code=11002:class=functional:scope=internal:level=high], "Message
 ErrDropMultipleTables,[code=11003:class=functional:scope=internal:level=high], "Message: not allowed operation: drop multiple tables in one statement, Workaround: It is recommended to include only one DDL operation in a statement executed upstream. Please manually handle it using dmctl (skipping the DDL statement or replacing the DDL statement with a specified DDL statement). For details, see https://docs.pingcap.com/tidb-data-migration/stable/handle-failed-sql-statements"
 ErrRenameMultipleTables,[code=11004:class=functional:scope=internal:level=high], "Message: not allowed operation: rename multiple tables in one statement, Workaround: It is recommended to include only one DDL operation in a statement executed upstream. Please manually handle it using dmctl (skipping the DDL statement or replacing the DDL statement with a specified DDL statement). For details, see https://docs.pingcap.com/tidb-data-migration/stable/handle-failed-sql-statements"
 ErrAlterMultipleTables,[code=11005:class=functional:scope=internal:level=high], "Message: not allowed operation: alter multiple tables in one statement, Workaround: It is recommended to include only one DDL operation in a statement executed upstream. Please manually handle it using dmctl (skipping the DDL statement or replacing the DDL statement with a specified DDL statement). For details, see https://docs.pingcap.com/tidb-data-migration/stable/handle-failed-sql-statements"
-ErrParseSQL,[code=11006:class=functional:scope=internal:level=high], "Message: parse statement, Workaround: Please manually handle it using dmctl (skipping the DDL statement or replacing the DDL statement with a specified DDL statement). For details, see https://docs.pingcap.com/tidb-data-migration/stable/handle-failed-sql-statements"
+ErrParseSQL,[code=11006:class=functional:scope=internal:level=high], "Message: parse statement: %s"
 ErrUnknownTypeDDL,[code=11007:class=functional:scope=internal:level=high], "Message: unknown type ddl %+v, Workaround: Please manually handle it using dmctl (skipping the DDL statement or replacing the DDL statement with a specified DDL statement). For details, see https://docs.pingcap.com/tidb-data-migration/stable/handle-failed-sql-statements"
 ErrRestoreASTNode,[code=11008:class=functional:scope=internal:level=high], "Message: restore ast node"
 ErrParseGTID,[code=11009:class=functional:scope=internal:level=high], "Message: parse GTID %s"
@@ -312,6 +312,7 @@ ErrSyncerFailpoint,[code=36063:class=sync-unit:scope=internal:level=low], "Messa
 ErrSyncerReplaceEvent,[code=36064:class=sync-unit:scope=internal:level=high]
 ErrSyncerOperatorNotExist,[code=36065:class=sync-unit:scope=internal:level=low], "Message: error operator not exist, position: %s"
 ErrSyncerReplaceEventNotExist,[code=36066:class=sync-unit:scope=internal:level=high], "Message: replace event not exist, location: %s"
+ErrSyncerParseDDL,[code=36067:class=sync-unit:scope=internal:level=high], "Message: parse DDL: %s, Workaround: Please confirm your DDL statement is correct and needed. For TiDB compatible DDL, see https://docs.pingcap.com/tidb/stable/mysql-compatibility#ddl. You can use `handle-error` command to skip or replace the DDL or add a binlog filter rule to ignore it if the DDL is not needed."
 ErrMasterSQLOpNilRequest,[code=38001:class=dm-master:scope=internal:level=medium], "Message: nil request not valid"
 ErrMasterSQLOpNotSupport,[code=38002:class=dm-master:scope=internal:level=medium], "Message: op %s not supported"
 ErrMasterSQLOpWithoutSharding,[code=38003:class=dm-master:scope=internal:level=medium], "Message: operate request without --sharding specified not valid"

dm/config/source_config.go

@@ -232,7 +232,8 @@ func (c *SourceConfig) Adjust(ctx context.Context, db *sql.DB) (err error) {
 		}
 	}
 
-	if c.EnableGTID {
+	// MariaDB automatically enabled gtid after 10.0.2, refer to https://mariadb.com/kb/en/gtid/#using-global-transaction-ids
+	if c.EnableGTID && c.Flavor != mysql.MariaDBFlavor {
 		val, err := utils.GetGTIDMode(ctx2, db)
 		if err != nil {
 			return err

errors.toml

@@ -65,9 +65,9 @@ workaround = "It is recommended to include only one DDL operation in a statement
 tags = ["internal", "high"]
 
 [error.DM-functional-11006]
-message = "parse statement"
+message = "parse statement: %s"
 description = ""
-workaround = "Please manually handle it using dmctl (skipping the DDL statement or replacing the DDL statement with a specified DDL statement). For details, see https://docs.pingcap.com/tidb-data-migration/stable/handle-failed-sql-statements"
+workaround = ""
 tags = ["internal", "high"]
 
 [error.DM-functional-11007]
@@ -1882,6 +1882,12 @@ description = ""
 workaround = ""
 tags = ["internal", "high"]
 
+[error.DM-sync-unit-36067]
+message = "parse DDL: %s"
+description = ""
+workaround = "Please confirm your DDL statement is correct and needed. For TiDB compatible DDL, see https://docs.pingcap.com/tidb/stable/mysql-compatibility#ddl. You can use `handle-error` command to skip or replace the DDL or add a binlog filter rule to ignore it if the DDL is not needed."
+tags = ["internal", "high"]
+
 [error.DM-dm-master-38001]
 message = "nil request not valid"
 description = ""

pkg/dumpling/utils_test.go

@@ -232,10 +232,10 @@ Finished dump at: 2020-09-30 12:16:49
 	}
 
 	for _, tc := range testCases {
-		err := ioutil.WriteFile(f.Name(), []byte(tc.source), 0644)
-		c.Assert(err, IsNil)
-		loc, loc2, err := ParseMetaData(f.Name(), "mysql")
-		c.Assert(err, IsNil)
+		err2 := ioutil.WriteFile(f.Name(), []byte(tc.source), 0644)
+		c.Assert(err2, IsNil)
+		loc, loc2, err2 := ParseMetaData(f.Name(), "mysql")
+		c.Assert(err2, IsNil)
 		c.Assert(loc.Position, DeepEquals, tc.pos)
 		gs, _ := gtid.ParserGTID("mysql", tc.gsetStr)
 		c.Assert(loc.GetGTID(), DeepEquals, gs)

pkg/terror/error_list.go

@@ -401,6 +401,7 @@ const (
 	codeSyncerReplaceEvent
 	codeSyncerOperatorNotExist
 	codeSyncerReplaceEventNotExist
+	codeSyncerParseDDL
 )
 
 // DM-master error code
@@ -622,7 +623,7 @@ var (
 	ErrDropMultipleTables     = New(codeDropMultipleTables, ClassFunctional, ScopeInternal, LevelHigh, "not allowed operation: drop multiple tables in one statement", "It is recommended to include only one DDL operation in a statement executed upstream. Please manually handle it using dmctl (skipping the DDL statement or replacing the DDL statement with a specified DDL statement). For details, see https://docs.pingcap.com/tidb-data-migration/stable/handle-failed-sql-statements")
 	ErrRenameMultipleTables   = New(codeRenameMultipleTables, ClassFunctional, ScopeInternal, LevelHigh, "not allowed operation: rename multiple tables in one statement", "It is recommended to include only one DDL operation in a statement executed upstream. Please manually handle it using dmctl (skipping the DDL statement or replacing the DDL statement with a specified DDL statement). For details, see https://docs.pingcap.com/tidb-data-migration/stable/handle-failed-sql-statements")
 	ErrAlterMultipleTables    = New(codeAlterMultipleTables, ClassFunctional, ScopeInternal, LevelHigh, "not allowed operation: alter multiple tables in one statement", "It is recommended to include only one DDL operation in a statement executed upstream. Please manually handle it using dmctl (skipping the DDL statement or replacing the DDL statement with a specified DDL statement). For details, see https://docs.pingcap.com/tidb-data-migration/stable/handle-failed-sql-statements")
-	ErrParseSQL               = New(codeParseSQL, ClassFunctional, ScopeInternal, LevelHigh, "parse statement", "Please manually handle it using dmctl (skipping the DDL statement or replacing the DDL statement with a specified DDL statement). For details, see https://docs.pingcap.com/tidb-data-migration/stable/handle-failed-sql-statements")
+	ErrParseSQL               = New(codeParseSQL, ClassFunctional, ScopeInternal, LevelHigh, "parse statement: %s", "")
 	ErrUnknownTypeDDL         = New(codeUnknownTypeDDL, ClassFunctional, ScopeInternal, LevelHigh, "unknown type ddl %+v", "Please manually handle it using dmctl (skipping the DDL statement or replacing the DDL statement with a specified DDL statement). For details, see https://docs.pingcap.com/tidb-data-migration/stable/handle-failed-sql-statements")
 	ErrRestoreASTNode         = New(codeRestoreASTNode, ClassFunctional, ScopeInternal, LevelHigh, "restore ast node", "")
 	ErrParseGTID              = New(codeParseGTID, ClassFunctional, ScopeInternal, LevelHigh, "parse GTID %s", "")
@@ -976,6 +977,7 @@ var (
 	ErrSyncerReplaceEvent                   = New(codeSyncerReplaceEvent, ClassSyncUnit, ScopeInternal, LevelHigh, "", "")
 	ErrSyncerOperatorNotExist               = New(codeSyncerOperatorNotExist, ClassSyncUnit, ScopeInternal, LevelLow, "error operator not exist, position: %s", "")
 	ErrSyncerReplaceEventNotExist           = New(codeSyncerReplaceEventNotExist, ClassSyncUnit, ScopeInternal, LevelHigh, "replace event not exist, location: %s", "")
+	ErrSyncerParseDDL                       = New(codeSyncerParseDDL, ClassSyncUnit, ScopeInternal, LevelHigh, "parse DDL: %s", "Please confirm your DDL statement is correct and needed. For TiDB compatible DDL, see https://docs.pingcap.com/tidb/stable/mysql-compatibility#ddl. You can use `handle-error` command to skip or replace the DDL or add a binlog filter rule to ignore it if the DDL is not needed.")
 
 	// DM-master error
 	ErrMasterSQLOpNilRequest        = New(codeMasterSQLOpNilRequest, ClassDMMaster, ScopeInternal, LevelMedium, "nil request not valid", "")

syncer/ddl.go

@@ -26,17 +26,6 @@ import (
 	"github.com/pingcap/dm/pkg/utils"
 )
 
-var (
-	// IncompatibleDDLFormat is for incompatible ddl
-	IncompatibleDDLFormat = `encountered incompatible DDL in TiDB:
-	please confirm your DDL statement is correct and needed.
-	for TiDB compatible DDL, please see the docs:
-	  English version: https://pingcap.com/docs/dev/reference/mysql-compatibility/#ddl
-	  Chinese version: https://pingcap.com/docs-cn/dev/reference/mysql-compatibility/#ddl
-	if the DDL is not needed, you can use a filter rule with "*" schema-pattern to ignore it.
-	 `
-)
-
 // parseDDLResult represents the result of parseDDLSQL
 type parseDDLResult struct {
 	stmt   ast.StmtNode
@@ -68,12 +57,12 @@ func (s *Syncer) parseDDLSQL(sql string, p *parser.Parser, schema string) (resul
 	stmts, err := parserpkg.Parse(p, sql, "", "")
 	if err != nil {
 		// log error rather than fatal, so other defer can be executed
-		s.tctx.L().Error(IncompatibleDDLFormat, zap.String("sql", sql))
+		s.tctx.L().Error("parse ddl", zap.String("sql", sql))
 		return parseDDLResult{
 			stmt:   nil,
 			ignore: false,
 			isDDL:  false,
-		}, terror.Annotatef(err, IncompatibleDDLFormat, sql)
+		}, terror.ErrSyncerParseDDL.Delegate(err, sql)
 	}
 
 	if len(stmts) == 0 {

tests/handle_error/run.sh

@@ -1384,6 +1384,32 @@ function DM_4231() {
      run_case 4231 "single-source-no-sharding" "init_table 11" "clean_table" ""
 }
 
+function DM_SKIP_INCOMPATIBLE_DDL_CASE() {
+    run_sql_source1 "insert into ${db}.${tb1} values(1);"
+
+    run_sql_source1 "CREATE FUNCTION ${db}.hello (s CHAR(20)) RETURNS CHAR(50) DETERMINISTIC RETURN CONCAT('Hello, ',s,'!');"
+    run_dm_ctl_with_retry $WORK_DIR "127.0.0.1:$MASTER_PORT" \
+            "query-status test" \
+            "\"stage\": \"Running\"" 2
+
+    run_sql_source1 "/*!50003 drop function ${db}.hello*/;"
+    run_dm_ctl_with_retry $WORK_DIR "127.0.0.1:$MASTER_PORT" \
+            "query-status test" \
+            "drop function `hello`" 1 \
+            "Please confirm your DDL statement is correct and needed." 1
+
+    run_dm_ctl $WORK_DIR "127.0.0.1:$MASTER_PORT" \
+            "handle-error test skip" \
+            "\"result\": true" 2
+
+    run_sql_source1 "insert into ${db}.${tb1} values(2);"
+    check_sync_diff $WORK_DIR $cur/conf/diff_config.toml
+}
+
+function DM_SKIP_INCOMPATIBLE_DDL() {
+    run_case SKIP_INCOMPATIBLE_DDL "single-source-no-sharding" "init_table 11" "clean_table" ""
+}
+
 function run() {
     init_cluster
     init_database
@@ -1393,6 +1419,7 @@ function run() {
     DM_REPLACE_ERROR_SHARDING
     DM_REPLACE_ERROR_MULTIPLE
     DM_EXEC_ERROR_SKIP
+    DM_SKIP_INCOMPATIBLE_DDL
 
     implement=(4202 4204 4206 4207 4209 4211 4213 4215 4216 4219 4220 4185 4201 4189 4210 4193 4230 4177 4231)
     for i in ${implement[@]}; do

tests/sharding/run.sh

@@ -18,8 +18,7 @@ EOF
 
 function run() {
     run_sql "SET @@GLOBAL.SQL_MODE='NO_ZERO_IN_DATE,NO_ZERO_DATE'" $MYSQL_PORT1 $MYSQL_PASSWORD1
-    run_sql "SET @@GLOBAL.SQL_MODE='NO_ZERO_IN_DATE,NO_ZERO_DATE,ANSI_QUOTES'" $MYSQL_PORT2 $MYSQL_PASSWORD2
-    run_sql_tidb "SET @@GLOBAL.SQL_MODE='NO_ZERO_IN_DATE,NO_ZERO_DATE'"
+    run_sql "SET @@GLOBAL.SQL_MODE='ANSI_QUOTES'" $MYSQL_PORT2 $MYSQL_PASSWORD2
 
     run_sql_file $cur/data/db1.prepare.sql $MYSQL_HOST1 $MYSQL_PORT1 $MYSQL_PASSWORD1
     check_contains 'Query OK, 2 rows affected'
@@ -60,7 +59,7 @@ function run() {
     # TODO: check sharding partition id
     # use sync_diff_inspector to check full dump loader
     echo "check sync diff for full dump and load"
-    run_sql "SET @@GLOBAL.SQL_MODE='NO_ZERO_IN_DATE,NO_ZERO_DATE'" $MYSQL_PORT2 $MYSQL_PASSWORD2
+    run_sql "SET @@GLOBAL.SQL_MODE=''" $MYSQL_PORT2 $MYSQL_PASSWORD2
     check_sync_diff $WORK_DIR $cur/conf/diff_config.toml
 
     run_sql_file $cur/data/db1.increment.sql $MYSQL_HOST1 $MYSQL_PORT1 $MYSQL_PASSWORD1
@@ -149,7 +148,6 @@ function run() {
         "task test has no source or not exist" 1
 
     run_sql_both_source "SET @@GLOBAL.SQL_MODE='ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION'"
-    run_sql_tidb "SET @@GLOBAL.SQL_MODE='ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION'"
 }
 
 cleanup_data db_target