From 09c4290e4f200109fc4f71d1814f6f3680b659ed Mon Sep 17 00:00:00 2001 From: Yujie Xia Date: Mon, 19 Dec 2022 11:54:12 +0800 Subject: [PATCH 1/3] lightning: disable foreign key checks --- br/pkg/lightning/restore/tidb.go | 52 ++------- br/pkg/lightning/restore/tidb_test.go | 106 +----------------- br/tests/lightning_foreign_key/config.toml | 4 + .../data/fk.t-schema.sql | 8 ++ br/tests/lightning_foreign_key/data/fk.t.csv | 6 + br/tests/lightning_foreign_key/run.sh | 28 +++++ 6 files changed, 58 insertions(+), 146 deletions(-) create mode 100644 br/tests/lightning_foreign_key/config.toml create mode 100644 br/tests/lightning_foreign_key/data/fk.t-schema.sql create mode 100644 br/tests/lightning_foreign_key/data/fk.t.csv create mode 100644 br/tests/lightning_foreign_key/run.sh diff --git a/br/pkg/lightning/restore/tidb.go b/br/pkg/lightning/restore/tidb.go index ddc8bb6c5ff19..e0c6580c020cb 100644 --- a/br/pkg/lightning/restore/tidb.go +++ b/br/pkg/lightning/restore/tidb.go @@ -95,8 +95,10 @@ func DBFromConfig(ctx context.Context, dsn config.DBStore) (*sql.DB, error) { "tidb_opt_write_row_id": "1", // always set auto-commit to ON "autocommit": "1", - // alway set transaction mode to optimistic + // always set transaction mode to optimistic "tidb_txn_mode": "optimistic", + // disable foreign key checks + "foreign_key_checks": "0", } if dsn.Vars != nil { @@ -143,47 +145,6 @@ func (timgr *TiDBManager) Close() { timgr.db.Close() } -func InitSchema(ctx context.Context, g glue.Glue, database string, tablesSchema map[string]string) error { - logger := log.FromContext(ctx).With(zap.String("db", database)) - sqlExecutor := g.GetSQLExecutor() - - var createDatabase strings.Builder - createDatabase.WriteString("CREATE DATABASE IF NOT EXISTS ") - common.WriteMySQLIdentifier(&createDatabase, database) - err := sqlExecutor.ExecuteWithLog(ctx, createDatabase.String(), "create database", logger) - if err != nil { - return errors.Trace(err) - } - - task := logger.Begin(zap.InfoLevel, "create tables") - var sqlCreateStmts []string -loopCreate: - for tbl, sqlCreateTable := range tablesSchema { - task.Debug("create table", zap.String("schema", sqlCreateTable)) - - sqlCreateStmts, err = createIfNotExistsStmt(g.GetParser(), sqlCreateTable, database, tbl) - if err != nil { - break - } - - // TODO: maybe we should put these createStems into a transaction - for _, s := range sqlCreateStmts { - err = sqlExecutor.ExecuteWithLog( - ctx, - s, - "create table", - logger.With(zap.String("table", common.UniqueTable(database, tbl))), - ) - if err != nil { - break loopCreate - } - } - } - task.End(zap.ErrorLevel, err) - - return errors.Trace(err) -} - func createIfNotExistsStmt(p *parser.Parser, createTable, dbName, tblName string) ([]string, error) { stmts, _, err := p.ParseSQL(createTable) if err != nil { @@ -194,11 +155,15 @@ func createIfNotExistsStmt(p *parser.Parser, createTable, dbName, tblName string ctx := format.NewRestoreCtx(format.DefaultRestoreFlags|format.RestoreTiDBSpecialComment, &res) retStmts := make([]string, 0, len(stmts)) +loop: for _, stmt := range stmts { switch node := stmt.(type) { case *ast.CreateDatabaseStmt: node.Name = model.NewCIStr(dbName) node.IfNotExists = true + case *ast.DropDatabaseStmt: + node.Name = model.NewCIStr(dbName) + node.IfExists = true case *ast.CreateTableStmt: node.Table.Schema = model.NewCIStr(dbName) node.Table.Name = model.NewCIStr(tblName) @@ -210,6 +175,9 @@ func createIfNotExistsStmt(p *parser.Parser, createTable, dbName, tblName string node.Tables[0].Schema = model.NewCIStr(dbName) node.Tables[0].Name = model.NewCIStr(tblName) node.IfExists = true + default: + // ignore other statements + continue loop } if err := stmt.Restore(ctx); err != nil { return []string{}, common.ErrInvalidSchemaStmt.Wrap(err).GenWithStackByArgs(createTable) diff --git a/br/pkg/lightning/restore/tidb_test.go b/br/pkg/lightning/restore/tidb_test.go index 9b204b2da22b1..7bda02717a104 100644 --- a/br/pkg/lightning/restore/tidb_test.go +++ b/br/pkg/lightning/restore/tidb_test.go @@ -120,10 +120,8 @@ func TestCreateTableIfNotExistsStmt(t *testing.T) { require.Equal(t, []string{"CREATE TABLE IF NOT EXISTS `testdb`.`ba``r` (`x` INT);"}, createSQLIfNotExistsStmt("create table foo(x int);", "ba`r")) - // conditional comments + // conditional comments should be removed require.Equal(t, []string{ - "SET NAMES 'binary';", - "SET @@SESSION.`FOREIGN_KEY_CHECKS`=0;", "CREATE TABLE IF NOT EXISTS `testdb`.`m` (`z` DOUBLE) ENGINE = InnoDB AUTO_INCREMENT = 8343230 DEFAULT CHARACTER SET = UTF8;", }, createSQLIfNotExistsStmt(` @@ -133,20 +131,11 @@ func TestCreateTableIfNotExistsStmt(t *testing.T) { `, "m")) // create view + // all set statements are ignored require.Equal(t, []string{ - "SET NAMES 'binary';", "DROP TABLE IF EXISTS `testdb`.`m`;", "DROP VIEW IF EXISTS `testdb`.`m`;", - "SET @`PREV_CHARACTER_SET_CLIENT`=@@`character_set_client`;", - "SET @`PREV_CHARACTER_SET_RESULTS`=@@`character_set_results`;", - "SET @`PREV_COLLATION_CONNECTION`=@@`collation_connection`;", - "SET @@SESSION.`character_set_client`=`utf8`;", - "SET @@SESSION.`character_set_results`=`utf8`;", - "SET @@SESSION.`collation_connection`=`utf8_general_ci`;", "CREATE ALGORITHM = UNDEFINED DEFINER = `root`@`192.168.198.178` SQL SECURITY DEFINER VIEW `testdb`.`m` (`s`) AS SELECT `s` FROM `db1`.`v1` WHERE `i`<2;", - "SET @@SESSION.`character_set_client`=@`PREV_CHARACTER_SET_CLIENT`;", - "SET @@SESSION.`character_set_results`=@`PREV_CHARACTER_SET_RESULTS`;", - "SET @@SESSION.`collation_connection`=@`PREV_COLLATION_CONNECTION`;", }, createSQLIfNotExistsStmt(` /*!40101 SET NAMES binary*/; @@ -165,97 +154,6 @@ func TestCreateTableIfNotExistsStmt(t *testing.T) { `, "m")) } -func TestInitSchema(t *testing.T) { - s := newTiDBSuite(t) - ctx := context.Background() - - s.mockDB. - ExpectExec("CREATE DATABASE IF NOT EXISTS `db`"). - WillReturnResult(sqlmock.NewResult(1, 1)) - s.mockDB. - ExpectExec("\\QCREATE TABLE IF NOT EXISTS `db`.`t1` (`a` INT PRIMARY KEY,`b` VARCHAR(200));\\E"). - WillReturnResult(sqlmock.NewResult(2, 1)) - s.mockDB. - ExpectExec("\\QSET @@SESSION.`FOREIGN_KEY_CHECKS`=0;\\E"). - WillReturnResult(sqlmock.NewResult(0, 0)) - s.mockDB. - ExpectExec("\\QCREATE TABLE IF NOT EXISTS `db`.`t2` (`xx` TEXT) AUTO_INCREMENT = 11203;\\E"). - WillReturnResult(sqlmock.NewResult(2, 1)) - s.mockDB. - ExpectClose() - - s.mockDB.MatchExpectationsInOrder(false) // maps are unordered. - err := InitSchema(ctx, s.tiGlue, "db", map[string]string{ - "t1": "create table t1 (a int primary key, b varchar(200));", - "t2": "/*!40014 SET FOREIGN_KEY_CHECKS=0*/;CREATE TABLE `db`.`t2` (xx TEXT) AUTO_INCREMENT=11203;", - }) - s.mockDB.MatchExpectationsInOrder(true) - require.NoError(t, err) -} - -func TestInitSchemaSyntaxError(t *testing.T) { - s := newTiDBSuite(t) - ctx := context.Background() - - s.mockDB. - ExpectExec("CREATE DATABASE IF NOT EXISTS `db`"). - WillReturnResult(sqlmock.NewResult(1, 1)) - s.mockDB. - ExpectClose() - - err := InitSchema(ctx, s.tiGlue, "db", map[string]string{ - "t1": "create table `t1` with invalid syntax;", - }) - require.Error(t, err) -} - -func TestInitSchemaErrorLost(t *testing.T) { - s := newTiDBSuite(t) - ctx := context.Background() - - s.mockDB. - ExpectExec("CREATE DATABASE IF NOT EXISTS `db`"). - WillReturnResult(sqlmock.NewResult(1, 1)) - - s.mockDB. - ExpectExec("CREATE TABLE IF NOT EXISTS.*"). - WillReturnError(&mysql.MySQLError{ - Number: tmysql.ErrTooBigFieldlength, - Message: "Column length too big", - }) - - s.mockDB. - ExpectClose() - - err := InitSchema(ctx, s.tiGlue, "db", map[string]string{ - "t1": "create table `t1` (a int);", - "t2": "create table t2 (a int primary key, b varchar(200));", - }) - require.Regexp(t, ".*Column length too big.*", err.Error()) -} - -func TestInitSchemaUnsupportedSchemaError(t *testing.T) { - s := newTiDBSuite(t) - ctx := context.Background() - - s.mockDB. - ExpectExec("CREATE DATABASE IF NOT EXISTS `db`"). - WillReturnResult(sqlmock.NewResult(1, 1)) - s.mockDB. - ExpectExec("CREATE TABLE IF NOT EXISTS `db`.`t1`.*"). - WillReturnError(&mysql.MySQLError{ - Number: tmysql.ErrTooBigFieldlength, - Message: "Column length too big", - }) - s.mockDB. - ExpectClose() - - err := InitSchema(ctx, s.tiGlue, "db", map[string]string{ - "t1": "create table `t1` (a VARCHAR(999999999));", - }) - require.Regexp(t, ".*Column length too big.*", err.Error()) -} - func TestDropTable(t *testing.T) { s := newTiDBSuite(t) ctx := context.Background() diff --git a/br/tests/lightning_foreign_key/config.toml b/br/tests/lightning_foreign_key/config.toml new file mode 100644 index 0000000000000..37a1026dc3b6c --- /dev/null +++ b/br/tests/lightning_foreign_key/config.toml @@ -0,0 +1,4 @@ +[tikv-importer] +# Set on-duplicate=error to force using insert statement to write data. +# It seems that foreign key check is not supported in replace statement. +on-duplicate = "error" diff --git a/br/tests/lightning_foreign_key/data/fk.t-schema.sql b/br/tests/lightning_foreign_key/data/fk.t-schema.sql new file mode 100644 index 0000000000000..98f00b9cadca8 --- /dev/null +++ b/br/tests/lightning_foreign_key/data/fk.t-schema.sql @@ -0,0 +1,8 @@ +CREATE TABLE `t` +( + `a` bigint(20) NOT NULL, + `b` bigint(20) DEFAULT NULL, + PRIMARY KEY (`a`) /*T![clustered_index] CLUSTERED */, + KEY `fk_1` (`b`), + CONSTRAINT `fk_1` FOREIGN KEY (`b`) REFERENCES `test`.`t2` (`a`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin; diff --git a/br/tests/lightning_foreign_key/data/fk.t.csv b/br/tests/lightning_foreign_key/data/fk.t.csv new file mode 100644 index 0000000000000..cd0368580a4c8 --- /dev/null +++ b/br/tests/lightning_foreign_key/data/fk.t.csv @@ -0,0 +1,6 @@ +a,b +1,1 +2,2 +3,3 +4,4 +5,5 diff --git a/br/tests/lightning_foreign_key/run.sh b/br/tests/lightning_foreign_key/run.sh new file mode 100644 index 0000000000000..631b55d907102 --- /dev/null +++ b/br/tests/lightning_foreign_key/run.sh @@ -0,0 +1,28 @@ +#!/bin/bash +# +# Copyright 2022 PingCAP, Inc. +# +# 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. + +set -eu + +# Create existing tables that import data will reference. +run_sql 'CREATE DATABASE IF NOT EXISTS fk;' +run_sql 'CREATE TABLE fk.t2 (a BIGINT PRIMARY KEY);' + +for BACKEND in tidb local; do + run_sql 'DROP TABLE IF EXISTS fk.t;' + run_lightning --backend $BACKEND + run_sql 'SELECT GROUP_CONCAT(a) FROM fk.t ORDER BY a;' + check_contains '1,2,3,4,5' +done From 376e42b833e11f5e8ebdecb2eb2d1c085eeb4154 Mon Sep 17 00:00:00 2001 From: Yujie Xia Date: Wed, 21 Dec 2022 16:01:29 +0800 Subject: [PATCH 2/3] do not ignore set statements --- br/pkg/lightning/restore/tidb.go | 4 ---- br/pkg/lightning/restore/tidb_test.go | 15 +++++++++++++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/br/pkg/lightning/restore/tidb.go b/br/pkg/lightning/restore/tidb.go index e0c6580c020cb..00d49af646e3c 100644 --- a/br/pkg/lightning/restore/tidb.go +++ b/br/pkg/lightning/restore/tidb.go @@ -155,7 +155,6 @@ func createIfNotExistsStmt(p *parser.Parser, createTable, dbName, tblName string ctx := format.NewRestoreCtx(format.DefaultRestoreFlags|format.RestoreTiDBSpecialComment, &res) retStmts := make([]string, 0, len(stmts)) -loop: for _, stmt := range stmts { switch node := stmt.(type) { case *ast.CreateDatabaseStmt: @@ -175,9 +174,6 @@ loop: node.Tables[0].Schema = model.NewCIStr(dbName) node.Tables[0].Name = model.NewCIStr(tblName) node.IfExists = true - default: - // ignore other statements - continue loop } if err := stmt.Restore(ctx); err != nil { return []string{}, common.ErrInvalidSchemaStmt.Wrap(err).GenWithStackByArgs(createTable) diff --git a/br/pkg/lightning/restore/tidb_test.go b/br/pkg/lightning/restore/tidb_test.go index 7bda02717a104..a3710d822d2dd 100644 --- a/br/pkg/lightning/restore/tidb_test.go +++ b/br/pkg/lightning/restore/tidb_test.go @@ -120,8 +120,10 @@ func TestCreateTableIfNotExistsStmt(t *testing.T) { require.Equal(t, []string{"CREATE TABLE IF NOT EXISTS `testdb`.`ba``r` (`x` INT);"}, createSQLIfNotExistsStmt("create table foo(x int);", "ba`r")) - // conditional comments should be removed + // conditional comments require.Equal(t, []string{ + "SET NAMES 'binary';", + "SET @@SESSION.`FOREIGN_KEY_CHECKS`=0;", "CREATE TABLE IF NOT EXISTS `testdb`.`m` (`z` DOUBLE) ENGINE = InnoDB AUTO_INCREMENT = 8343230 DEFAULT CHARACTER SET = UTF8;", }, createSQLIfNotExistsStmt(` @@ -131,11 +133,20 @@ func TestCreateTableIfNotExistsStmt(t *testing.T) { `, "m")) // create view - // all set statements are ignored require.Equal(t, []string{ + "SET NAMES 'binary';", "DROP TABLE IF EXISTS `testdb`.`m`;", "DROP VIEW IF EXISTS `testdb`.`m`;", + "SET @`PREV_CHARACTER_SET_CLIENT`=@@`character_set_client`;", + "SET @`PREV_CHARACTER_SET_RESULTS`=@@`character_set_results`;", + "SET @`PREV_COLLATION_CONNECTION`=@@`collation_connection`;", + "SET @@SESSION.`character_set_client`=`utf8`;", + "SET @@SESSION.`character_set_results`=`utf8`;", + "SET @@SESSION.`collation_connection`=`utf8_general_ci`;", "CREATE ALGORITHM = UNDEFINED DEFINER = `root`@`192.168.198.178` SQL SECURITY DEFINER VIEW `testdb`.`m` (`s`) AS SELECT `s` FROM `db1`.`v1` WHERE `i`<2;", + "SET @@SESSION.`character_set_client`=@`PREV_CHARACTER_SET_CLIENT`;", + "SET @@SESSION.`character_set_results`=@`PREV_CHARACTER_SET_RESULTS`;", + "SET @@SESSION.`collation_connection`=@`PREV_COLLATION_CONNECTION`;", }, createSQLIfNotExistsStmt(` /*!40101 SET NAMES binary*/; From 188723c4a23435b2e1083697e511eea600764c3f Mon Sep 17 00:00:00 2001 From: Yujie Xia Date: Fri, 23 Dec 2022 14:20:31 +0800 Subject: [PATCH 3/3] Update br/tests/lightning_foreign_key/config.toml Co-authored-by: lance6716 --- br/tests/lightning_foreign_key/config.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/br/tests/lightning_foreign_key/config.toml b/br/tests/lightning_foreign_key/config.toml index 37a1026dc3b6c..3c85a830bfa22 100644 --- a/br/tests/lightning_foreign_key/config.toml +++ b/br/tests/lightning_foreign_key/config.toml @@ -1,4 +1,3 @@ [tikv-importer] # Set on-duplicate=error to force using insert statement to write data. -# It seems that foreign key check is not supported in replace statement. on-duplicate = "error"