github-actions[bot] commented on code in PR #67500:
URL: https://github.com/apache/doris/pull/67500#discussion_r3943607363
##########
be/src/core/value/time_value.h:
##########
@@ -64,9 +64,11 @@ class TimeValue {
static TimeType make_time(int64_t hour, int64_t minute, int64_t second,
int64_t microsecond = 0,
bool negative = false) {
if constexpr (CHECK) {
- // the max time value is 838:59:59.999999
+ // the max time value is 838:59:59.000000
if (std::abs(hour) > 838 || std::abs(minute) >= 60 ||
std::abs(second) >= 60 ||
- std::abs(microsecond) >= 1000000) [[unlikely]] {
+ std::abs(microsecond) >= 1000000 ||
+ (std::abs(hour) == 838 && std::abs(minute) == 59 &&
std::abs(second) == 59 &&
Review Comment:
[P1] Revalidate after appending numeric fractions
This endpoint check only sees the whole-second value on numeric casts.
`from_float` and `from_decimal` first call `from_integer`, so DECIMAL
`8385959.000001` reaches `make_time<true>(838, 59, 59, 0)` and passes;
`init_microsecond` then appends the remaining microsecond, but calls
`TimeValue::valid` only when the fraction carries a full second. The DECIMAL
and FLOAT/DOUBLE batch writers therefore store raw `3020399000001` (and the
negative counterpart) as non-NULL even in strict mode. Formatting clamps that
payload to `838:59:59.000000`, while raw equality/grouping still distinguishes
it from the real endpoint. Please validate the fully assembled value after
`init_microsecond` for both numeric paths and cover both signs and scales 1-6
with a raw equality oracle.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
