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, executor: add iso 8601 and timezone support for temporal string literal #20534

Merged
merged 17 commits into from
Oct 27, 2020

Conversation

ichn-hu
Copy link
Contributor

@ichn-hu ichn-hu commented Oct 20, 2020

What problem does this PR solve?

Issue Number: close #20348 and addtionally close #20507

Problem Summary:

Added timezone support for datetime and timestamp string literal.

What is changed and how it works?

Proposal: see discussions here

What's Changed:

Added parsing, absorbing and converting logic for correct handling of ISO8601 and MySQL 8.0 timezone.

How it Works:

  • Parsing is done by enumerating all possible scenariors.
  • Absorbing is done by look ahead at the separated parts of date and time.
  • Converting is done by first interpret the time by given timezone and then convert back to system timezone.

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to 4.0, need to be delivered in 4.0.8.

Check List

Tests

  • Unit test
  • Integration test

Side effects

  • Performance regression
    • Consumes more CPU

Release note

  • No release note

@ichn-hu ichn-hu requested a review from a team as a code owner October 20, 2020 07:38
@ichn-hu ichn-hu requested review from wshwsh12 and removed request for a team October 20, 2020 07:38
@ichn-hu
Copy link
Contributor Author

ichn-hu commented Oct 21, 2020

/label status/WIP

@ichn-hu ichn-hu changed the title expression, executor: Add iso 8601 and timezone support for temporal string literal [WIP] expression, executor: add iso 8601 and timezone support for temporal string literal [WIP] Oct 21, 2020
@ichn-hu
Copy link
Contributor Author

ichn-hu commented Oct 22, 2020

/cc @XuHuaiyu /cc @zz-jason

@ichn-hu
Copy link
Contributor Author

ichn-hu commented Oct 22, 2020

/unlabel status/WIP

@ichn-hu
Copy link
Contributor Author

ichn-hu commented Oct 22, 2020

/cc @XuHuaiyu

@ichn-hu
Copy link
Contributor Author

ichn-hu commented Oct 22, 2020

/cc @zz-jason

@ichn-hu
Copy link
Contributor Author

ichn-hu commented Oct 22, 2020

@XuHuaiyu @zz-jason PTAL ASAP, lets ship it to 4.0.8 (by the end of tmr), I will continue to add test cases, but the main functionality is already done.

@ichn-hu
Copy link
Contributor Author

ichn-hu commented Oct 22, 2020

/run-all-tests

@ichn-hu ichn-hu changed the title expression, executor: add iso 8601 and timezone support for temporal string literal [WIP] expression, executor: add iso 8601 and timezone support for temporal string literal Oct 22, 2020
@breezewish
Copy link
Member

Cast has been pushed down to TiKV, maybe TiKV need to be updated as well?

types/time.go Show resolved Hide resolved
types/time.go Show resolved Hide resolved
types/time.go Outdated Show resolved Hide resolved
types/time.go Show resolved Hide resolved
types/time.go Show resolved Hide resolved
types/time.go Outdated Show resolved Hide resolved
types/time.go Show resolved Hide resolved
types/time.go Show resolved Hide resolved
@ichn-hu
Copy link
Contributor Author

ichn-hu commented Oct 22, 2020

/run-all-tests

@ichn-hu
Copy link
Contributor Author

ichn-hu commented Oct 23, 2020

/cc @SunRunAway

@ichn-hu
Copy link
Contributor Author

ichn-hu commented Oct 23, 2020

/cc @rebelice

@ichn-hu
Copy link
Contributor Author

ichn-hu commented Oct 26, 2020

/cc @lzmhhh123

@ichn-hu
Copy link
Contributor Author

ichn-hu commented Oct 27, 2020

/run-all-tests

@ichn-hu
Copy link
Contributor Author

ichn-hu commented Oct 27, 2020

/run-all-tests

@ichn-hu
Copy link
Contributor Author

ichn-hu commented Oct 27, 2020

/run-check-dev

@ichn-hu
Copy link
Contributor Author

ichn-hu commented Oct 27, 2020

/run-all-tests

2 similar comments
@ichn-hu
Copy link
Contributor Author

ichn-hu commented Oct 27, 2020

/run-all-tests

@ichn-hu
Copy link
Contributor Author

ichn-hu commented Oct 27, 2020

/run-all-tests

@lzmhhh123 lzmhhh123 merged commit 23d8b30 into pingcap:master Oct 27, 2020
@lzmhhh123
Copy link
Contributor

/run-cherry-picker

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Oct 27, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #20670

ti-srebot added a commit that referenced this pull request Oct 27, 2020
…string literal (#20534) (#20670)

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ichn-hu ichn-hu deleted the add-iso-8601 branch December 24, 2020 10:22
@ichn-hu ichn-hu restored the add-iso-8601 branch December 24, 2020 10:22
ti-chi-bot pushed a commit to tikv/tikv that referenced this pull request Feb 3, 2021
<!--
Thank you for contributing to TiKV!

If you haven't already, please read TiKV's [CONTRIBUTING](https://github.com/tikv/tikv/blob/master/CONTRIBUTING.md) document.

If you're unsure about anything, just ask; somebody should be along to answer within a day or two.

PR Title Format:
1. module [, module2, module3]: what's changed
2. *: what's changed

If you want to open the **Challenge Program** pull request, please use the following template:
https://raw.githubusercontent.com/tikv/.github/master/.github/PULL_REQUEST_TEMPLATE/challenge-program.md
You can use it with query parameters: https://github.com/tikv/tikv/compare/master...${you branch}?template=challenge-program.md
-->

### What problem does this PR solve?

Problem Summary: previously in TiDB pingcap/tidb#20534 , we supported parsing time string in ISO 8601 format, but the change has not yet ported to TiKV, and this PR gets the job done.

### What is changed and how it works?

What's Changed:

### Related changes

- Need to cherry-pick to the release branch 4.0.

### Check List <!--REMOVE the items that are not applicable-->

Tests <!-- At least one of them must be included. -->

- Unit test
- Integration test, see https://github.com/pingcap/tidb-test/pull/1149

Side effects

- Performance regression
    - Consumes more CPU

### Release note <!-- bugfixes or new feature need a release note -->

- port ISO8601 parsing from TiDB to TiKV's coprocessor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/expression sig/execution SIG execution status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
5 participants