dumpling: add collation_compatible config in dumpling #31114

WizardXiao · 2021-12-29T06:11:57Z

What problem does this PR solve?

Issue Number: close #3420

Problem Summary:
See #3420.

What is changed and how it works?

add config collation_compatible in dumpling.
default loose handle create sql by original sql, will not add default collation as upstream.
strict will add default collation as upstream, and downstream will occur error when downstream don't support

Check List

Tests

Unit test

Documentation

Contains variable changes

Release note

add collation_compatible config in dumpling

ti-chi-bot · 2021-12-29T06:11:59Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

Ehco1996
lichunzhu

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

WizardXiao · 2021-12-29T06:12:55Z

/component dumpling

WizardXiao · 2021-12-29T06:16:07Z

/cc @lichunzhu @Ehco1996 @lance6716

sre-bot · 2021-12-29T06:26:54Z

Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/55f758e579a4c55aed92f0aa4d1e1439988057e1

lichunzhu

Is it possible to add a unit test/integration test for this method?

dumpling/export/config.go

WizardXiao · 2021-12-29T06:50:33Z

Maybe move collation_compatible judgement into adjustxxx will be easier to add ut. I will try. By the way, integration test will be added in tiflow repo together with increment.

lichunzhu

LGTM

Ehco1996 · 2021-12-29T07:56:28Z

dumpling/export/dump.go

@@ -443,7 +452,10 @@ func adjustDatabaseCollation(tctx *tcontext.Context, parser *parser.Parser, orig
 }

 // adjustTableCollation adjusts table collation
