Skip to content

Function SUBTIME ADDTIME should return NULL with a warning if its argument is an invalid time value.#11262

Merged
lzmhhh123 merged 3 commits into
pingcap:masterfrom
tcmichael:bugfix-11175
Jul 19, 2019
Merged

Function SUBTIME ADDTIME should return NULL with a warning if its argument is an invalid time value.#11262
lzmhhh123 merged 3 commits into
pingcap:masterfrom
tcmichael:bugfix-11175

Conversation

@tcmichael
Copy link
Copy Markdown

@tcmichael tcmichael commented Jul 15, 2019 •
edited
Loading

What problem does this PR solve?

Function SUBTIME ADDTIME should return NULL with a warning if its argument is an invalid time value.
Closes #11175

What is changed and how it works?

ignores ErrTruncatedWrongVal.

Check List

Tests

  • Unit test
  • Integration test

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 15, 2019 •
edited
Loading

Codecov Report

Merging #11262 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #11262   +/-   ##
===========================================
  Coverage   81.3855%   81.3855%           
===========================================
  Files           423        423           
  Lines         90607      90607           
===========================================
  Hits          73741      73741           
  Misses        11570      11570           
  Partials       5296       5296

@tcmichael tcmichael changed the title Function SUBTIME should return NULL with a warning if its argument is an invalid time value. Function SUBTIME ADDTIME should return NULL with a warning if its argument is an invalid time value. Jul 15, 2019
@tcmichael
Copy link
Copy Markdown
Author

tcmichael commented Jul 15, 2019

@SunRunAway PTAL

SunRunAway
SunRunAway suggested changes Jul 15, 2019
View reviewed changes
Comment thread expression/integration_test.go Outdated
Comment thread expression/builtin_time.go Outdated
@SunRunAway SunRunAway added component/expression contribution This PR is from a community contributor. type/bugfix This PR fixes a bug. type/compatibility labels Jul 15, 2019
@tcmichael
Copy link
Copy Markdown
Author

tcmichael commented Jul 16, 2019

@SunRunAway PTAL

SunRunAway
SunRunAway reviewed Jul 16, 2019
View reviewed changes
Copy link
Copy Markdown
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

@tcmichael tcmichael force-pushed the bugfix-11175 branch 2 times, most recently from 6b4cbbe to 364b912 Compare July 16, 2019 07:06
@SunRunAway SunRunAway mentioned this pull request Jul 16, 2019
Closed
lzmhhh123
lzmhhh123 reviewed Jul 18, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@lzmhhh123 lzmhhh123 left a comment

Choose a reason for hiding this comment

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

@tcmichael Should we also consider other types of builtin_time like builtinSubStringAndDurationSig?

@tcmichael tcmichael force-pushed the bugfix-11175 branch 2 times, most recently from efe6269 to 6b8f90f Compare July 19, 2019 02:58
@tcmichael
Copy link
Copy Markdown
Author

tcmichael commented Jul 19, 2019

@lzmhhh123 I tried to find out the functions of the same case, hoping that I found them all.

lzmhhh123
lzmhhh123 approved these changes Jul 19, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@lzmhhh123 lzmhhh123 left a comment

Choose a reason for hiding this comment

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

@tcmichael Nice job! LGTM.

@lzmhhh123
Copy link
Copy Markdown
Contributor

lzmhhh123 commented Jul 19, 2019

@SunRunAway Please take a look. If ok, disable the change requested.

SunRunAway
SunRunAway approved these changes Jul 19, 2019
View reviewed changes
Copy link
Copy Markdown
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

@SunRunAway
Copy link
Copy Markdown
Contributor

SunRunAway commented Jul 19, 2019

/run-all-tests

@SunRunAway
Copy link
Copy Markdown
Contributor

SunRunAway commented Jul 19, 2019

/run-all-tests

@lzmhhh123 lzmhhh123 merged commit d977edf into pingcap:master Jul 19, 2019
@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Jul 19, 2019

cherry pick to release-2.1 failed

@sre-bot sre-bot mentioned this pull request Jul 19, 2019
Merged
@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Jul 19, 2019

cherry pick to release-3.0 in PR #11337

@lzmhhh123 lzmhhh123 mentioned this pull request Jul 19, 2019
Closed
@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Apr 7, 2020

It seems that, not for sure, we failed to cherry-pick this commit to release-2.1. Please comment '/run-cherry-picker' to try to trigger the cherry-picker if we did fail to cherry-pick this commit before. @SunRunAway PTAL.

@hidehalo hidehalo mentioned this pull request Aug 16, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zz-jason zz-jason Awaiting requested review from zz-jason

@eurekaka eurekaka Awaiting requested review from eurekaka

2 more reviewers

@SunRunAway SunRunAway SunRunAway approved these changes

@lzmhhh123 lzmhhh123 lzmhhh123 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

component/expression contribution This PR is from a community contributor. type/bugfix This PR fixes a bug. type/compatibility

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Function SUBTIME should return NULL with a warning if its argument is an invalid time value.

5 participants

@tcmichael @lzmhhh123 @SunRunAway @sre-bot @zz-jason