Skip to content

expression: fix incorrect result of logical operators#12173

Merged
sre-bot merged 3 commits intopingcap:masterfrom
sduzh:issue-11199
Oct 14, 2019
Merged

expression: fix incorrect result of logical operators#12173
sre-bot merged 3 commits intopingcap:masterfrom
sduzh:issue-11199

Conversation

@sduzh
Copy link
Contributor

@sduzh sduzh commented Sep 12, 2019 •
edited by SunRunAway
Loading

What problem does this PR solve?

Fix #11199
Fix #11188

TiDB> SELECT NULL OR 0.00000001000000000000;
+--------------------------------+
| NULL OR 0.00000001000000000000 |
+--------------------------------+
|                              1 |
+--------------------------------+
1 row in set (0.00 sec)

TiDB> SELECT 0 OR 0.00000001000000000000;
+-----------------------------+
| 0 OR 0.00000001000000000000 |
+-----------------------------+
|                           1 |
+-----------------------------+
1 row in set (0.00 sec)

TiDB> SELECT 0 OR "0.01a";
+--------------+
| 0 OR "0.01a" |
+--------------+
|            1 |
+--------------+
1 row in set, 1 warning (0.00 sec)

What is changed and how it works?

Wrap the input parameters of the builtin AND、OR and XOR operators with IS TRUE function before cast them to int type.

Check List

Tests

  • Unit test

Code changes

Side effects

Related changes

Release note

  • fix incorrect result of logical operators (OR, AND)

@sre-bot sre-bot added the contribution This PR is from a community contributor. label Sep 12, 2019
@codecov
Copy link

codecov bot commented Sep 12, 2019 •
edited
Loading

Codecov Report

Merging #12173 into master will decrease coverage by 0.0466%.
The diff coverage is n/a.

@@               Coverage Diff                @@
##             master     #12173        +/-   ##
================================================
- Coverage   79.9183%   79.8717%   -0.0467%     
================================================
  Files           462        462                
  Lines        104892     105101       +209     
================================================
+ Hits          83828      83946       +118     
- Misses        14910      14990        +80     
- Partials       6154       6165        +11

@zz-jason
Copy link
Member

zz-jason commented Sep 17, 2019

@sduzh should we directly modify builtinLogicOrSig to return 1 when the two input parameters are null and 1?

@sduzh
Copy link
Contributor Author

sduzh commented Sep 18, 2019

@sduzh should we directly modify builtinLogicOrSig to return 1 when the two input parameters are null and 1?

@zz-jason Sorry, I didn't understand what you mean. Isn't this the current implementation?

qw4990
qw4990 reviewed Sep 27, 2019
View reviewed changes
SunRunAway
SunRunAway reviewed Sep 30, 2019
View reviewed changes
SunRunAway
SunRunAway reviewed Sep 30, 2019
View reviewed changes
Copy link
Contributor