-func adjustTableCollation(tctx *tcontext.Context, parser *parser.Parser, originSQL string, charsetAndDefaultCollationMap map[string]string) (string, error) {
+func adjustTableCollation(tctx *tcontext.Context, collationCompatible string, parser *parser.Parser, originSQL string, charsetAndDefaultCollationMap map[string]string) (string, error) {
+	if collationCompatible != StrictCollationCompatible {


maybe do this check out of adjustTableCollation? not strong , current code lgtm too

yes, i do this like that before, but i find it is maybe hard to add UT for this config, because the judgement will be in dumpDatabases.

ok, maybe we can enhance this later, because codefreeze is coming, i will merge this pr first

WizardXiao · 2021-12-29T08:04:27Z

/merge

ti-chi-bot · 2021-12-29T08:04:30Z

@WizardXiao: /merge is only allowed for the committers, you can assign this pull request to the committer in list by filling /assign @committer in the comment to help merge this pull request.

In response to this:

/merge

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

WizardXiao · 2021-12-29T08:06:25Z

Please help merge this pr, if there is no other conversation. thank you. @lichunzhu

Ehco1996 · 2021-12-29T08:18:53Z

/run-integration-dumpling-test

Ehco1996 · 2021-12-29T08:22:46Z

Please help merge this pr, if there is no other conversation. thank you. @lichunzhu

will merge this pr after dumpling it test passed

Ehco1996 · 2021-12-29T08:32:50Z

/merge

ti-chi-bot · 2021-12-29T08:32:54Z

This pull request has been accepted and is ready to merge.

Commit hash: e3ef220

WizardXiao · 2021-12-29T09:52:02Z

/run-check_dev_2

WizardXiao · 2021-12-29T10:13:55Z

/run-unit-test

WizardXiao · 2021-12-29T10:30:37Z

/run-check_dev_2

ti-chi-bot · 2021-12-29T10:38:20Z

[FORMAT CHECKER NOTIFICATION]

Notice: Please provide the linked issue number on one line in the PR body, for example: Issue Number: close #123 or Issue Number: ref #456.

_{📖 For more info, you can check the "Contribute Code" section in the development guide.}

Signed-off-by: ti-srebot <[email protected]>

ti-srebot · 2022-01-18T10:14:55Z

cherry pick to release-5.3 in PR #31777

commit-message: add collation_compatible config in dumpling

e171d80

ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 29, 2021

ti-chi-bot added the component/dumpling This is related to Dumpling of TiDB. label Dec 29, 2021

ti-chi-bot requested review from Ehco1996, lance6716 and lichunzhu December 29, 2021 06:16

WizardXiao changed the title ~~commit-message: add collation_compatible config in dumpling~~ dumpling: add collation_compatible config in dumpling Dec 29, 2021

lichunzhu reviewed Dec 29, 2021

View reviewed changes

dumpling/export/config.go Outdated Show resolved Hide resolved

dumpling/export/config.go Outdated Show resolved Hide resolved

WizardXiao added 3 commits December 29, 2021 15:04

commit-message: fix name and some ut

e25000d

commit-message: add issue number

000a8df

commit-message: delete useless annotation

9e33475

lichunzhu approved these changes Dec 29, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 29, 2021

Ehco1996 approved these changes Dec 29, 2021

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 29, 2021

Merge branch 'master' into add-conf-collation_compatible-dumpling

e3ef220

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 29, 2021

ti-chi-bot added 3 commits December 29, 2021 16:33

Merge branch 'master' into add-conf-collation_compatible-dumpling

7b498b0

Merge branch 'master' into add-conf-collation_compatible-dumpling

02212d6

Merge branch 'master' into add-conf-collation_compatible-dumpling

1558617

Merge branch 'master' into add-conf-collation_compatible-dumpling

d770619

Merge branch 'master' into add-conf-collation_compatible-dumpling

43b81a8

Merge branch 'master' into add-conf-collation_compatible-dumpling

55f758e

ti-chi-bot merged commit 1e7f0dc into pingcap:master Dec 29, 2021

lance6716 added the needs-cherry-pick-release-5.3 Type: Need cherry pick to release-5.3 label Jan 18, 2022

ti-srebot mentioned this pull request Jan 18, 2022

dumpling: add collation_compatible config in dumpling (#31114) #31777

Closed

2 tasks

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Jan 18, 2022

cherry pick pingcap#31114 to release-5.3

da1a8ee

Signed-off-by: ti-srebot <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dumpling: add collation_compatible config in dumpling #31114

dumpling: add collation_compatible config in dumpling #31114

WizardXiao commented Dec 29, 2021 •

edited

Loading

ti-chi-bot commented Dec 29, 2021 •

edited

Loading

WizardXiao commented Dec 29, 2021

WizardXiao commented Dec 29, 2021

sre-bot commented Dec 29, 2021 •

edited

Loading

lichunzhu left a comment

WizardXiao commented Dec 29, 2021 •

edited

Loading

lichunzhu left a comment

Ehco1996 Dec 29, 2021

WizardXiao Dec 29, 2021

Ehco1996 Dec 29, 2021

WizardXiao commented Dec 29, 2021

ti-chi-bot commented Dec 29, 2021

WizardXiao commented Dec 29, 2021

Ehco1996 commented Dec 29, 2021

Ehco1996 commented Dec 29, 2021 •

edited

Loading

Ehco1996 commented Dec 29, 2021

ti-chi-bot commented Dec 29, 2021

WizardXiao commented Dec 29, 2021

WizardXiao commented Dec 29, 2021

WizardXiao commented Dec 29, 2021

ti-chi-bot commented Dec 29, 2021

ti-srebot commented Jan 18, 2022

dumpling: add collation_compatible config in dumpling #31114

dumpling: add collation_compatible config in dumpling #31114

Conversation

WizardXiao commented Dec 29, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Dec 29, 2021 • edited Loading

WizardXiao commented Dec 29, 2021

WizardXiao commented Dec 29, 2021

sre-bot commented Dec 29, 2021 • edited Loading

lichunzhu left a comment

Choose a reason for hiding this comment

WizardXiao commented Dec 29, 2021 • edited Loading

lichunzhu left a comment

Choose a reason for hiding this comment

Ehco1996 Dec 29, 2021

Choose a reason for hiding this comment

WizardXiao Dec 29, 2021

Choose a reason for hiding this comment

Ehco1996 Dec 29, 2021

Choose a reason for hiding this comment

WizardXiao commented Dec 29, 2021

ti-chi-bot commented Dec 29, 2021

WizardXiao commented Dec 29, 2021

Ehco1996 commented Dec 29, 2021

Ehco1996 commented Dec 29, 2021 • edited Loading

Ehco1996 commented Dec 29, 2021

ti-chi-bot commented Dec 29, 2021

WizardXiao commented Dec 29, 2021

WizardXiao commented Dec 29, 2021

WizardXiao commented Dec 29, 2021

ti-chi-bot commented Dec 29, 2021

ti-srebot commented Jan 18, 2022

WizardXiao commented Dec 29, 2021 •

edited

Loading

ti-chi-bot commented Dec 29, 2021 •

edited

Loading

sre-bot commented Dec 29, 2021 •

edited

Loading

WizardXiao commented Dec 29, 2021 •

edited

Loading

Ehco1996 commented Dec 29, 2021 •

edited

Loading