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

Fix prefix length is larger than the value used. #668

Merged
merged 4 commits into from
Apr 19, 2019
Merged

Fix prefix length is larger than the value used. #668

merged 4 commits into from
Apr 19, 2019

Conversation

zhexuany
Copy link
Contributor

With the following schema:

 CREATE TABLE `idx` (
  `c_id` text DEFAULT NULL,
  KEY `c_id` (`c_id`(10))
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin

the query select * from idx where c_id == 'dairy' will fail. This pr fixes this.

@zhexuany zhexuany requested a review from birdstorm April 19, 2019 06:25
ilovesoup
ilovesoup reviewed Apr 19, 2019
View reviewed changes
tikv-client/src/main/java/com/pingcap/tikv/types/Converter.java Outdated
} else if (val instanceof String) {
return ((String) val).substring(0, prefixLength).getBytes(StandardCharsets.UTF_8);
return ((String) val).substring(0, Math.min(((String) val).length(), prefixLength))
Copy link
Contributor

Choose a reason for hiding this comment

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

String valStr = (String)val;
valStr.substring(0, Math.min(valStr.length(), prefixLength))
Make it clean.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

ilovesoup
ilovesoup approved these changes Apr 19, 2019
View reviewed changes
Copy link
Contributor

@ilovesoup ilovesoup left a comment

Choose a reason for hiding this comment

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

LGTM as long as comment addressed.

birdstorm
birdstorm reviewed Apr 19, 2019
View reviewed changes
core/src/test/scala/org/apache/spark/sql/expression/index/PrefixIndexTestSuite.scala Outdated
@@ -57,6 +57,7 @@ class PrefixIndexTestSuite extends BaseTiSparkSuite {
explainAndRunTest("select a, b from prefix where b LIKE 'ÿ%'", skipJDBC = true)
explainAndRunTest("select a, b from prefix where b LIKE '%b'")
explainAndRunTest("select a, b from prefix where b LIKE '%'")
explainAndRunTest("select * from prefix where b = 'b'")
Copy link
Contributor

Choose a reason for hiding this comment

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

I purpose to move this line to Line 38 and add issue link.

birdstorm
birdstorm approved these changes Apr 19, 2019
View reviewed changes
Copy link
Contributor

@birdstorm birdstorm left a comment

Choose a reason for hiding this comment

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

LGTM as long as comments resolved.

@zhexuany zhexuany merged commit 85e67c8 into pingcap:master Apr 19, 2019
@zhexuany zhexuany deleted the fix_prefix branch April 19, 2019 06:58
wfxxh pushed a commit to wanfangdata/tispark that referenced this pull request Jun 30, 2023
@zhexuany
Fix prefix length is larger than the value used. (pingcap#668)
a8a7cbe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@birdstorm birdstorm birdstorm approved these changes

@ilovesoup ilovesoup ilovesoup approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zhexuany @ilovesoup @birdstorm