*: differentiate types for user variables #18973

eurekaka · 2020-08-04T09:16:49Z

What problem does this PR solve?

Issue Number: close #16820 , close #11212

#16603 and #16362 can be closed by this PR as well.

Problem Summary:

We treated all user variables as string types, so there would be cast functions added which prevent the planner to choose better plans, such as PointGet. The problem is more frequent when plan cache is enabled.

What is changed and how it works?

What's Changed:

Infer types for user variables, if the user variable is not set before, we treat it as string type. The behaviors reference https://dev.mysql.com/doc/refman/8.0/en/user-variables.html.

Related changes

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

Check List

Tests

Unit test

Side effects

Performance regression
- Consumes more CPU

Release note

differentiate types for user variables

codecov · 2020-08-05T03:57:39Z

Codecov Report

Merging #18973 into master will decrease coverage by 0.0853%.
The diff coverage is 67.8304%.

@@               Coverage Diff                @@
##             master     #18973        +/-   ##
================================================
- Coverage   79.4390%   79.3537%   -0.0853%     
================================================
  Files           546        546                
  Lines        148631     148293       -338     
================================================
- Hits         118071     117676       -395     
- Misses        21075      21102        +27     
- Partials       9485       9515        +30

eurekaka · 2020-08-05T07:48:22Z

executor/prepared_test.go

-	tk.MustExec("set @v1 = 3")
-	tk.MustExec("set @v2 = 3")
+	tk.MustExec("set @v1 = '3'")
+	tk.MustExec("set @v2 = '3'")


A related bug: #19002. When the variable type and column type mismatches, point-get with plan-cache enabled would return wrong result. It should be fixed in another PR, so I modify the test cases here first to make the types consistent, since the test purpose here is not on the type mismatch problem.

expression/integration_test.go

winoros

lgtm

ghost · 2020-11-16T02:06:03Z

Aha, that's interesting. @SunRunAway @nullnotnil any thoughts?

LGTM

lzmhhh123 · 2020-11-16T08:14:19Z

/run-all-tests

lzmhhh123 · 2020-11-16T08:15:59Z

I think we can directly convert the user variable to the specified type in the plan cache during the evaluation. Since MySQL handled it in this way.

