Gabriel39 commented on code in PR #68649:
URL: https://github.com/apache/doris/pull/68649#discussion_r4140125879
##########
be/src/format_v2/orc/orc_reader.cpp:
##########
@@ -1502,6 +1522,37 @@ Status
OrcReader::_init_search_argument_from_local_filters() {
}
try {
+ // SARG can skip rows before DATE decoding detects an invalid ordinal.
File statistics
+ // must prove the selected DATE domains are representable before
enabling SDK pruning.
+ const auto dates_are_representable = [&](const auto& self,
+ const ::orc::Type& type) ->
bool {
+ if (type.getKind() == ::orc::TypeKind::DATE) {
+ const auto stats =
+
_state->reader->getColumnStatistics(cast_set<uint32_t>(type.getColumnId()));
+ if (stats == nullptr) {
+ return false;
+ }
+ if (stats->getNumberOfValues() == 0) {
+ return true;
+ }
+ segment_v2::ZoneMap zone_map;
+ return set_date_zone_map(*stats, &zone_map);
+ }
+ for (uint64_t child = 0; child < type.getSubtypeCount(); ++child) {
+ if (!self(self, *type.getSubtype(child))) {
+ return false;
+ }
+ }
+ return true;
+ };
+ for (const auto column_id : _state->read_columns) {
Review Comment:
Fixed in 768c4220031. The DATE SARG guard now uses
collect_projected_type_ids, matching includeTypes for partial projections.
Unread DATE siblings no longer disable pruning. The new partial-STRUCT
regression fails before the fix and passes afterward; its control projects the
invalid DATE and verifies that the conversion error is still preserved.
##########
be/src/format_v2/parquet/parquet_statistics.cpp:
##########
@@ -1661,6 +1661,14 @@ bool set_native_page_scalar_min_max(const
tparquet::ColumnIndex& column_index,
}
const auto min_value =
unaligned_load<ValueType>(column_index.min_values[page_idx].data());
const auto max_value =
unaligned_load<ValueType>(column_index.max_values[page_idx].data());
+ if constexpr (std::is_same_v<ValueType, int32_t>) {
+ if (remove_nullable(column_schema.type)->get_primitive_type() ==
TYPE_DATEV2 &&
+ !epoch_days_range_is_representable(min_value, max_value)) {
Review Comment:
Fixed in 768c4220031. An unsafe DATE page now produces unavailable
statistics for that page while allowing both page-index evaluators to continue
with other pages. Added a three-page regression covering IS NULL and range
predicates through single-slot and multi-slot OR evaluation. It fails before
the fix and passes afterward. All 245 related ASAN tests and clang-format 16
checks pass.
##########
be/src/core/data_type_serde/data_type_datev2_serde.cpp:
##########
@@ -217,13 +236,12 @@ Status DataTypeDateV2SerDe::write_column_to_arrow(const
IColumn& column, const N
const auto& col_data = static_cast<const ColumnDateV2&>(column).get_data();
auto& date32_builder = assert_cast<arrow::Date32Builder&>(*array_builder);
for (size_t i = start; i < end; ++i) {
- auto daynr = col_data[i].daynr() - date_threshold;
if (null_map && (*null_map)[i]) {
RETURN_IF_ERROR(checkArrowStatus(date32_builder.AppendNull(),
column, *array_builder));
} else {
- RETURN_IF_ERROR(
- checkArrowStatus(date32_builder.Append(cast_set<int,
int64_t, false>(daynr)),
- column, *array_builder));
+ RETURN_IF_ERROR(checkArrowStatus(
+
date32_builder.Append(daynr_to_epoch_days(col_data[i].daynr())), column,
Review Comment:
V1 reader changes and mixed-version/rolling-upgrade compatibility are
explicitly outside the requested scope of this PR. No changes are being made
for this finding. The PR description now states this scope, and this thread
remains open rather than being marked fixed.
--
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]
