Gabriel39 commented on code in PR #68649:
URL: https://github.com/apache/doris/pull/68649#discussion_r4150164519
##########
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:
Added an explicit offline migration path in 082429b00f2: [tool and
procedure](https://github.com/apache/doris/blob/082429b00f2392f8e8eabcee6e6a5a94a403e7aa/tools/legacy_date_migration/README.md).
It requires confirmation of the former Doris encoding, corrects only the
legacy ordinal window, handles nullable/nested DATE values, and publishes a
separate output without overwriting the source or an existing destination. The
release note now directs historical files through this migration before
consumers switch to the corrected reader.
Ten tool tests and six real FE/BE migration/read/export/read checks pass,
using files containing the exact legacy boundary ordinals for ORC and Parquet.
The checks include year zero, modern dates, maximum DATE and NULL. Existing ORC
v1 reading is documented as an optional migration route; actual Parquet testing
shows that switching scanners is not a reliable general solution. Iceberg
instructions require a new table or a supported transactional rewrite that
recomputes partition metadata, preserving old snapshots. No automatic origin
detection, v1 code change, or rolling-upgrade gate is claimed.
##########
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:
Fixed in 082429b00f2. DATE preflight now evaluates an invalid-ordinal SARG
against file-level stripe metadata through getNeedReadStripes, without calling
getStripeStatistics or loading stripe footers/ROW_INDEX streams. Missing or
inconclusive DATE statistics retain the stripe. The SDK cached evaluator is
consumed while its options remain alive, before restoring the query SARG.
The new malformed fixture changes only an unused ROW_INDEX column ID from 3
to 100. The previous implementation hits the SDK vector bounds assertion; the
fix safely prunes the stripe. A separate I/O regression also fails before the
fix (7 reads versus 2, 602 bytes versus 440) and now confirms equal read
calls/bytes with and without DATE projection. All 250 selected ASAN tests pass,
including the complete ORC suite and existing unsafe-DATE error/pruning tests.
##########
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:
Fixed in 082429b00f2. Each nested DATE statistics ID is checked against
getNumberOfColumns before the SDK lookup, returning NotSupported so COUNT falls
back to materialization. The neighboring COUNT-root and MIN/MAX statistics
lookups are bounded as well.
A new fixture retains the complete struct schema and data but removes the
nested DATE stripe-statistics entry and has no row indexes. It reproduces the
SDK vector bounds assertion before the fix. Afterward, metadata COUNT is
declined and ordinary reading returns both intact struct values. This
regression and all 250 selected ASAN tests pass.
--
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]