@SunRunAway SunRunAway left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hi, your PR looks really awesome. It would be better to take another look at other logical functions (https://dev.mysql.com/doc/refman/8.0/en/logical-operators.html), and fix them together.
Thanks again.

@sduzh sduzh closed this Oct 1, 2019
@sduzh sduzh reopened this Oct 1, 2019
@CLAassistant
Copy link

CLAassistant commented Oct 1, 2019 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@sduzh sduzh changed the title expression: fix incorrect OR result (#11199) expression: fix incorrect result of logical operators (#11199) Oct 1, 2019
SunRunAway
SunRunAway reviewed Oct 9, 2019
View reviewed changes
Copy link
Contributor

@SunRunAway SunRunAway left a comment

Choose a reason for hiding this comment

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

LGTM

@XuHuaiyu XuHuaiyu changed the title expression: fix incorrect result of logical operators (#11199) expression: fix incorrect result of logical operators Oct 9, 2019
XuHuaiyu
XuHuaiyu previously approved these changes Oct 9, 2019
View reviewed changes
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

@zz-jason zz-jason added status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. labels Oct 9, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Oct 9, 2019

Your auto merge job has been accepted, waiting for 12558, 12564, 12551, 12552, 12553

@qw4990 qw4990 removed their request for review October 9, 2019 08:48
@zz-jason zz-jason requested a review from Reminiscent as a code owner October 9, 2019 08:49
@sre-bot
Copy link
Contributor

sre-bot commented Oct 9, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Oct 9, 2019

@sduzh merge failed.

@sduzh sduzh deleted the issue-11199 branch October 14, 2019 13:37
This was referenced Oct 18, 2019
Merged
Merged
Merged
Closed
SunRunAway added a commit to SunRunAway/tidb that referenced this pull request Oct 18, 2019
@SunRunAway
Merge branch 'release-3.0' into automated-cherry-pick-of-pingcap#12173-…
f2cb598 
…upstream-release-3.0
zimulala added a commit to SunRunAway/tidb that referenced this pull request Oct 18, 2019
@zimulala
Merge branch 'release-3.0' into automated-cherry-pick-of-pingcap#12173-…
328cf89 
…upstream-release-3.0
SunRunAway added a commit to SunRunAway/tidb that referenced this pull request Oct 18, 2019
@SunRunAway
Merge branch 'release-3.1' into automated-cherry-pick-of-pingcap#12173-…
7a96df6 
…upstream-release-3.1
eurekaka added a commit to SunRunAway/tidb that referenced this pull request Oct 18, 2019
@eurekaka
Merge branch 'release-3.0' into automated-cherry-pick-of-pingcap#12173-…
e6960cd 
…upstream-release-3.0
eurekaka added a commit to SunRunAway/tidb that referenced this pull request Oct 18, 2019
@eurekaka
Merge branch 'release-3.0' into automated-cherry-pick-of-pingcap#12173-…
3609ea0 
…upstream-release-3.0
eurekaka pushed a commit that referenced this pull request Oct 18, 2019
@SunRunAway @eurekaka
expression: fix incorrect result of logical operators (#12173) (#12811)
93f8151
eurekaka added a commit to SunRunAway/tidb that referenced this pull request Oct 18, 2019
@eurekaka
Merge branch 'release-2.1' into automated-cherry-pick-of-pingcap#12173-…
3b47d9c 
…upstream-release-2.1
eurekaka added a commit to SunRunAway/tidb that referenced this pull request Oct 18, 2019
@eurekaka
Merge branch 'release-3.1' into automated-cherry-pick-of-pingcap#12173-…
86f3832 
…upstream-release-3.1
sre-bot pushed a commit that referenced this pull request Oct 18, 2019
@SunRunAway @sre-bot
expression: fix incorrect result of logical operators (#12173) (#12813)
e848851
sre-bot pushed a commit that referenced this pull request Oct 18, 2019
@SunRunAway @sre-bot
expression: fix incorrect result of logical operators (#12173) (#12810)
7c69ca4
@qw4990 qw4990 mentioned this pull request Nov 1, 2019
Closed
@cyliu0 cyliu0 mentioned this pull request Nov 6, 2019
Closed
XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019
@XiaTianliang
expression: fix incorrect result of logical operators (pingcap#12173)
93f0950
@SunRunAway SunRunAway mentioned this pull request Mar 13, 2020
Closed
@Reminiscent Reminiscent mentioned this pull request Mar 31, 2020
Merged
@sre-bot sre-bot mentioned this pull request Mar 31, 2020
Merged
@Reminiscent Reminiscent mentioned this pull request Mar 31, 2020
Closed
@XuHuaiyu XuHuaiyu mentioned this pull request Apr 16, 2020
Merged
@sre-bot sre-bot mentioned this pull request Apr 16, 2020
Merged
@Reminiscent Reminiscent mentioned this pull request Apr 17, 2020
Merged
This was referenced Apr 20, 2020
Closed
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qw4990 qw4990 qw4990 left review comments

@zz-jason zz-jason zz-jason approved these changes

@SunRunAway SunRunAway Awaiting requested review from SunRunAway

@XuHuaiyu XuHuaiyu Awaiting requested review from XuHuaiyu

Assignees

No one assigned

Labels

component/expression contribution This PR is from a community contributor. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Result of function OR is not correct. Function NOT/IF returns incorrect result with decimal argument

8 participants

@sduzh @zz-jason @CLAassistant @sre-bot @SunRunAway @qw4990 @XuHuaiyu @francis0407