Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dumpling: supports foreign key limitations #39914

Merged
merged 8 commits into from
Dec 16, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions dumpling/export/dump.go
Original file line number Diff line number Diff line change
Expand Up @@ -1219,9 +1219,7 @@ func dumpTableMeta(tctx *tcontext.Context, conf *Config, conn *BaseConn, db stri
selectedField: selectField,
selectedLen: selectLen,
hasImplicitRowID: hasImplicitRowID,
specCmts: []string{
"/*!40101 SET NAMES binary*/;",
},
specCmts: getSpecialComments(conf.ServerInfo.ServerType),
}

if conf.NoSchemas {
Expand Down
5 changes: 3 additions & 2 deletions dumpling/export/ir.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"strings"

"github.com/pingcap/errors"
"github.com/pingcap/tidb/br/pkg/version"
tcontext "github.com/pingcap/tidb/dumpling/context"
)

Expand Down Expand Up @@ -85,7 +86,7 @@ type MetaIR interface {
MetaSQL() string
}

func setTableMetaFromRows(rows *sql.Rows) (TableMeta, error) {
func setTableMetaFromRows(serverType version.ServerType, rows *sql.Rows) (TableMeta, error) {
tps, err := rows.ColumnTypes()
if err != nil {
return nil, errors.Trace(err)
Expand All @@ -101,6 +102,6 @@ func setTableMetaFromRows(rows *sql.Rows) (TableMeta, error) {
colTypes: tps,
selectedField: strings.Join(nms, ","),
selectedLen: len(nms),
specCmts: []string{"/*!40101 SET NAMES binary*/;"},
specCmts: getSpecialComments(serverType),
}, nil
}
20 changes: 20 additions & 0 deletions dumpling/export/ir_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"strings"

"github.com/pingcap/errors"
"github.com/pingcap/tidb/br/pkg/version"
tcontext "github.com/pingcap/tidb/dumpling/context"
"go.uber.org/zap"
)
Expand Down Expand Up @@ -370,3 +371,22 @@ func (td *multiQueriesChunk) Close() error {
func (*multiQueriesChunk) RawRows() *sql.Rows {
return nil
}

var serverSpecialComments = map[version.ServerType][]string{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these comments for dump files or queries?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems they are written to dump files. These comments take effects when importing not dumping, so we should not using upstream database to decide comments

Copy link
Contributor Author

@lichunzhu lichunzhu Dec 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These comments are used for dumped data files. They are used for scenes that we directly restore sqls back through MySQL cmd.
For import tools, they need to re-decide the used comments. These changes are copied from mydumper https://github.com/mydumper/mydumper/blob/e4d05b1a76b7a529d24c43a4d043790f355c5085/src/mydumper_jobs.c#L290. I don't think export tools adding this comment is conflicted with import tools.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need an integration test with lightning later to check we can handle mariadb case😂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/pingcap/tiflow/blob/099da1e08f1310c2366093f2c314a51e0cde772b/dm/loader/loader.go#L1218

DM loader may not be able to handle SET FOREIGN_KEY_CHECKS=0;, not sure what's the behaviour for lightning

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Lightning shouldn't read these comments😂 It should add its own comments according to downstream database.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ExpectExec("\\QSET @@SESSION.`FOREIGN_KEY_CHECKS`=0;\\E").

Seems lightning will handle these comments.

Copy link
Contributor Author

@lichunzhu lichunzhu Dec 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/pingcap/tiflow/blob/099da1e08f1310c2366093f2c314a51e0cde772b/dm/loader/loader.go#L1218

DM loader may not be able to handle SET FOREIGN_KEY_CHECKS=0;, not sure what's the behaviour for lightning

I think DM loader's handle is correct. Loader should execute foreign key handlement itself when it finds its downstream is TiDB. See https://github.com/mydumper/mydumper/blob/e4d05b1a76b7a529d24c43a4d043790f355c5085/src/myloader.c#L388

version.ServerTypeMySQL: {
"/*!40014 SET FOREIGN_KEY_CHECKS=0*/;",
"/*!40101 SET NAMES binary*/;",
},
version.ServerTypeTiDB: {
"/*!40014 SET FOREIGN_KEY_CHECKS=0*/;",
"/*!40101 SET NAMES binary*/;",
},
version.ServerTypeMariaDB: {
"/*!40101 SET NAMES binary*/;",
"SET FOREIGN_KEY_CHECKS=0;",
},
}

func getSpecialComments(serverType version.ServerType) []string {
return serverSpecialComments[serverType]
}
26 changes: 12 additions & 14 deletions dumpling/export/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (w *Writer) WritePolicyMeta(policy, createSQL string) error {
if err != nil {
return err
}
return writeMetaToFile(tctx, "placement-policy", createSQL, w.extStorage, fileName+".sql", conf.CompressType)
return w.writeMetaToFile(tctx, "placement-policy", createSQL, fileName+".sql")
}

// WriteDatabaseMeta writes database meta to a file
Expand All @@ -143,7 +143,7 @@ func (w *Writer) WriteDatabaseMeta(db, createSQL string) error {
if err != nil {
return err
}
return writeMetaToFile(tctx, db, createSQL, w.extStorage, fileName+".sql", conf.CompressType)
return w.writeMetaToFile(tctx, db, createSQL, fileName+".sql")
}

// WriteTableMeta writes table meta to a file
Expand All @@ -153,7 +153,7 @@ func (w *Writer) WriteTableMeta(db, table, createSQL string) error {
if err != nil {
return err
}
return writeMetaToFile(tctx, db, createSQL, w.extStorage, fileName+".sql", conf.CompressType)
return w.writeMetaToFile(tctx, db, createSQL, fileName+".sql")
}

// WriteViewMeta writes view meta to a file
Expand All @@ -167,11 +167,11 @@ func (w *Writer) WriteViewMeta(db, view, createTableSQL, createViewSQL string) e
if err != nil {
return err
}
err = writeMetaToFile(tctx, db, createTableSQL, w.extStorage, fileNameTable+".sql", conf.CompressType)
err = w.writeMetaToFile(tctx, db, createTableSQL, fileNameTable+".sql")
if err != nil {
return err
}
return writeMetaToFile(tctx, db, createViewSQL, w.extStorage, fileNameView+".sql", conf.CompressType)
return w.writeMetaToFile(tctx, db, createViewSQL, fileNameView+".sql")
}

// WriteSequenceMeta writes sequence meta to a file
Expand All @@ -181,7 +181,7 @@ func (w *Writer) WriteSequenceMeta(db, sequence, createSQL string) error {
if err != nil {
return err
}
return writeMetaToFile(tctx, db, createSQL, w.extStorage, fileName+".sql", conf.CompressType)
return w.writeMetaToFile(tctx, db, createSQL, fileName+".sql")
}

// WriteTableData writes table data to a file with retry
Expand Down Expand Up @@ -213,7 +213,7 @@ func (w *Writer) WriteTableData(meta TableMeta, ir TableDataIR, currentChunk int
return
}
if conf.SQL != "" {
meta, err = setTableMetaFromRows(ir.RawRows())
meta, err = setTableMetaFromRows(w.conf.ServerInfo.ServerType, ir.RawRows())
if err != nil {
return err
}
Expand Down Expand Up @@ -270,19 +270,17 @@ func (w *Writer) tryToWriteTableData(tctx *tcontext.Context, meta TableMeta, ir
return nil
}

func writeMetaToFile(tctx *tcontext.Context, target, metaSQL string, s storage.ExternalStorage, path string, compressType storage.CompressType) error {
fileWriter, tearDown, err := buildFileWriter(tctx, s, path, compressType)
func (w *Writer) writeMetaToFile(tctx *tcontext.Context, target, metaSQL string, path string) error {
fileWriter, tearDown, err := buildFileWriter(tctx, w.extStorage, path, w.conf.CompressType)
if err != nil {
return errors.Trace(err)
}
defer tearDown(tctx)

return WriteMeta(tctx, &metaData{
target: target,
metaSQL: metaSQL,
specCmts: []string{
"/*!40101 SET NAMES binary*/;",
},
target: target,
metaSQL: metaSQL,
specCmts: getSpecialComments(w.conf.ServerInfo.ServerType),
}, fileWriter)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
INSERT INTO `escape` VALUES
('''', '"'),
Expand Down
1 change: 1 addition & 0 deletions dumpling/tests/naughty_strings/data/naughty_strings.t.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
INSERT INTO `t` VALUES
(''),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
INSERT INTO `escape` VALUES
('\'','\"'),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
INSERT INTO `t` VALUES
(''),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
/*T![placement] CREATE PLACEMENT POLICY `x` PRIMARY_REGION="cn-east-1" REGIONS="cn-east-1,cn-east" */;
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
/*T![placement] CREATE PLACEMENT POLICY `x1` FOLLOWERS=4 */;
1 change: 1 addition & 0 deletions dumpling/tests/primary_key/result/pk_case_0.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
INSERT INTO `pk_case_0` VALUES
(0,10),
Expand Down
1 change: 1 addition & 0 deletions dumpling/tests/primary_key/result/pk_case_1.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
INSERT INTO `pk_case_1` VALUES
(0,10),
Expand Down
1 change: 1 addition & 0 deletions dumpling/tests/primary_key/result/pk_case_2.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
INSERT INTO `pk_case_2` VALUES
(0,10),
Expand Down
1 change: 1 addition & 0 deletions dumpling/tests/primary_key/result/pk_case_3.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
INSERT INTO `pk_case_3` VALUES
(6,4,x'000000000101000000000000000000f03f000000000000f03f'),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
CREATE DATABASE `quo``te/database` /*!40100 DEFAULT CHARACTER SET latin1 */;
1 change: 1 addition & 0 deletions dumpling/tests/quote/data/quote-database-schema-create.sql
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
CREATE DATABASE `quo``te/database` /*!40100 DEFAULT CHARACTER SET latin1 */ /*!80016 DEFAULT ENCRYPTION='N' */;
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
CREATE TABLE `quo``te/table` (
`quo``te/col` int(11) NOT NULL,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
CREATE TABLE `quo``te/table` (
`quo``te/col` int NOT NULL,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
INSERT INTO `quo``te/table` (`quo``te/col`,`a`) VALUES
(0,10),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
INSERT INTO `quo``te/table` (`quo``te/col`,`a`) VALUES
(0,10),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
CREATE SEQUENCE `s` start with 1 minvalue 1 maxvalue 9223372036854775806 increment by 1 cache 1000 nocycle ENGINE=InnoDB;
SELECT SETVAL(`s`,1001);
1 change: 1 addition & 0 deletions dumpling/tests/views/data/views-schema-create.sql
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
CREATE DATABASE `views` /*!40100 DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_bin */ /*!80016 DEFAULT ENCRYPTION='N' */;
1 change: 1 addition & 0 deletions dumpling/tests/views/data/views.v-schema-view.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
DROP TABLE IF EXISTS `v`;
DROP VIEW IF EXISTS `v`;
Expand Down
1 change: 1 addition & 0 deletions dumpling/tests/views/data/views.v-schema.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/*!40014 SET FOREIGN_KEY_CHECKS=0*/;
/*!40101 SET NAMES binary*/;
CREATE TABLE `v`(
`a` int,
Expand Down