Skip to content

expression: fix issue that period_diff is not compatible with MySQL 8.0#10383

Merged
zz-jason merged 6 commits intopingcap:masterfrom
sourcelliu:myfeature-perioddiff
May 14, 2019
Merged

expression: fix issue that period_diff is not compatible with MySQL 8.0#10383
zz-jason merged 6 commits intopingcap:masterfrom
sourcelliu:myfeature-perioddiff

Conversation

@sourcelliu
Copy link
Copy Markdown
Contributor

What problem does this PR solve?

fix #10382

What is changed and how it works?

add validPeriod. the method come from #10380 .
@qw4990 I used your 'validPeriod' method ,but change a little.

Check List

Tests

  • Unit test
  • Integration test

@shenli shenli added the contribution This PR is from a community contributor. label May 7, 2019
@codecov
Copy link
Copy Markdown

codecov bot commented May 7, 2019

Codecov Report

Merging #10383 into master will increase coverage by 0.0285%.
The diff coverage is 100%.

@@               Coverage Diff                @@
##             master     #10383        +/-   ##
================================================
+ Coverage   77.2926%   77.3211%   +0.0285%     
================================================
  Files           412        412                
  Lines         86571      86627        +56     
================================================
+ Hits          66913      66981        +68     
+ Misses        14514      14501        -13     
- Partials       5144       5145         +1

@qw4990
Copy link
Copy Markdown
Contributor

qw4990 commented May 8, 2019

Thanks for your contribution.
But it seems this PR has conflict with #10380, so please wait #10380 to be merged.

@qw4990
Copy link
Copy Markdown
Contributor

qw4990 commented May 9, 2019

@mantuliu #10380 has been merged into master, so please git rebase your PR and fix conflicts, PTAL~

@qw4990
Copy link
Copy Markdown
Contributor

qw4990 commented May 13, 2019

PTAL~ @mantuliu

@sourcelliu
Copy link
Copy Markdown
Contributor Author

@qw4990 take a look.thanks.
PTAL~

@qw4990 qw4990 requested review from alivxxx and qw4990 May 14, 2019 07:49
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

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
Copy link
Copy Markdown
Contributor

alivxxx commented May 14, 2019

/run-all-tests

@alivxxx alivxxx added the status/LGT2 Indicates that a PR has LGTM 2. label May 14, 2019
@qw4990
Copy link
Copy Markdown
Contributor

qw4990 commented May 14, 2019

/run-all-tests

@alivxxx
Copy link
Copy Markdown
Contributor

alivxxx commented May 14, 2019

/run-integration-ddl-test

@qw4990
Copy link
Copy Markdown
Contributor

qw4990 commented May 14, 2019

/run-all-tests

@qw4990
Copy link
Copy Markdown
Contributor

qw4990 commented May 14, 2019

@mantuliu It seems there are some CI problems not caused by your PR.
Please git rebase this PR to the newest master branch and /run-all-tests again to try to fix them.

@zz-jason
Copy link
Copy Markdown
Member

/run-all-tests

@codecov
Copy link
Copy Markdown

codecov bot commented May 14, 2019

Codecov Report

Merging #10383 into master will increase coverage by 0.0102%.
The diff coverage is 100%.

@@               Coverage Diff                @@
##             master     #10383        +/-   ##
================================================
+ Coverage   77.2926%   77.3029%   +0.0102%     
================================================
  Files           412        412                
  Lines         86571      86575         +4     
================================================
+ Hits          66913      66925        +12     
+ Misses        14514      14505         -9     
- Partials       5144       5145         +1

@zz-jason zz-jason merged commit 7deedf8 into pingcap:master May 14, 2019
@qw4990
Copy link
Copy Markdown
Contributor

qw4990 commented May 14, 2019

@mantuliu Thanks for your contribution.
Could you please cherry-pick this PR to branch release-2.1?

@sourcelliu
Copy link
Copy Markdown
Contributor Author

@qw4990 ok,I will cherry-pick this PR to branch release-2.1.

@sourcelliu
Copy link
Copy Markdown
Contributor Author

@qw4990 Please cherry-pick #10380 firstly,thanks.

@qw4990
Copy link
Copy Markdown
Contributor

qw4990 commented May 15, 2019

@qw4990 Please cherry-pick #10380 firstly,thanks.

It's in progress #10430 and waited to merge.

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.

Function period_diff is not compatible with MySQL 8.0

6 participants