github-actions[bot] commented on code in PR #68649:
URL: https://github.com/apache/doris/pull/68649#discussion_r4146799308
##########
be/src/core/data_type_serde/data_type_datev2_serde.cpp:
##########
@@ -278,12 +295,20 @@ 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) {
- int32_t date_value = 0;
+ // 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;
- memcpy(&date_value, raw_byte_ptr, element_size);
+ auto date_value = unaligned_load<int32_t>(raw_byte_ptr);
DateV2Value<DateV2ValueType> v;
- v.get_date_from_daynr(date_value + date_threshold);
+ if (const auto status = decode_epoch_days(date_value, &v);
!status.ok()) {
+ return Status::InvalidArgument(
+ "Arrow Date32 value is outside the Doris DATE range:
row={}, days={}",
+ value_i, date_value);
+ }
col_data.emplace_back(v);
}
Review Comment:
[P1] Preserve a read path for DATE files written by the prior Doris encoder.
A Parquet or ORC file exported or committed to Iceberg before this change
stores `0000-01-01` as -719527 and `0000-02-28` as -719469 through the old
`daynr - 719528` encoder; the prior readers recovered both dates. This shared
decoder returns `0000-01-02` for the first and rejects or nulls the second, so
upgrading the BE changes results for existing files. This is the reverse
direction of the open v1/older-BE thread: fixing those readers does not fix
historical files on this reader. The release note asks for re-export, but
already committed files remain silently readable with wrong values. Provide an
explicit legacy-file read/migration path, with fixtures written by the old
encoder, before making the new interpretation the only default.
##########
be/src/format_v2/orc/orc_reader.cpp:
##########
@@ -2275,6 +2370,17 @@ Status OrcReader::get_aggregate_result(const
format::FileAggregateRequest& reque
_state->root_type->getSubtype(static_cast<uint64_t>(count_projection.local_id()));
DORIS_CHECK(count_type != nullptr);
+ std::vector<uint32_t> date_column_ids;
+ const auto collect_dates = [&](auto&& self, const ::orc::Type& type)
-> void {
+ if (type.getKind() == ::orc::TypeKind::DATE) {
+
date_column_ids.push_back(cast_set<uint32_t>(type.getColumnId()));
+ }
+ for (uint64_t i = 0; i < type.getSubtypeCount(); ++i) {
+ self(self, *type.getSubtype(i));
+ }
+ };
+ collect_dates(collect_dates, *count_type);
+
Review Comment:
[P1] Check the stripe-statistics column count before looking up nested DATE
IDs. For `COUNT(s)` on `struct<id:int,s:struct<d:date>>`, a stripe with
complete footer schema, only three metadata `colStats` entries (IDs 0-2), and
no `ROW_INDEX` streams passes the earlier checks and `getStripeStatistics`; the
prior top-level COUNT lookup of `s` (ID 2) stays in range. This new loop calls
`getColumnStatistics(3)` for `d`, while the pinned ORC SDK indexes its
three-element `colStats` vector without a bounds check, so malformed external
metadata can crash or corrupt the BE instead of falling back to row validation.
Check `column_id < stripe_statistics->getNumberOfColumns()` and return
`NotSupported` or corruption before the lookup; cover truncated nested DATE
statistics in a regression.
##########
be/src/format_v2/orc/orc_reader.cpp:
##########
@@ -1670,8 +1722,39 @@ Status OrcReader::_select_stripe_ranges_by_statistics() {
}
std::vector<int> sarg_needed_stripes;
+ std::set<uint64_t> unsafe_date_stripes;
try {
Review Comment:
[P1] Avoid loading row indexes for every stripe during DATE preflight. With
a projected DATE and a selective `id` SARG, this call runs on every split
stripe before `getNeedReadStripes` can discard safe stripes. The pinned ORC SDK
fetches each stripe footer and parses every `ROW_INDEX` stream, including
unprojected columns. A valid nonempty index in an otherwise pruned stripe whose
stream column ID is changed from 3 to 100 passes earlier footer checks but
reaches the SDK's unchecked `indexStats[stream.column()]` access, which can
corrupt memory or crash before Doris's exception fallback. Even valid remote
files pay index I/O and parsing for skipped stripes. Read DATE bounds from
metadata without loading row indexes; if this SDK path remains, validate the
stream column ID and cover the malformed pruned stripe and I/O counts.
--
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]
