morningman commented on code in PR #69581:
URL: https://github.com/apache/doris/pull/69581#discussion_r4395538887
##########
be/src/core/data_type_serde/data_type_datev2_serde.cpp:
##########
@@ -277,11 +277,27 @@ Status
DataTypeDateV2SerDe::read_column_from_arrow(IColumn& column, const arrow:
const auto* base_ptr = reinterpret_cast<const
uint8_t*>(concrete_array->raw_values());
const size_t element_size = sizeof(int32_t);
for (auto value_i = start; value_i < end; ++value_i) {
+ // A null slot has no calendar value and its payload may be
outside the DATE range.
+ if (concrete_array->IsNull(value_i)) {
+ col_data.emplace_back(DateV2Value<DateV2ValueType>());
+ continue;
+ }
const uint8_t* raw_byte_ptr = base_ptr + value_i * element_size;
auto date_value = unaligned_load<int32_t>(raw_byte_ptr);
+ // Arrow allows any int32 day, Doris DATE only day numbers
1..DATE_MAX_DAYNR
+ // (0000-01-01..9999-12-31). For example, 2932897 is 10000-01-01:
its day number
+ // 3652425 makes get_date_from_daynr() fail, and ignoring that
result used to append
+ // a non-null 0000-00-00. Add in 64 bits so INT32_MAX cannot
overflow, and reject
+ // the value like the DATE64 branch above.
+ const int64_t daynr = static_cast<int64_t>(date_value) +
date_threshold;
DateV2Value<DateV2ValueType> v;
- v.get_date_from_daynr(date_value + date_threshold);
+ if (daynr <= 0 || daynr > DATE_MAX_DAYNR ||
Review Comment:
Confirmed, and fixed in `a5e1ead1de1`. Arrow leaves the child values under a
null list, fixed-size-list, map or struct row unspecified, and the array SerDe
passed the whole child range to the element SerDe, which only skipped the
child's own nulls. The DATE32 range check that this PR adds therefore rejected
a hidden day such as 2932897 and failed a batch whose visible values are all
valid. The array, map and struct SerDes now hand their children a validity
bitmap that also clears the slots hidden by a null parent
(`mask_arrow_children_hidden_by_null_parents()`), so the element SerDe neither
validates nor decodes them; the parent's null map hides them as before. This
covers DATE32, DATE64 and any other element check; only the validity buffer is
replaced, and nothing is allocated when no parent row in the range is null.
`LanceTableReaderTypeTest.IgnoresDate32PayloadHiddenByNullParents` reads your
FIXED_SIZE_LIST<DATE32, 1> case, a LIST with a non-empty null segment, a
STRUCT, a MAP
and a LIST<STRUCT>, and fails without the change; `ArrowParentNullsTest`
covers the helper. branch-4.1 and branch-4.2 have the same check and SerDes.
--
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]
