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.

Sign up for GitHub

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

Jump to bottom

expression: support GBK for builtin function Decode and Encode #29315

Merged
merged 25 commits into from
Nov 3, 2021
Merged

expression: support GBK for builtin function Decode and Encode #29315

merged 25 commits into from
Nov 3, 2021

Conversation

hawkingrei
Copy link
Member

@hawkingrei hawkingrei commented Nov 1, 2021 •
edited
Loading

What problem does this PR solve?

Issue Number:

close #29314

close #29169

Problem Summary:

expression: support GBK for builtin function Decode and Encode

Check List

Tests

  • Unit test
  • Integration test

Release note

None

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Nov 1, 2021 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • AilinKid
  • xiongjiwei

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.

@ti-chi-bot ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 1, 2021
@hawkingrei
Copy link
Member Author

/wip

@hawkingrei hawkingrei changed the title expression: support GBK for builtin function Decode and Encode [WIP] expression: support GBK for builtin function Decode and Encode Nov 1, 2021
@ti-chi-bot ti-chi-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 1, 2021
@hawkingrei
expression: support GBK for builtin function Decode and Encode
4be11b3 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@hawkingrei
expression: support GBK for builtin function Decode and encode
143bedd 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@hawkingrei
expression: support GBK for builtin function Decode and encode
23478d8 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 2, 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 Nov 2, 2021
@hawkingrei
update
8249213 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@hawkingrei hawkingrei changed the title [WIP] expression: support GBK for builtin function Decode and Encode expression: support GBK for builtin function Decode and Encode Nov 2, 2021
@ti-chi-bot ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 2, 2021
@hawkingrei
Copy link
Member Author

/run-check_dev_2

@hawkingrei
expression: support GBK for builtin function Decode and Encode
48ecc6a 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@hawkingrei
expression: support GBK for builtin function Decode and encode
034b79a 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@hawkingrei
expression: support GBK for builtin function Decode and encode
461a992 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
zimulala
zimulala reviewed Nov 3, 2021
View reviewed changes
cmd/explaintest/t/new_character_set_builtin.test Outdated Show resolved Hide resolved
expression/builtin_encryption_vec.go Outdated Show resolved Hide resolved
expression/builtin_encryption.go Outdated Show resolved Hide resolved
hawkingrei and others added 5 commits November 3, 2021 07:25
@hawkingrei
expression: support GBK for builtin function Decode and Encode
27be1aa 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@hawkingrei
expression: support GBK for builtin function Decode and encode
d761d25 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@hawkingrei
expression: support GBK for builtin function Decode and encode
915fd5c 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@hawkingrei
expression: support GBK for builtin function Decode and encode
945cfa0 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@AilinKid
Merge branch 'master' into add_gbk_for_decode
b8ee640
@AilinKid
Copy link
Contributor

AilinKid commented Nov 3, 2021

image
this file is a binary file, is it right?

@hawkingrei
Copy link
Member Author

hawkingrei commented Nov 3, 2021 •
edited
Loading

image this file is a binary file, is it right?

it is fixed.

@hawkingrei
expression: support GBK for builtin function Decode and encode
e981114 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@hawkingrei
Merge branch 'add_gbk_for_decode' of github.com:hawkingrei/tidb into …
c56fc78 
…add_gbk_for_decode
@hawkingrei
Merge branch 'master' of https://github.com/pingcap/tidb into add_gbk…
2885640 
…_for_decode
@hawkingrei
Merge branch 'add_gbk_for_decode' of github.com:hawkingrei/tidb into …
9db095b 
…add_gbk_for_decode
@hawkingrei
expression: support GBK for builtin function Decode and encode
2ec28d9 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@hawkingrei
Merge branch 'add_gbk_for_decode' of github.com:hawkingrei/tidb into …
4967cd7 
…add_gbk_for_decode
@hawkingrei
expression: support GBK for builtin function Decode and encode
f6b5899 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@hawkingrei
Merge branch 'add_gbk_for_decode' of github.com:hawkingrei/tidb into …
e09a734 
…add_gbk_for_decode
@hawkingrei
expression: support GBK for builtin function Decode and Encode
ce4c9d0 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
AilinKid
AilinKid approved these changes Nov 3, 2021
View reviewed changes
Copy link
Contributor

@AilinKid AilinKid left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 3, 2021
xiongjiwei
xiongjiwei reviewed Nov 3, 2021
View reviewed changes
cmd/explaintest/r/new_character_set_builtin.result
drop table if exists t;
create table t (a char(20) charset utf8mb4, b char(20) charset gbk, c binary(20));
insert into t values ('一二三', '一二三', '一二三');
select decode(encode(a,"monty"),"monty") = a, md5(decode(encode(b,"monty"),"monty")) = md5(b), decode(encode(c,"monty"),"monty") = c from t;
Copy link
Contributor

Choose a reason for hiding this comment

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

I doubt it can test the encode and decode function with gbk charset. Suppose we do nothing with these two functions, they are still equal.

Copy link
Member Author

Choose a reason for hiding this comment

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

The same here is fine, the reason why the above one needs md5 is that there is no way to do the = calculation here. Otherwise, an error will be reported.

(1267, "Illegal mix of collations (utf8_general_ci,COERCIBLE) and (gbk_chinese_ci,IMPLICIT) for operation '='")

@ti-chi-bot 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 Nov 3, 2021
@xiongjiwei
Copy link
Contributor

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: f5af85f

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 3, 2021
@ti-chi-bot ti-chi-bot merged commit 9d9915b into pingcap:master Nov 3, 2021
@hawkingrei hawkingrei deleted the add_gbk_for_decode branch November 3, 2021 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zimulala zimulala zimulala left review comments

@Defined2014 Defined2014 Defined2014 left review comments

@AilinKid AilinKid AilinKid approved these changes

@xiongjiwei xiongjiwei xiongjiwei approved these changes

Assignees
No one assigned
Labels
release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support GBK for builtin function Encode Support GBK for builtin function Decode
6 participants
@hawkingrei @ti-chi-bot @AilinKid @xiongjiwei @zimulala @Defined2014