colexecbase: add all remaining casts from strings #83958
Conversation
|
This change is |
This commit does a few minor things: - actually uses the error in a few places when constructing a ParseError - refactors some of the interval-parsing functions to expose them to be used in the follow-up commit - extracts a helper method to construct an error when timestamp exceeds bounds. Release note: None
This commit sorts the information about natively supported casts lexicographically so that it is easier to see what is actually supported. This is simply a mechanical change. Release note: None
This commit adds the native casts from strings to all remaining natively-supported types (dates, decimals, floats, ints, intervals, timestamps, jsons). I was inspired to do this because the combination of this commit and the vectorized rendering on top of the wrapped row-by-row processors would expose some bugs (e.g. cockroachdb#83094). Release note: None
yuzefovich
changed the title
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, 2 of 2 files at r2, 3 of 4 files at r3, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball, @msirek, and @yuzefovich)
pkg/sql/colexec/execgen/cmd/execgen/cast_gen_util.go line 474 at r3 (raw file):
_now := %[3]s.GetRelativeParseTime() _dateStyle := %[3]s.GetDateStyle() _t, _, err := pgdate.ParseTimestamp%[5]s(_now, _dateStyle, string(%[2]s))
What does %[5]s represent here?
There was a problem hiding this comment.
Reviewable status:
pkg/sql/colexec/execgen/cmd/execgen/cast_gen_util.go line 474 at r3 (raw file):
Previously, msirek (Mark Sirek) wrote…
What does
%[5]srepresent here?
It refers to the fifth argument for the "format" string (which is parseTimestampKind in this case). Depending on the type (i.e. timestamp vs timestamptz) we want to use different methods for parsing which differ only in a suffix.
There was a problem hiding this comment.
Reviewed 1 of 4 files at r3.
Reviewable status:
pkg/sql/colexec/execgen/cmd/execgen/cast_gen_util.go line 474 at r3 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
It refers to the fifth argument for the "format" string (which is
parseTimestampKindin this case). Depending on the type (i.e. timestamp vs timestamptz) we want to use different methods for parsing which differ only in a suffix.
Thanks!
|
TFTR! bors r+ |
|
Build failed: |
|
Unrelated flake. bors r+ |
|
Build failed: |
|
Another unrelated flake. bors r+ |
|
Build failed: |
|
bors r+ |
|
Build succeeded: |
tree: minor cleanup
This commit does a few minor things:
used in the follow-up commit
bounds.
Release note: None
colexecbase: sort native cast info lexicographically
This commit sorts the information about natively supported casts
lexicographically so that it is easier to see what is actually
supported. This is simply a mechanical change.
Release note: None
colexecbase: add all remaining casts from strings
This commit adds the native casts from strings to all remaining
natively-supported types (dates, decimals, floats, ints, intervals,
timestamps, jsons).
I was inspired to do this because the combination of this
commit and the vectorized rendering on top of the wrapped row-by-row
processors would expose some bugs (e.g. #83094).
Addresses: #48135.
Release note: None