Skip to content

collation: fix wrong compare result for unicode collate#18785

Merged
ti-chi-bot[bot] merged 2 commits intotikv:masterfrom
Defined2014:fix-18784
Aug 5, 2025
Merged

collation: fix wrong compare result for unicode collate#18785
ti-chi-bot[bot] merged 2 commits intotikv:masterfrom
Defined2014:fix-18784

Conversation

@Defined2014
Copy link
Contributor

@Defined2014 Defined2014 commented Aug 4, 2025 •
edited
Loading

What is changed and how it works?

Issue Number: Close #18784

What's Changed:

Follow tidb code, see https://github.com/pingcap/tidb/blob/2a522358ceb60095eea898503682595dd3612492/pkg/util/collate/ucaimpl/unicode_ci.go.tpl#L50 to get more informations.

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Release note

None

@ti-chi-bot ti-chi-bot bot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. contribution This PR is from a community contributor. labels Aug 4, 2025
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Aug 4, 2025

Hi @Defined2014. Thanks for your PR.

I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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 kubernetes-sigs/prow repository.

@ti-chi-bot ti-chi-bot bot added dco-signoff: no Indicates the PR's author has not signed dco. needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 4, 2025
@ti-chi-bot ti-chi-bot bot added dco-signoff: yes Indicates the PR's author has signed the dco. release-note-none Denotes a PR that doesn't merit a release note. and removed dco-signoff: no Indicates the PR's author has not signed dco. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Aug 4, 2025
Defined2014
Defined2014 commented Aug 4, 2025
View reviewed changes
components/tidb_query_datatype/src/codec/collation/collator/utf8mb4_uca/mod.rs
let mut bn = 0;

while !sa.is_empty() || !sb.is_empty() {
loop {
Copy link
Contributor Author

@Defined2014 Defined2014 Aug 4, 2025

Choose a reason for hiding this comment

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

Follow https://github.com/pingcap/tidb/blob/2a522358ceb60095eea898503682595dd3612492/pkg/util/collate/ucaimpl/unicode_ci.go.tpl#L50

@Defined2014
Copy link
Contributor Author

Defined2014 commented Aug 4, 2025

/cc @breezewish @wjhuang2016

@ti-chi-bot ti-chi-bot bot requested review from breezewish and wjhuang2016 August 4, 2025 13:22
@Defined2014 Defined2014 mentioned this pull request Aug 4, 2025
Closed
@ti-chi-bot ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Aug 5, 2025
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Aug 5, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: breezewish, wjhuang2016

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Aug 5, 2025
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Aug 5, 2025

[LGTM Timeline notifier]

Timeline:

  • 2025-08-05 02:54:33.792243202 +0000 UTC m=+323183.935004367: ☑️ agreed by breezewish.
  • 2025-08-05 03:35:04.598813039 +0000 UTC m=+325614.741574204: ☑️ agreed by wjhuang2016.

@Defined2014
Copy link
Contributor Author

Defined2014 commented Aug 5, 2025

/retest

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Aug 5, 2025

@Defined2014: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

Details

In response to this:

/retest

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 kubernetes-sigs/prow repository.

@Defined2014
Copy link
Contributor Author

Defined2014 commented Aug 5, 2025

/test pull-unit-test

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Aug 5, 2025

@Defined2014: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

Details

In response to this:

/test pull-unit-test

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 kubernetes-sigs/prow repository.

@Defined2014
collation: fix wrong compare result for unicode collate
352b3d0 
Signed-off-by: mohangjie <mohangjie1995@gmail.com>
@Defined2014
update
9ddbda7 
Signed-off-by: mohangjie <mohangjie1995@gmail.com>

update

Signed-off-by: mohangjie <mohangjie1995@gmail.com>
@ti-chi-bot ti-chi-bot bot merged commit 8151fe5 into tikv:master Aug 5, 2025
8 checks passed
@ti-chi-bot ti-chi-bot bot added this to the Pool milestone Aug 5, 2025
@Defined2014
Copy link
Contributor Author

Defined2014 commented Aug 5, 2025

/label needs-cherry-pick-release-7.5
/label needs-cherry-pick-release-8.5

@ti-chi-bot ti-chi-bot bot added needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. labels Aug 5, 2025
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Aug 5, 2025

In response to a cherrypick label: new pull request created to branch release-7.5: #18790.

@ti-chi-bot ti-chi-bot mentioned this pull request Aug 5, 2025
Merged
9 tasks
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Aug 5, 2025

In response to a cherrypick label: new pull request created to branch release-8.5: #18791.

@ti-chi-bot ti-chi-bot mentioned this pull request Aug 5, 2025
Merged
9 tasks
ti-chi-bot bot pushed a commit that referenced this pull request Aug 8, 2025
@ti-chi-bot @Defined2014
collation: fix wrong compare result for unicode collate (#18785) (#18790
efe98e6 
)

close #18784

Follow tidb code, see https://github.com/pingcap/tidb/blob/2a522358ceb60095eea898503682595dd3612492/pkg/util/collate/ucaimpl/unicode_ci.go.tpl#L50 to get more informations.

Signed-off-by: mohangjie <mohangjie1995@gmail.com>

Co-authored-by: mohangjie <mohangjie1995@gmail.com>
ti-chi-bot bot pushed a commit that referenced this pull request Aug 8, 2025
@ti-chi-bot @Defined2014
collation: fix wrong compare result for unicode collate (#18785) (#18791
13b9af5 
)

close #18784

Follow tidb code, see https://github.com/pingcap/tidb/blob/2a522358ceb60095eea898503682595dd3612492/pkg/util/collate/ucaimpl/unicode_ci.go.tpl#L50 to get more informations.

Signed-off-by: mohangjie <mohangjie1995@gmail.com>

Co-authored-by: mohangjie <mohangjie1995@gmail.com>
@Defined2014 Defined2014 deleted the fix-18784 branch August 8, 2025 05:24
yibin87 pushed a commit to yibin87/tikv that referenced this pull request Aug 11, 2025
@Defined2014 @yibin87
collation: fix wrong compare result for unicode collate (tikv#18785)
bedb1f1 
close tikv#18784

Follow tidb code, see https://github.com/pingcap/tidb/blob/2a522358ceb60095eea898503682595dd3612492/pkg/util/collate/ucaimpl/unicode_ci.go.tpl#L50 to get more informations.

Signed-off-by: mohangjie <mohangjie1995@gmail.com>
3AceShowHand pushed a commit to 3AceShowHand/tikv that referenced this pull request Oct 13, 2025
@Defined2014 @3AceShowHand
collation: fix wrong compare result for unicode collate (tikv#18785)
5405624 
close tikv#18784

Follow tidb code, see https://github.com/pingcap/tidb/blob/2a522358ceb60095eea898503682595dd3612492/pkg/util/collate/ucaimpl/unicode_ci.go.tpl#L50 to get more informations.

Signed-off-by: mohangjie <mohangjie1995@gmail.com>
Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@breezewish breezewish breezewish approved these changes

+1 more reviewer

@wjhuang2016 wjhuang2016 wjhuang2016 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved contribution This PR is from a community contributor. dco-signoff: yes Indicates the PR's author has signed the dco. lgtm needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

Pool

Development

Successfully merging this pull request may close these issues.

Result wrong with utf8mb4_unicode_ci

4 participants

@Defined2014 @ti-chi-bot @breezewish @wjhuang2016