Skip to content

Conversation

birdstorm
Copy link
Contributor

What have you changed? (mandatory)

cherry pick #7194 to release 2.0

What is the type of the changes? (mandatory)

  • Bug fix (non-breaking change which fixes an issue)

How has this PR been tested? (mandatory)

Does this PR affect documentation (docs/docs-cn) update? (mandatory)

Does this PR affect tidb-ansible update? (mandatory)

Does this PR need to be added to the release notes? (mandatory)

release note:
fix prefix index, when the charset is utf8 or utf8mb4, truncate it from runes rather than bytes.

Refer to a related PR or issue link (optional)

Benchmark result if necessary (optional)

Add a few positive/negative examples (optional)

@birdstorm birdstorm added type/bugfix This PR fixes a bug. sig/planner SIG: Planner type/2.0 cherry-pick labels Aug 1, 2018
@birdstorm birdstorm requested review from coocood and winoros August 1, 2018 10:04
@zz-jason
Copy link
Member

zz-jason commented Aug 1, 2018

/run-all-tests

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@winoros
Copy link
Member

winoros commented Aug 1, 2018

/run-all-tests tidb-test=release-2.0 tikv=release-2.0 pd=release-2.0

Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

lgtm

if ic.Length != types.UnspecifiedLength && utf8.RuneCount(val) > ic.Length {
rs := bytes.Runes(val)
colValue := v.GetBytes()
isUTF8Charset := colCharset == charset.CharsetUTF8 || colCharset == charset.CharsetUTF8MB4
Copy link
Member

Choose a reason for hiding this comment

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

Can we extract a function to types package, like

func TruncateValue(d *Datum, charset string, length int)

Copy link
Member

Choose a reason for hiding this comment

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

never mind, this is a cherry-pick, we can do it later.

@zimulala zimulala added the status/LGT2 Indicates that a PR has LGTM 2. label Aug 2, 2018
@coocood coocood merged commit 7ccecdd into pingcap:release-2.0 Aug 2, 2018
@birdstorm birdstorm deleted the cherry-pick/7194 branch August 2, 2018 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/planner SIG: Planner status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants