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
Introduce setting date_time_overflow_behavior
to control the overflow behavior when converting to Date
/ Date32
/ DateTime
/ DateTime64
#55696
Conversation
Current behavior is left as default, but the behavior itself is not very good. |
0ec8dc0
to
eb7c6a3
Compare
This is an automated comment for commit 2da12ec with description of existing statuses. It's updated for the latest CI running ✅ Click here to open a full report in a separate page Successful checks
|
00a4fc6
to
a84e6dd
Compare
b62db6f
to
1f81cce
Compare
date_time_overflow_mode
to control the overflow behavior when converting to Date
/ Date32
/ DateTime
/ DateTime64
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
6f4d5fb
to
014babb
Compare
014babb
to
f37f7f3
Compare
throw Exception{ErrorCodes::VALUE_IS_OUT_OF_RANGE_OF_DATA_TYPE, | ||
"Input value {} of a column \"{}\" is out of allowed Date32 range, which is [{}, {}]", days_num, column_name, DAYNUM_OFFSET_EPOCH, DATE_LUT_MAX_EXTEND_DAY_NUM}; | ||
{ | ||
if (date_time_overflow_mode == FormatSettings::DateTimeOverflowMode::Saturate) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
Sorry, I know I asked before but should we replace the
if
by aswitch
? -
If the mode is
ignore
, we'll throw an exception. That doesn't sound right. EDIT: I think you wanted to preserve the previous behavior which wasthrow
and therefore different from the setting default valueignore
. So that's okay then, but please let's add a comment to not confuse future readers of the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Yes, here it is fine to replace it
- Comment added
Closes #51402, #39249.
This is a better version of #40217.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Introduced setting
date_time_overflow_behavior
with possible valuesignore
,throw
,saturate
that controls the overflow behavior when converting from Date, Date32, DateTime64, Integer or Float to Date, Date32, DateTime or DateTime64.