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, types: Function DAY/DAYOFMONTH the NoZeroDateMode sql mode check when string cast to time #19930

Closed
wants to merge 6 commits into from

Conversation

Patrick0308
Copy link
Contributor

@Patrick0308 Patrick0308 commented Sep 10, 2020

What problem does this PR solve?

Issue Number: a part of #11178

Problem Summary:

What is changed and how it works?

What's Changed: check the NoZeroDateMode sql mode check when string cast to time .

How it Works:

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test

Release note

  • Function DAY/DAYOFMONTH modify

@Patrick0308 Patrick0308 requested a review from a team as a code owner September 10, 2020 09:28
@Patrick0308 Patrick0308 requested review from qw4990 and removed request for a team September 10, 2020 09:28
@ti-srebot ti-srebot added the contribution This PR is from a community contributor. label Sep 10, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Sep 10, 2020

Please follow PR Title Format:

  • pkg [, pkg2, pkg3]: what's changed

Or if the count of mainly changed packages are more than 3, use

  • *: what's changed

@Patrick0308 Patrick0308 changed the title Function DAY/DAYOFMONTH cast Decimal to DateTime Expression: Function DAY/DAYOFMONTH cast Decimal to DateTime Sep 10, 2020
@Patrick0308 Patrick0308 changed the title Expression: Function DAY/DAYOFMONTH cast Decimal to DateTime expression: Function DAY/DAYOFMONTH cast Decimal to DateTime Sep 10, 2020
@Patrick0308 Patrick0308 changed the title expression: Function DAY/DAYOFMONTH cast Decimal to DateTime expression, types: Function DAY/DAYOFMONTH cast Decimal to DateTime Sep 10, 2020
Copy link
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.

Please add cases in #11178 as a unit test into TiDB. @Patrick0308

@Patrick0308
Copy link
Contributor Author

@qw4990 OK

@Patrick0308 Patrick0308 changed the title expression, types: Function DAY/DAYOFMONTH cast Decimal to DateTime expression, types: Function DAY/DAYOFMONTH the NoZeroDateMode sql mode check when string cast to time Sep 20, 2020
@Patrick0308
Copy link
Contributor Author

/run-all-tests

@Patrick0308
Copy link
Contributor Author

/run-all-tests

@Patrick0308
Copy link
Contributor Author

/run-all-tests

1 similar comment
@Patrick0308
Copy link
Contributor Author

/run-all-tests

@Patrick0308
Copy link
Contributor Author

/run-all-tests

Comment on lines +1260 to +1262
if res.IsZero() && b.ctx.GetSessionVars().SQLMode.HasNoZeroDateMode() {
return types.ZeroDate, true, handleInvalidTimeError(b.ctx, types.ErrWrongValue.GenWithStackByArgs(types.DateTimeStr, res.String()))
}
Copy link
Member

Choose a reason for hiding this comment

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

Please add a test case to cover this.

@zz-jason
Copy link
Member

zz-jason commented Feb 9, 2021

I'm going to close this PR since it hasn't been updated for a long time. feel free to reopen if you want to continue with it. thank you for your contribution.

BTW, seems the original issue has been fixed.

@zz-jason zz-jason closed this Feb 9, 2021
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants