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.

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

expression: fix enum and set type expression in where clause #22785

Merged
merged 5 commits into from
Feb 19, 2021

Conversation

lzmhhh123
Copy link
Contributor

Signed-off-by: lzmhhh123 lzmhhh123@gmail.com

What problem does this PR solve?

Issue Number: close #22717

What is changed and how it works?

How it Works:

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test

Release note

  • fix enum and set type expression in where clause

Signed-off-by: lzmhhh123 <lzmhhh123@gmail.com>
@lzmhhh123 lzmhhh123 requested a review from a team as a code owner February 18, 2021 07:23
@lzmhhh123 lzmhhh123 requested review from qw4990 and removed request for a team February 18, 2021 07:23
Signed-off-by: lzmhhh123 <lzmhhh123@gmail.com>
@lzmhhh123
Copy link
Contributor Author

/build

@@ -459,7 +459,8 @@ func toBool(sc *stmtctx.StatementContext, tp *types.FieldType, eType types.EvalT
if tp.Hybrid() {
switch tp.Tp {
case mysql.TypeEnum, mysql.TypeSet:
fVal = float64(len(sVal))
// Enum and Set type value to bool is always true.
fVal = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reference in the mysql manual?

Copy link
Contributor

Choose a reason for hiding this comment

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

mysql> create table t(a enum("a", "b", "c"));
Query OK, 0 rows affected (0.02 sec)

mysql> insert into t values('a'),('b');
Query OK, 2 rows affected (0.00 sec)
Records: 2  Duplicates: 0  Warnings: 0

mysql> set @@sql_mode='';
Query OK, 0 rows affected, 1 warning (0.00 sec)

mysql> insert into t values("");
Query OK, 1 row affected, 1 warning (0.00 sec)

mysql> select * from t;
+------+
| a    |
+------+
| a    |
| b    |
|      |
+------+
3 rows in set (0.00 sec)

mysql> select * from t where a;
+------+
| a    |
+------+
| a    |
| b    |
+------+
2 rows in set (0.00 sec)

@ichn-hu ichn-hu mentioned this pull request Feb 18, 2021
Signed-off-by: lzmhhh123 <lzmhhh123@gmail.com>
fVal = float64(len(sVal))
if fVal == 0 {
// Check whether "" in set type.
Copy link
Contributor

Choose a reason for hiding this comment

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

change this comment to:
The elements listed in the column specification are assigned index numbers, beginning with 1. The index value of the empty string error value (distinguish from a “normal” empty string) is 0. Thus we need to check whether it's an empty string error value when `fVal==0`

Signed-off-by: lzmhhh123 <lzmhhh123@gmail.com>
Copy link
Contributor

@XuHuaiyu XuHuaiyu 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-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 19, 2021
Copy link
Contributor

@ichn-hu ichn-hu 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-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Feb 19, 2021
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Feb 19, 2021
@lzmhhh123
Copy link
Contributor Author

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 19, 2021
@ti-srebot
Copy link
Contributor

Your auto merge job has been accepted, waiting for:

  • 22790

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 8c2db1b into pingcap:master Feb 19, 2021
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Feb 19, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #22814

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Feb 19, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-5.0-rc in PR #22815

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/expression sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bug-fix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong enum calculation result in where condition.
4 participants