pgdate: Improve handling of negative years #35602
Merged
+37
−4
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 |
knz
approved these changes
Mar 11, 2019
| @@ -1193,3 +1193,32 @@ query T | |||
| SELECT date_trunc('month', "date") AS date_trunc_month_created_at FROM "topics"; | |||
| ---- | |||
| 2017-12-01 00:00:00 +0000 UTC | |||
|
|
|||
|
|
|||
| # Test negative years, ensuring that we don't trigger any of the | |||
There was a problem hiding this comment.
maybe add subtest regression_35255 here
| @@ -320,7 +320,7 @@ func (fe *fieldExtract) interpretNumber(chunk numberChunk, textMonth bool) error | |||
| case fe.Wants(fieldYear) && fe.Wants(fieldMonth) && fe.Wants(fieldDay): | |||
| // Example: All date formats, we're starting from scratch. | |||
| switch { | |||
| case chunk.magnitude >= 6: | |||
| case chunk.magnitude >= 6 && chunk.separator != '-': | |||
There was a problem hiding this comment.
Just curious, why do you exclude the sign here?
bobvawter
commented
Mar 11, 2019
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis and @knz)
pkg/sql/logictest/testdata/logic_test/datetime, line 1198 at r1 (raw file):
Previously, knz (kena) wrote…
maybe add
subtest regression_35255here
Done.
pkg/util/timeutil/pgdate/field_extract.go, line 323 at r1 (raw file):
Previously, knz (kena) wrote…
Just curious, why do you exclude the sign here?
Updated with a comment explaining that we don't want large-magnitude, negative years to be incorrectly interpreted.
|
bors r+ |
craig bot
pushed a commit
that referenced
this pull request
Mar 11, 2019
35442: roachtest: add nightly multi-AZ tpccbench test r=nvanbenschoten a=nvanbenschoten This test deviates from similar roachtests in a few useful ways: - it tests across three AZs in the same region - it tests a 6-node cluster - it loads 5k tpcc warehouses - it runs on only about half of them (testing the impact of cold data) I've run this a number of times over the past few days and settled upon 2500 warehouses as a good estimate. Release note: None 35602: pgdate: Improve handling of negative years r=bobvawter a=bobvawter The current parsing code doesn't handle negative year values and returns an unhelpful error message. This change allows negative years to be specified, so that negative-year datums can at least be round-tripped through the parser. Supports #28099 Fixes #35255 Release note: None Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com> Co-authored-by: Bob Vawter <bob@cockroachlabs.com>
Build succeeded |
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.
The current parsing code doesn't handle negative year values and returns an
unhelpful error message. This change allows negative years to be specified, so
that negative-year datums can at least be round-tripped through the parser.
Supports #28099
Fixes #35255
Release note: None