Skip to content

util, types: don't let SPM be affected by charset#23161

Merged
XuHuaiyu merged 26 commits intopingcap:masterfrom
rebelice:dev_remove_charset
Mar 12, 2021
Merged

util, types: don't let SPM be affected by charset#23161
XuHuaiyu merged 26 commits intopingcap:masterfrom
rebelice:dev_remove_charset

Conversation

@rebelice
Copy link
Contributor

@rebelice rebelice commented Mar 8, 2021

What problem does this PR solve?

Issue Number: close #xxx

Different default charsets in different drivers(or versions) make SQL bind cannot work.
So we don't let SPM be affected by charset.

We add a flag RestoreStringWithoutCharset to control this.

Need to merge parser pr first

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test

Side effects

Release note

  • don't let SPM be affected by charset

@rebelice rebelice requested a review from a team as a code owner March 8, 2021 06:25
@rebelice rebelice requested review from XuHuaiyu and removed request for a team March 8, 2021 06:25
@ti-chi-bot ti-chi-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 8, 2021
@rebelice
Copy link
Contributor Author

rebelice commented Mar 8, 2021

/run-check_dev_2 parser=pr/1184

@rebelice
Copy link
Contributor Author

rebelice commented Mar 8, 2021

/rebuild parser=pr/1184

@rebelice
Copy link
Contributor Author

rebelice commented Mar 8, 2021

/cc @qw4990, @eurekaka

@rebelice
Copy link
Contributor Author

rebelice commented Mar 8, 2021

/sig planner

@ti-chi-bot ti-chi-bot added the sig/planner SIG: Planner label Mar 8, 2021
@rebelice
Copy link
Contributor Author

rebelice commented Mar 8, 2021

/cc @qw4990

@ti-chi-bot ti-chi-bot requested a review from qw4990 March 8, 2021 06:33
@rebelice
Copy link
Contributor Author

rebelice commented Mar 8, 2021

/cc @eurekaka

@ti-chi-bot ti-chi-bot requested a review from eurekaka March 8, 2021 06:33
Copy link
Contributor

@eurekaka eurekaka left a comment

Choose a reason for hiding this comment

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

Please add test for this PR.

@eurekaka eurekaka added priority/release-blocker This issue blocks a release. Please solve it ASAP. needs-cherry-pick-4.0 labels Mar 12, 2021
@rebelice rebelice requested a review from a team as a code owner March 12, 2021 07:02
@ti-chi-bot ti-chi-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 12, 2021
Copy link
Contributor

@eurekaka eurekaka left a comment

Choose a reason for hiding this comment

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

We need an upgrade function as well.

@ti-chi-bot
Copy link
Member

Merge canceled because a new commit is pushed.

@ti-chi-bot ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed status/can-merge Indicates a PR has been approved by a committer. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 12, 2021
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 12, 2021
@rebelice
Copy link
Contributor Author

/run-all-tests

@rebelice
Copy link
Contributor Author

/run-all-tests

1 similar comment
@rebelice
Copy link
Contributor Author

/run-all-tests

@rebelice
Copy link
Contributor Author

/run-all-tests

bindCase.bindText,
bindCase.db,
)
fmt.Println("[sql]", sql)
Copy link
Contributor

Choose a reason for hiding this comment

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

useless ?

upgradeToVer59,
upgradeToVer60,
upgradeToVer61,
// We will redo upgradeToVer61 in upgradeToVer6, it is skipped here.
Copy link
Contributor

Choose a reason for hiding this comment

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

upgradeToVer6 ?

@rebelice
Copy link
Contributor Author

/run-all-tests

@XuHuaiyu
Copy link
Contributor

/lgtm

@XuHuaiyu XuHuaiyu merged commit 2bea06e into pingcap:master Mar 12, 2021
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Mar 12, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #23295

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority/release-blocker This issue blocks a release. Please solve it ASAP. sig/planner SIG: Planner sig/sql-infra SIG: SQL Infra size/L Denotes a PR that changes 100-499 lines, ignoring generated files. status/LGT2 Indicates that a PR has LGTM 2.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants