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

types: match MySQL behavior with datetime delimiters (#17376) #17500

Closed
wants to merge 3 commits into from

Conversation

sre-bot
Copy link
Contributor

@sre-bot sre-bot commented May 29, 2020

cherry-pick #17376 to release-3.1


What problem does this PR solve?

Issue Number: close #17277

Problem Summary: select date '2020-02--1' fails

What is changed and how it works?

  • types.ParseDateFormat changed datetime splitting to match MySQL 5.7.
  • Any ASCII punctuation character (i.e. printable non-alphanumeric) is a valid delimiter.
  • Additionally, (space) and T are valid delimiters if they appear between time and date.
  • Multiple consecutive delimiters are accepted, and handled as a single delimiter.
  • Consecutive delimiters can be mixed (e.g. -./ or T/TT).
  • This PR does not apply to types.ParseDuration, since it has additional validation.

Check List

  • Unit tests

Backwards compatibility

  • In previous TiDB versions, T and (space) were valid delimiters anywhere in a datetime string. After this change, using T or (space) as a delimiter is an error except between date and time.

Release note

  • Datetime parsing now matches MySQL 5.7 behavior for delimiters.

@sre-bot
Copy link
Contributor Author

sre-bot commented May 29, 2020

/run-all-tests

@wjhuang2016
Copy link
Member

@smola Could you resolve the conflict?

@bb7133 bb7133 modified the milestones: v3.1.2, v3.1.3 Jun 6, 2020
@Reminiscent
Copy link
Contributor

@smola @wjhuang2016 PTAL

@@ -1812,86 +1812,3 @@ func (s *testTimeSuite) TestFromGoTime(c *C) {
}

}
<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should remove these benchmarks?

Copy link
Contributor

Choose a reason for hiding this comment

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

@wjhuang2016 Update. PTAL

Copy link
Member

@wjhuang2016 wjhuang2016 left a comment

Choose a reason for hiding this comment

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

LGTM

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. type/3.1-cherry-pick
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants