timeutil: unify parsing of AT TIME ZONE and SET TIME ZONE #43414
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
otan
force-pushed
the
forreal_fix
branch
2 times, most recently
from
f5b42ee
to
99e0a28
Compare
We previously had multiple permutations of parsing TIME ZONE params using the two different syntaxes of `AT TIME ZONE` and `SET TIME ZONE`. This aims to bring them all under one roof. Furthermore, we upgrade SET TIME ZONE functionality to respect colons in time names without the GMT/UTC prefix, e.g. '3:00'. Release note (sql change, bug fix): * We previously did not support `AT TIME ZONE` parsing for anything other than precise location strings, e.g. only `AT TIME ZONE 'Australia/Sydney'` works. This PR adds support for parsing `AT TIME ZONE` with various other offset behaviour that is supported by, SET TIME ZONE e.g. `AT TIME ZONE '+3'`, `AT TIME ZONE 'GMT+4'` * We previously did not support SET TIME ZONE with colons, e.g. `+4:00`. This PR adds that support in.
solongordon
approved these changes
Jan 13, 2020
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @solongordon)
|
bors r=solongordon |
craig bot
pushed a commit
that referenced
this pull request
Jan 13, 2020
43379: tree: fix various interval parsing bugs with interval qualifiers r=solongordon a=otan This PR aims to address all issues related to bugs with intervals when we have interval qualifiers (e.g. DAY TO HOUR, etc). The major thing of note is removing one block of parsing from `ParseDInterval` into `sqlStdToDuration`, to simplify the logic for these bug fixes. The other ones are (hopefully) straightforward to review. Release note (sql change, bug fix): * Previously, `SELECT interval '1-2 1' DAY TO HOUR` would fail, but is permitted as per the SQL standard. This is now fixed. * Previously, adding spaces to intervals with qualifiers, e.g. `SELECT interval ' 1 ' YEAR` would try evaluate it as seconds always. Patched to use the qualifier as the multiplier instead. * Previously, adding a decimal point to days, e.g. `SELECT interval '1.5 01:00:00'` would return `1 day 01:00:00`, instead of postgres which returns `1 day 13:00:00`. This PR fixes that behaviour to match postgres. * Previously, using the `Y-M <constant>` format for intervals, e.g. `SELECT INTERVAL '1-2 3'` will always resolve the '<constant>' component (3) as seconds. However, for items such as `SELECT INTERVAL '1-2 3' DAY`, the constant (3) should represent days, as does postgres. 43414: timeutil: unify parsing of AT TIME ZONE and SET TIME ZONE r=solongordon a=otan Resolves cockroachdb/django-cockroachdb#97. We previously had multiple permutations of parsing TIME ZONE params using the two different syntaxes of `AT TIME ZONE` and `SET TIME ZONE`. This aims to bring them all under one roof. Furthermore, we upgrade SET TIME ZONE functionality to respect colons in time names without the GMT/UTC prefix, e.g. '3:00'. Release note (sql change, bug fix): * We previously did not support `AT TIME ZONE` parsing for anything other than precise location strings, e.g. only `AT TIME ZONE 'Australia/Sydney'` works. This PR adds support for parsing `AT TIME ZONE` with various other offset behaviour that is supported by, SET TIME ZONE e.g. `AT TIME ZONE '+3'`, `AT TIME ZONE 'GMT+4'` * We previously did not support SET TIME ZONE with colons, e.g. `+4:00`. This PR adds that support in. 43798: demo: add splits to movr dataset r=knz a=rohany Fixes #42592. This PR adds split points to the movr dataset so that ranges will be spread out among all of the nodes in the cluster. Release note (cli change): The movr dataset will be split among all nodes in the demo cluster. Co-authored-by: Oliver Tan <otan@cockroachlabs.com> Co-authored-by: Rohan Yadav <rohany@alumni.cmu.edu>
Build succeeded |
This was referenced Feb 19, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves cockroachdb/django-cockroachdb#97.
We previously had multiple permutations of parsing TIME ZONE params
using the two different syntaxes of
AT TIME ZONEandSET TIME ZONE.This aims to bring them all under one roof.
Furthermore, we upgrade SET TIME ZONE functionality to respect colons
in time names without the GMT/UTC prefix, e.g. '3:00'.
Release note (sql change, bug fix):
AT TIME ZONEparsing for anythingother than precise location strings, e.g. only
AT TIME ZONE 'Australia/Sydney'works. This PR adds support for parsingAT TIME ZONEwith various other offset behaviour that is supported by,SET TIME ZONE e.g.
AT TIME ZONE '+3',AT TIME ZONE 'GMT+4'+4:00. This PR adds that support in.