Skip to content

planner: fix wrong DATE/DATETIME comparison in BETWEEN function#10313

Merged
qw4990 merged 6 commits intopingcap:masterfrom
erjiaqing:issue_9764
May 6, 2019
Merged

planner: fix wrong DATE/DATETIME comparison in BETWEEN function#10313
qw4990 merged 6 commits intopingcap:masterfrom
erjiaqing:issue_9764

Conversation

@erjiaqing
Copy link
Copy Markdown
Contributor

@erjiaqing erjiaqing commented Apr 29, 2019

What problem does this PR solve?

Fix #9764

What is changed and how it works?

Add converts in betweenToExpression when GetCmpTp4MinMax returns DateTime type. The behavior should be same as MySQL 8.0 now.

In mysql, If all values are string-like (string & datetime) and contains datetime, then values are converted to datetime.

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 29, 2019

Codecov Report

Merging #10313 into master will increase coverage by 0.017%.
The diff coverage is 100%.

@@              Coverage Diff               @@
##             master    #10313       +/-   ##
==============================================
+ Coverage   77.6699%   77.687%   +0.017%     
==============================================
  Files           411       411               
  Lines         85441     85457       +16     
==============================================
+ Hits          66362     66389       +27     
+ Misses        14117     14112        -5     
+ Partials       4962      4956        -6

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 29, 2019

Codecov Report

Merging #10313 into master will increase coverage by 0.0071%.
The diff coverage is 100%.

@@               Coverage Diff                @@
##             master     #10313        +/-   ##
================================================
+ Coverage   77.6576%   77.6648%   +0.0071%     
================================================
  Files           411        411                
  Lines         85434      85466        +32     
================================================
+ Hits          66346      66377        +31     
+ Misses        14127      14125         -2     
- Partials       4961       4964         +3

@erjiaqing erjiaqing changed the title check if between contains datetime planner: check if between contains datetime Apr 29, 2019
@erjiaqing erjiaqing changed the title planner: check if between contains datetime planner: fix wrong DATE/DATETIME comparison in BETWEEN function Apr 29, 2019
@qw4990 qw4990 self-requested a review April 29, 2019 13:40
@qw4990 qw4990 requested a review from XuHuaiyu April 30, 2019 03:31
Comment thread planner/core/expression_rewriter.go Outdated
Copy link
Copy Markdown
Contributor

@qw4990 qw4990 left a comment

Choose a reason for hiding this comment

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

LGTM

@c4pt0r c4pt0r added the status/LGT1 Indicates that a PR has LGTM 1. label May 1, 2019
@qw4990 qw4990 requested review from alivxxx and winoros May 6, 2019 02:52
Copy link
Copy Markdown
Contributor

@alivxxx alivxxx left a comment

Choose a reason for hiding this comment

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

LGTM

@alivxxx alivxxx added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels May 6, 2019
@alivxxx
Copy link
Copy Markdown
Contributor

alivxxx commented May 6, 2019

/run-all-tests

@qw4990 qw4990 merged commit 7e80053 into pingcap:master May 6, 2019
@erjiaqing erjiaqing deleted the issue_9764 branch May 9, 2019 09:05
erjiaqing added a commit to erjiaqing/tidb that referenced this pull request May 9, 2019
@sre-bot sre-bot added the contribution This PR is from a community contributor. label Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/expression contribution This PR is from a community contributor. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug. type/compatibility

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrong DATE/DATETIME comparison in BETWEEN function

5 participants