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.

Sign up for GitHub

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

Jump to bottom

expression: fix compatibility behaviors in zero datetime with MySQL #21220

Merged
merged 14 commits into from
Dec 7, 2020
Merged

expression: fix compatibility behaviors in zero datetime with MySQL #21220

merged 14 commits into from
Dec 7, 2020

Conversation

lzmhhh123
Copy link
Contributor

What problem does this PR solve?

Issue Number: close #9728

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

Side effects

  • Performance regression
    • Consumes more CPU

Release note

  • fix compatibility behaviors in zero datetime with MySQL.

@lzmhhh123 lzmhhh123 requested a review from a team as a code owner November 23, 2020 09:40
@lzmhhh123 lzmhhh123 requested review from wshwsh12 and removed request for a team November 23, 2020 09:40
@lzmhhh123
Copy link
Contributor Author

/cc @qw4990

@ti-srebot ti-srebot requested a review from qw4990 November 23, 2020 09:40
@lzmhhh123
Copy link
Contributor Author

/cc @ichn-hu

@ti-srebot ti-srebot requested a review from ichn-hu November 23, 2020 09:40
@lzmhhh123
Copy link
Contributor Author

/run-all-tests

@ichn-hu ichn-hu mentioned this pull request Nov 23, 2020
Closed
@lzmhhh123
Copy link
Contributor Author

/run-all-tests

@lzmhhh123
Copy link
Contributor Author

/run-unit-test

ichn-hu
ichn-hu previously approved these changes Dec 1, 2020
View reviewed changes
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 added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 1, 2020
wshwsh12
wshwsh12 reviewed Dec 3, 2020
View reviewed changes
Copy link
Contributor

@wshwsh12 wshwsh12 left a comment

Choose a reason for hiding this comment

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

I think vectorized code should also be modified.

executor/executor_test.go Outdated Show resolved Hide resolved
expression/builtin_time.go Outdated Show resolved Hide resolved
@lzmhhh123 lzmhhh123 linked an issue Dec 4, 2020 that may be closed by this pull request
Function DAY/DAYOFMONTH should do type conversation #11178
Closed
@lzmhhh123
address comment
fe3d89c 
Signed-off-by: lzmhhh123 <[email protected]>
@lzmhhh123
fix
3a401af 
Signed-off-by: lzmhhh123 <[email protected]>
@lzmhhh123
address comment
e09cd17 
Signed-off-by: lzmhhh123 <[email protected]>
@lzmhhh123
fix
34a2401 
Signed-off-by: lzmhhh123 <[email protected]>
@lzmhhh123
fix
3420704 
Signed-off-by: lzmhhh123 <[email protected]>
@lzmhhh123
Merge branch 'master' into bug-fix/fix_zero_datetime
42ca414
wshwsh12
wshwsh12 reviewed Dec 4, 2020
View reviewed changes
Copy link
Contributor

@wshwsh12 wshwsh12 left a comment

Choose a reason for hiding this comment

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

(b *builtinExtractDatetimeSig) vecEvalInt should also be modified.

@lzmhhh123
Copy link
Contributor Author

builtinExtractDatetimeSig

No, builtinExtractDatetimeSig has another implementation. And it's right.

@lzmhhh123
modify vec
5900ab1 
Signed-off-by: lzmhhh123 <[email protected]>
@wshwsh12
Copy link
Contributor

wshwsh12 commented Dec 7, 2020

lgtm

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Dec 7, 2020
@ti-challenge-bot
Copy link

There is no reward for this challenge pull request, so you can request a reward from @SunRunAway.

Details

Tip : About reward you can refs to reward-command.

Warning: None

@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Dec 7, 2020
@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 Dec 7, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@lzmhhh123 merge failed.

@lzmhhh123 lzmhhh123 merged commit 0deb0a3 into pingcap:master Dec 7, 2020
@lzmhhh123 lzmhhh123 deleted the bug-fix/fix_zero_datetime branch December 7, 2020 06:13
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Dec 7, 2020
@lzmhhh123 @ti-srebot
cherry pick pingcap#21220 to release-3.0
2246b2e 
Signed-off-by: ti-srebot <[email protected]>
@ti-srebot ti-srebot mentioned this pull request Dec 7, 2020
Closed
@ti-srebot
Copy link
Contributor

cherry pick to release-3.0 in PR #21524

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Dec 7, 2020
@lzmhhh123 @ti-srebot
cherry pick pingcap#21220 to release-4.0
7673500 
Signed-off-by: ti-srebot <[email protected]>
@ti-srebot ti-srebot mentioned this pull request Dec 7, 2020
Merged
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #21525

asiafrank pushed a commit to asiafrank/tidb that referenced this pull request Dec 8, 2020
@lzmhhh123 @asiafrank
expression: fix compatibility behaviors in zero datetime with MySQL (p…
57718b5 
…ingcap#21220)
ti-srebot added a commit that referenced this pull request Jan 26, 2021
@ti-srebot
expression: fix compatibility behaviors in zero datetime with MySQL (#…
12d2482 
…21220) (#21525)

Signed-off-by: ti-srebot <[email protected]>
Signed-off-by: lzmhhh123 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wshwsh12 wshwsh12 wshwsh12 left review comments

@ti-srebot ti-srebot ti-srebot approved these changes

@ichn-hu ichn-hu ichn-hu left review comments

@qw4990 qw4990 Awaiting requested review from qw4990

Assignees
No one assigned
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/bugfix This PR fixes a bug.
Projects
None yet
Milestone
No milestone
4 participants
@lzmhhh123 @wshwsh12 @ti-srebot @ichn-hu