github-actions[bot] commented on code in PR #68257:
URL: https://github.com/apache/doris/pull/68257#discussion_r4056544529
##########
be/src/core/data_type_serde/data_type_datetimev2_serde.cpp:
##########
@@ -683,7 +683,13 @@ Status
DataTypeDateTimeV2SerDe::read_column_from_arrow(IColumn& column,
// "2022-01-01 11:11:11.111", timestamp = 1641035471111, divisor =
1000,
// set_microsecond(111000)
v.set_microsecond(remainder * DIVISOR_FOR_MICRO / divisor);
- col_data.emplace_back(v);
+ DateV2Value<DateTimeV2ValueType> scaled_v;
+ if (!transform_date_scale(_scale, 6, scaled_v, v)) {
Review Comment:
[P1] Round the Arrow instant before timezone projection
At this point the source semantics have already been lost in two ways. For
`TIMESTAMP(NANO) -> DATETIMEV2(6)`, line 685 truncates `123456789ns` to
`123456us`, and `transform_date_scale(6, 6, ...)` can only copy it, while the
existing TIMESTAMP_NS cast contract rounds it to `123457us` (and carries
`999999500ns` into the next second). For timezone-aware input, rounding the
already-projected civil value also crosses DST incorrectly:
`1710064799999500us` with `America/Los_Angeles` and scale 3 becomes the
nonexistent `2024-03-10 02:00:00`, whereas the rounded instant maps to
`03:00:00`. Please round the epoch quotient/fraction at its actual Arrow unit
before `from_unixtime`, preserve the negative floor-normalization, validate the
final projected DATETIMEV2 range, and add NANO scale-6 plus DST-boundary
coverage.
##########
be/src/core/data_type_serde/data_type_datetimev2_serde.cpp:
##########
@@ -683,7 +683,13 @@ Status
DataTypeDateTimeV2SerDe::read_column_from_arrow(IColumn& column,
// "2022-01-01 11:11:11.111", timestamp = 1641035471111, divisor =
1000,
// set_microsecond(111000)
v.set_microsecond(remainder * DIVISOR_FOR_MICRO / divisor);
- col_data.emplace_back(v);
+ DateV2Value<DateTimeV2ValueType> scaled_v;
+ if (!transform_date_scale(_scale, 6, scaled_v, v)) {
+ return Status::DataQualityError(
+ "Arrow timestamp exceeds DATETIMEV2 range after
rounding to scale {}",
+ _scale);
+ }
+ col_data.emplace_back(scaled_v);
Review Comment:
[P1] Skip null-slot payloads before the new failure path
`DataTypeNullableSerDe` copies the validity bitmap and then sends every
physical slot to this nested serde, but Arrow does not define the data buffer
under a null bit. The new fallible rounding therefore makes that hidden payload
observable: a null `timestamp[us]` slot containing `253402300799999500` read as
`Nullable(DateTimeV2(3))` rounds the hidden max-date value past year 9999 and
returns `DataQualityError` even though the logical value is NULL. The
TIMESTAMP_NS and TIMESTAMPTZ Arrow readers already handle this contract by
appending a default and continuing when `IsNull(i)`. Please do the same before
decoding/rounding here and cover the nullable upper-bound case.
--
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]