After the discussion, we decide to support different variable types in the plan cache. And c4bf6d0 (#18973) is to solve it.

lzmhhh123 · 2020-11-16T08:20:03Z

/run-sqllogic-test-2

lzmhhh123 · 2020-11-16T08:29:00Z

/run-all-tests

lzmhhh123 · 2020-11-16T08:36:27Z

/run-build

lzmhhh123 · 2020-11-16T09:21:16Z

/run-all-tests

lzmhhh123 · 2020-11-16T09:46:45Z

/run-unit-test

… user_variable

lzmhhh123 · 2020-11-16T11:00:24Z

/run-all-tests

lzmhhh123 · 2020-11-17T06:12:00Z

/run-check_dev_2

ti-challenge-bot · 2020-11-17T09:03:49Z

@eurekaka, Congratulations, you get 300 in this PR, and your total score is 300 in high-performance challenge program.

Details

Tip : None

Warning:
The pull request merged, eurekaka got the score. But it seems linked issue not picked.

cc: Mentor @lzmhhh123

ti-srebot · 2020-11-17T09:05:19Z

cherry pick to release-4.0 in PR #21107

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

Signed-off-by: ti-srebot <ti-srebot@pingcap.com> Co-authored-by: Kenan Yao <cauchy1992@gmail.com>

eurekaka added type/enhancement The issue or PR belongs to an enhancement. status/WIP sig/planner SIG: Planner component/expression sig/execution SIG execution labels Aug 4, 2020

eurekaka force-pushed the user_variable branch from 24b96b2 to 587d2a5 Compare August 4, 2020 09:38

github-actions bot added the component/executor label Aug 5, 2020

eurekaka force-pushed the user_variable branch from 587d2a5 to f5b9ee8 Compare August 5, 2020 03:45

eurekaka commented Aug 5, 2020

View reviewed changes

eurekaka force-pushed the user_variable branch from f5b9ee8 to e095dfd Compare August 5, 2020 10:00

eurekaka marked this pull request as ready for review August 5, 2020 10:00

eurekaka requested review from a team as code owners August 5, 2020 10:00

eurekaka requested review from qw4990 and winoros and removed request for a team August 5, 2020 10:00

eurekaka force-pushed the user_variable branch from e095dfd to 9ef334b Compare August 5, 2020 10:03

eurekaka removed the status/WIP label Aug 6, 2020

winoros reviewed Aug 6, 2020

View reviewed changes

expression/integration_test.go Show resolved Hide resolved

eurekaka requested a review from winoros August 11, 2020 03:54

github-actions bot added the sig/executor label Aug 11, 2020

AndreMouche removed the sig/executor label Aug 20, 2020

eurekaka force-pushed the user_variable branch 3 times, most recently from 340c3ab to 7298efd Compare September 18, 2020 07:44

winoros reviewed Sep 23, 2020

View reviewed changes

ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 23, 2020

eurekaka mentioned this pull request Sep 29, 2020

enhance range calculation upon user variables #8733

Open

lzmhhh123 added 2 commits November 16, 2020 15:42

plan cache support different variable types

c4bf6d0

fix fmt

b67bf45

Merge branch 'master' into user_variable

7ce45a5

lzmhhh123 added 2 commits November 16, 2020 18:59

fix unit test

2c9256d

Merge branch 'user_variable' of https://github.com/eurekaka/tidb into…

3013339

… user_variable

eurekaka added the status/all-tests-passed label Nov 17, 2020

Merge branch 'master' into user_variable

5b02aa8

qw4990 approved these changes Nov 17, 2020

View reviewed changes

eurekaka merged commit de75e60 into pingcap:master Nov 17, 2020

eurekaka deleted the user_variable branch November 17, 2020 09:03

ti-srebot mentioned this pull request Nov 17, 2020

*: differentiate types for user variables (#18973) #21107

Merged

asiafrank mentioned this pull request Nov 18, 2020

planner: fold the GetVar into a constant if the query contains no SetVar for the same user variable #20965

Closed

eurekaka added a commit to ti-srebot/tidb that referenced this pull request Nov 26, 2020

cherry pick pingcap#18973 to release-4.0

1a1b177

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

eurekaka added a commit that referenced this pull request Nov 26, 2020

cherry pick #18973 to release-4.0 (#21107)

9cc0ea7

Signed-off-by: ti-srebot <ti-srebot@pingcap.com> Co-authored-by: Kenan Yao <cauchy1992@gmail.com>

wjhuang2016 mentioned this pull request Dec 11, 2020

float type ？ #6882

Closed

time-and-fate mentioned this pull request Mar 10, 2021

planner: fix plan cache not working caused by type difference #23175

Merged

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: differentiate types for user variables #18973

*: differentiate types for user variables #18973

eurekaka commented Aug 4, 2020 •

edited by lzmhhh123

Loading

codecov bot commented Aug 5, 2020 •

edited

Loading

eurekaka Aug 5, 2020 •

edited

Loading

winoros left a comment

ghost commented Nov 16, 2020

lzmhhh123 commented Nov 16, 2020

lzmhhh123 commented Nov 16, 2020

lzmhhh123 commented Nov 16, 2020

lzmhhh123 commented Nov 16, 2020

lzmhhh123 commented Nov 16, 2020

lzmhhh123 commented Nov 16, 2020

lzmhhh123 commented Nov 16, 2020

lzmhhh123 commented Nov 16, 2020

lzmhhh123 commented Nov 17, 2020

ti-challenge-bot bot commented Nov 17, 2020

ti-srebot commented Nov 17, 2020

*: differentiate types for user variables #18973

*: differentiate types for user variables #18973

Conversation

eurekaka commented Aug 4, 2020 • edited by lzmhhh123 Loading

What problem does this PR solve?

What is changed and how it works?

Related changes

Check List

Release note

codecov bot commented Aug 5, 2020 • edited Loading

Codecov Report

eurekaka Aug 5, 2020 • edited Loading

Choose a reason for hiding this comment

winoros left a comment

Choose a reason for hiding this comment

ghost commented Nov 16, 2020

lzmhhh123 commented Nov 16, 2020

lzmhhh123 commented Nov 16, 2020

lzmhhh123 commented Nov 16, 2020

lzmhhh123 commented Nov 16, 2020

lzmhhh123 commented Nov 16, 2020

lzmhhh123 commented Nov 16, 2020

lzmhhh123 commented Nov 16, 2020

lzmhhh123 commented Nov 16, 2020

lzmhhh123 commented Nov 17, 2020

ti-challenge-bot bot commented Nov 17, 2020

ti-srebot commented Nov 17, 2020

eurekaka commented Aug 4, 2020 •

edited by lzmhhh123

Loading

codecov bot commented Aug 5, 2020 •

edited

Loading

eurekaka Aug 5, 2020 •

edited

Loading