From e7d100e81eaa69811936ede029978ee92acdfe4f Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 15:05:58 +0800 Subject: [PATCH 01/23] [fix](be) Preserve row groups when null count is missing ### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: FileScannerV2 treated a missing optional Parquet null_count as proof that a row group contained no NULL values. When min/max statistics were present, IS NULL predicates could therefore prune a row group that actually contained NULL rows. Treat unknown null counts conservatively as possibly containing NULL while retaining available min/max statistics, and cover the metadata combination with a unit test. ### Release note Fix incorrect IS NULL results for Parquet files whose writers omit null_count statistics. ### Check List (For Author) - Test: Unit Test - ParquetStatistics* BE unit tests - Behavior changed: Yes, row groups with unknown Parquet null counts are no longer discarded by IS NULL pruning - Does this need documentation: No --- .../format_v2/parquet/parquet_statistics.cpp | 2 +- .../parquet/parquet_statistics_test.cpp | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/be/src/format_v2/parquet/parquet_statistics.cpp b/be/src/format_v2/parquet/parquet_statistics.cpp index d0692a80205f3f..9b48875059eb58 100644 --- a/be/src/format_v2/parquet/parquet_statistics.cpp +++ b/be/src/format_v2/parquet/parquet_statistics.cpp @@ -701,7 +701,7 @@ ParquetColumnStatistics ParquetStatisticsUtils::TransformColumnStatistics( return result; } - result.has_null = statistics->HasNullCount() && statistics->null_count() > 0; + result.has_null = !statistics->HasNullCount() || statistics->null_count() > 0; result.has_not_null = statistics->num_values() > 0 || statistics->HasMinMax(); result.has_null_count = statistics->HasNullCount(); if (!result.has_not_null || !statistics->HasMinMax()) { diff --git a/be/test/format_v2/parquet/parquet_statistics_test.cpp b/be/test/format_v2/parquet/parquet_statistics_test.cpp index e5cdd8a87457e0..87243d26e302c8 100644 --- a/be/test/format_v2/parquet/parquet_statistics_test.cpp +++ b/be/test/format_v2/parquet/parquet_statistics_test.cpp @@ -416,6 +416,26 @@ TEST(ParquetStatisticsTransformTest, HandlesMissingStatisticsAndAllNullChunks) { EXPECT_FALSE(all_null_stats.has_min_max); } +TEST(ParquetStatisticsTransformTest, MissingNullCountConservativelyReportsPossibleNulls) { + auto table = arrow::Table::Make(arrow::schema({arrow::field("i", arrow::int32(), true)}), + {int32_array({1, std::nullopt, 3})}); + auto reader = make_reader(table, 3, false, true); + auto schema = build_file_schema(*reader); + auto file_statistics = reader->metadata()->RowGroup(0)->ColumnChunk(0)->statistics(); + auto statistics_without_null_count = ::parquet::MakeStatistics<::parquet::Int32Type>( + reader->metadata()->schema()->Column(0), file_statistics->EncodeMin(), + file_statistics->EncodeMax(), file_statistics->num_values(), 0, 0, true, false, false); + + const auto statistics = format::parquet::ParquetStatisticsUtils::TransformColumnStatistics( + *schema[0], statistics_without_null_count); + EXPECT_FALSE(statistics.has_null_count); + EXPECT_TRUE(statistics.has_null); + EXPECT_TRUE(statistics.has_not_null); + EXPECT_TRUE(statistics.has_min_max); + EXPECT_EQ(statistics.min_value.get(), 1); + EXPECT_EQ(statistics.max_value.get(), 3); +} + TEST(ParquetStatisticsPruningTest, ExprZonemapPredicatesAndNullPredicatesPruneRowGroups) { auto table = arrow::Table::Make(arrow::schema({arrow::field("i", arrow::int32(), true)}), {int32_array({std::nullopt, std::nullopt, 3, 4, 5, 6})}); From f3e632345f3e8a85aceb73114eb00eedc4766e88 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 14:36:48 +0800 Subject: [PATCH 02/23] [fix](be) Prevent DST timestamp statistics mispruning ### What problem does this PR solve? Issue Number: N/A Related PR: #64456 Problem Summary: FileScannerV2 converts UTC timestamp statistics into session-local DATETIMEV2 values before ZoneMap evaluation. Across a daylight-saving time rollback, that conversion is not monotonic, so converted metadata endpoints may exclude local values that occur inside the UTC interval. Parquet row-group and page-index pruning, and ORC statistics pruning, could therefore skip matching data. Detect backward timezone transitions within UTC statistics ranges and conservatively disable min/max pruning for those ranges while retaining statistics for monotonic ranges and TIMESTAMPTZ mappings. ### Release note Fix incorrect Parquet and ORC file scan pruning for UTC timestamps across daylight-saving time rollbacks. ### Check List (For Author) - Test: Unit Test - Remote ASAN BE UT: TimestampStatisticsTest.*, ParquetStatisticsTransformTest.*, and NewOrcReaderTest.AggregatePushdownTimestamp* (7 passed) - Behavior changed: Yes. Non-monotonic local timestamp statistics no longer prune rows or pages. - Does this need documentation: No --- be/src/format_v2/orc/orc_reader.cpp | 6 ++ .../format_v2/parquet/parquet_statistics.cpp | 43 ++++++++++++++ be/src/format_v2/timestamp_statistics.h | 59 +++++++++++++++++++ .../parquet/parquet_statistics_test.cpp | 28 +++++++++ .../format_v2/timestamp_statistics_test.cpp | 44 ++++++++++++++ 5 files changed, 180 insertions(+) create mode 100644 be/src/format_v2/timestamp_statistics.h create mode 100644 be/test/format_v2/timestamp_statistics_test.cpp diff --git a/be/src/format_v2/orc/orc_reader.cpp b/be/src/format_v2/orc/orc_reader.cpp index 32b8aed2a888db..6f9ea2e10aaaca 100644 --- a/be/src/format_v2/orc/orc_reader.cpp +++ b/be/src/format_v2/orc/orc_reader.cpp @@ -69,6 +69,7 @@ #include "exprs/vslot_ref.h" #include "format_v2/column_mapper.h" #include "format_v2/orc/orc_search_argument.h" +#include "format_v2/timestamp_statistics.h" #include "io/fs/file_reader.h" #include "runtime/exec_env.h" #include "runtime/runtime_profile.h" @@ -531,6 +532,11 @@ bool set_timestamp_zone_map(const ::orc::ColumnStatistics& statistics, timestamp_statistics->getMaximum(), timestamp_statistics->getMaximumNanos())); return true; } + if (!format::utc_timestamp_range_is_monotonic( + format::floor_epoch_seconds(timestamp_statistics->getMinimum(), 1000), + format::floor_epoch_seconds(timestamp_statistics->getMaximum(), 1000), timezone)) { + return false; + } zone_map->min_value = Field::create_field(datetime_v2_from_orc_millis( timestamp_statistics->getMinimum(), timestamp_statistics->getMinimumNanos(), timezone)); zone_map->max_value = Field::create_field(datetime_v2_from_orc_millis( diff --git a/be/src/format_v2/parquet/parquet_statistics.cpp b/be/src/format_v2/parquet/parquet_statistics.cpp index 9b48875059eb58..22d7d4bc168395 100644 --- a/be/src/format_v2/parquet/parquet_statistics.cpp +++ b/be/src/format_v2/parquet/parquet_statistics.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -45,6 +46,7 @@ #include "exprs/expr_zonemap_filter.h" #include "exprs/vexpr_context.h" #include "format_v2/parquet/parquet_column_schema.h" +#include "format_v2/timestamp_statistics.h" #include "runtime/runtime_profile.h" #include "storage/index/zone_map/zone_map_index.h" #include "storage/index/zone_map/zonemap_eval_context.h" @@ -118,6 +120,35 @@ bool set_decoded_field(const ParquetColumnSchema& column_schema, DecodedValueKin return read_decoded_field(column_schema, view, field, timezone).ok(); } +int64_t floor_timestamp_seconds(int64_t value, ParquetTimeUnit time_unit) { + int64_t units_per_second = 1; + switch (time_unit) { + case ParquetTimeUnit::MILLIS: + units_per_second = 1000; + break; + case ParquetTimeUnit::MICROS: + units_per_second = 1000000; + break; + case ParquetTimeUnit::NANOS: + units_per_second = 1000000000; + break; + default: + DORIS_CHECK(false); + } + return format::floor_epoch_seconds(value, units_per_second); +} + +bool timestamp_min_max_is_safe(const ParquetColumnSchema& column_schema, int64_t min_value, + int64_t max_value, const cctz::time_zone* timezone) { + if (!column_schema.type_descriptor.is_timestamp || + !column_schema.type_descriptor.timestamp_is_adjusted_to_utc || timezone == nullptr) { + return true; + } + return format::utc_timestamp_range_is_monotonic( + floor_timestamp_seconds(min_value, column_schema.type_descriptor.time_unit), + floor_timestamp_seconds(max_value, column_schema.type_descriptor.time_unit), *timezone); +} + template bool set_decoded_min_max(const std::shared_ptr<::parquet::Statistics>& statistics, const ParquetColumnSchema& column_schema, DecodedValueKind value_kind, @@ -125,6 +156,12 @@ bool set_decoded_min_max(const std::shared_ptr<::parquet::Statistics>& statistic const cctz::time_zone* timezone) { auto typed_statistics = std::static_pointer_cast<::parquet::TypedStatistics>(statistics); + if constexpr (std::is_same_v) { + if (!timestamp_min_max_is_safe(column_schema, typed_statistics->min(), + typed_statistics->max(), timezone)) { + return false; + } + } if (!set_decoded_field(column_schema, value_kind, typed_statistics->min(), &column_statistics->min_value, timezone) || !set_decoded_field(column_schema, value_kind, typed_statistics->max(), @@ -969,6 +1006,12 @@ bool set_page_decoded_min_max(const std::shared_ptr<::parquet::ColumnIndex>& col page_idx >= typed_index->max_values().size()) { return false; } + if constexpr (std::is_same_v) { + if (!timestamp_min_max_is_safe(column_schema, typed_index->min_values()[page_idx], + typed_index->max_values()[page_idx], timezone)) { + return false; + } + } if (!set_decoded_field(column_schema, value_kind, typed_index->min_values()[page_idx], &page_statistics->min_value, timezone) || !set_decoded_field(column_schema, value_kind, typed_index->max_values()[page_idx], diff --git a/be/src/format_v2/timestamp_statistics.h b/be/src/format_v2/timestamp_statistics.h new file mode 100644 index 00000000000000..6bfd861c05a9d7 --- /dev/null +++ b/be/src/format_v2/timestamp_statistics.h @@ -0,0 +1,59 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#pragma once + +#include + +#include + +#include "common/check.h" + +namespace doris::format { + +inline int64_t floor_epoch_seconds(int64_t value, int64_t units_per_second) { + DORIS_CHECK(units_per_second > 0); + auto seconds = value / units_per_second; + if (value < 0 && value % units_per_second != 0) { + --seconds; + } + return seconds; +} + +// UTC instants are ordered, but converting them to civil DATETIMEV2 values is not monotonic when +// a timezone moves its clock backward. Metadata min/max converted across such a transition cannot +// safely form a ZoneMap: the true civil minimum or maximum may occur inside the UTC interval. +inline bool utc_timestamp_range_is_monotonic(int64_t min_seconds, int64_t max_seconds, + const cctz::time_zone& timezone) { + DORIS_CHECK(min_seconds <= max_seconds); + auto current = cctz::time_point(cctz::seconds(min_seconds)); + const auto range_end = cctz::time_point(cctz::seconds(max_seconds)); + cctz::time_zone::civil_transition transition; + while (timezone.next_transition(current, &transition)) { + const auto transition_time = timezone.lookup(transition.to).trans; + if (transition_time > range_end) { + return true; + } + if (transition.to < transition.from) { + return false; + } + current = transition_time; + } + return true; +} + +} // namespace doris::format diff --git a/be/test/format_v2/parquet/parquet_statistics_test.cpp b/be/test/format_v2/parquet/parquet_statistics_test.cpp index 87243d26e302c8..b56773e7786552 100644 --- a/be/test/format_v2/parquet/parquet_statistics_test.cpp +++ b/be/test/format_v2/parquet/parquet_statistics_test.cpp @@ -392,6 +392,34 @@ TEST(ParquetStatisticsTransformTest, ConvertsMinMaxNullCountUnsignedStringAndTim EXPECT_LT(timestamp_stats.min_value, timestamp_stats.max_value); } +TEST(ParquetStatisticsTransformTest, DisablesUtcTimestampMinMaxAcrossDstRollback) { + constexpr int64_t MICROS_PER_SECOND = 1000000; + // America/New_York moved from UTC-04:00 to UTC-05:00 at 2021-11-07 06:00:00 UTC. + // Both UTC endpoints below map to 01:30 local time, while values inside the interval cover + // 01:00 through 01:59. Endpoint conversion therefore cannot represent the true local range. + auto table = arrow::Table::Make( + arrow::schema( + {arrow::field("ts", arrow::timestamp(arrow::TimeUnit::MICRO, "UTC"), false)}), + {timestamp_array({1636263000 * MICROS_PER_SECOND, 1636263900 * MICROS_PER_SECOND, + 1636266600 * MICROS_PER_SECOND})}); + auto reader = make_reader(table, 3, false, true); + auto schema = build_file_schema(*reader); + auto statistics = reader->metadata()->RowGroup(0)->ColumnChunk(0)->statistics(); + + cctz::time_zone new_york; + ASSERT_TRUE(cctz::load_time_zone("America/New_York", &new_york)); + const auto local_stats = format::parquet::ParquetStatisticsUtils::TransformColumnStatistics( + *schema[0], statistics, &new_york); + EXPECT_TRUE(local_stats.has_not_null); + EXPECT_FALSE(local_stats.has_min_max); + + auto utc = cctz::utc_time_zone(); + const auto utc_stats = format::parquet::ParquetStatisticsUtils::TransformColumnStatistics( + *schema[0], statistics, &utc); + EXPECT_TRUE(utc_stats.has_min_max); + EXPECT_LT(utc_stats.min_value, utc_stats.max_value); +} + TEST(ParquetStatisticsTransformTest, HandlesMissingStatisticsAndAllNullChunks) { auto no_stats_table = arrow::Table::Make( arrow::schema({arrow::field("i", arrow::int32(), true)}), {int32_array({1, 2, 3})}); diff --git a/be/test/format_v2/timestamp_statistics_test.cpp b/be/test/format_v2/timestamp_statistics_test.cpp new file mode 100644 index 00000000000000..1eae798a13cbe8 --- /dev/null +++ b/be/test/format_v2/timestamp_statistics_test.cpp @@ -0,0 +1,44 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "format_v2/timestamp_statistics.h" + +#include +#include + +namespace doris::format { + +TEST(TimestampStatisticsTest, DetectsBackwardTimezoneTransitionsInUtcRange) { + cctz::time_zone new_york; + ASSERT_TRUE(cctz::load_time_zone("America/New_York", &new_york)); + + // The 2021 fall transition at 06:00 UTC makes local civil time move backward by one hour. + EXPECT_FALSE(utc_timestamp_range_is_monotonic(1636263000, 1636266600, new_york)); + EXPECT_FALSE(utc_timestamp_range_is_monotonic(1636263000, 1636264800, new_york)); + EXPECT_TRUE(utc_timestamp_range_is_monotonic(1636264800, 1636266600, new_york)); + + // The 2021 spring transition at 07:00 UTC skips civil values but preserves ordering. + EXPECT_TRUE(utc_timestamp_range_is_monotonic(1615703400, 1615707000, new_york)); +} + +TEST(TimestampStatisticsTest, FloorsNegativeEpochFractions) { + EXPECT_EQ(floor_epoch_seconds(1001, 1000), 1); + EXPECT_EQ(floor_epoch_seconds(-1, 1000), -1); + EXPECT_EQ(floor_epoch_seconds(-1001, 1000), -2); +} + +} // namespace doris::format From 47c1be27caacb1f614385c3159888fb2522d3122 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 16:02:12 +0800 Subject: [PATCH 03/23] [fix](be) guard timestamp pruning across DST rollback Disable unsafe ORC timestamp SARG pushdown in civil years with backward timezone transitions, while preserving Parquet min/max pruning for TIMESTAMPTZ values that retain UTC ordering. Add ORC, Parquet, and timezone transition regression coverage. --- be/src/format_v2/orc/orc_search_argument.cpp | 32 ++++++- .../format_v2/parquet/parquet_statistics.cpp | 5 +- be/src/format_v2/timestamp_statistics.h | 5 +- be/test/format_v2/orc/orc_reader_test.cpp | 95 ++++++++++++++++++- .../parquet/parquet_statistics_test.cpp | 27 ++++++ .../format_v2/timestamp_statistics_test.cpp | 4 + 6 files changed, 161 insertions(+), 7 deletions(-) diff --git a/be/src/format_v2/orc/orc_search_argument.cpp b/be/src/format_v2/orc/orc_search_argument.cpp index 9c40a60ae85fa6..4766fa8304587a 100644 --- a/be/src/format_v2/orc/orc_search_argument.cpp +++ b/be/src/format_v2/orc/orc_search_argument.cpp @@ -45,6 +45,7 @@ #include "exprs/vliteral.h" #include "exprs/vslot_ref.h" #include "exprs/vtopn_pred.h" +#include "format_v2/timestamp_statistics.h" namespace doris::format::orc { namespace { @@ -597,20 +598,47 @@ std::optional date_time_literal_parts(const Field& field) std::optional<::orc::Literal> make_timestamp_literal(const Field& field, const cctz::time_zone& timezone) { + const auto civil_year_is_monotonic = [&](const cctz::civil_second& civil_seconds) { + // Localized file predicates may already have selected one side of a repeated civil time. + // Checking the whole civil year catches that lossy representation while retaining SARG + // pruning for years and zones without a backward transition. + const auto year_start = + cctz::convert(cctz::civil_second(civil_seconds.year(), 1, 1), timezone); + const auto next_year_start = + cctz::convert(cctz::civil_second(civil_seconds.year() + 1, 1, 1), timezone); + return format::utc_timestamp_range_is_monotonic(year_start.time_since_epoch().count(), + next_year_start.time_since_epoch().count(), + timezone); + }; switch (field.get_type()) { case TYPE_DATETIME: { const auto& datetime = field.get(); const cctz::civil_second civil_seconds(datetime.year(), datetime.month(), datetime.day(), datetime.hour(), datetime.minute(), datetime.second()); - return ::orc::Literal(cctz::convert(civil_seconds, timezone).time_since_epoch().count(), 0); + const auto lookup = timezone.lookup(civil_seconds); + // ORC SearchArguments accept one UTC timestamp literal. A repeated or skipped local civil + // time has no unique UTC representation, so pushing it down could prune a stripe that + // contains another valid interpretation. Keep such predicates for row-level evaluation. + if (!civil_year_is_monotonic(civil_seconds) || + lookup.kind != cctz::time_zone::civil_lookup::UNIQUE) { + return std::nullopt; + } + return ::orc::Literal(lookup.pre.time_since_epoch().count(), 0); } case TYPE_DATETIMEV2: { const auto& datetime = field.get(); const cctz::civil_second civil_seconds(datetime.year(), datetime.month(), datetime.day(), datetime.hour(), datetime.minute(), datetime.second()); - const auto seconds = cctz::convert(civil_seconds, timezone).time_since_epoch().count(); + const auto lookup = timezone.lookup(civil_seconds); + // See the DATETIME path above. The fractional part does not disambiguate a civil time + // repeated by a backward timezone transition. + if (!civil_year_is_monotonic(civil_seconds) || + lookup.kind != cctz::time_zone::civil_lookup::UNIQUE) { + return std::nullopt; + } + const auto seconds = lookup.pre.time_since_epoch().count(); const auto nanos = cast_set(datetime.microsecond() * 1000); return ::orc::Literal(seconds, nanos); } diff --git a/be/src/format_v2/parquet/parquet_statistics.cpp b/be/src/format_v2/parquet/parquet_statistics.cpp index 22d7d4bc168395..62be7ad649cd25 100644 --- a/be/src/format_v2/parquet/parquet_statistics.cpp +++ b/be/src/format_v2/parquet/parquet_statistics.cpp @@ -141,7 +141,10 @@ int64_t floor_timestamp_seconds(int64_t value, ParquetTimeUnit time_unit) { bool timestamp_min_max_is_safe(const ParquetColumnSchema& column_schema, int64_t min_value, int64_t max_value, const cctz::time_zone* timezone) { if (!column_schema.type_descriptor.is_timestamp || - !column_schema.type_descriptor.timestamp_is_adjusted_to_utc || timezone == nullptr) { + !column_schema.type_descriptor.timestamp_is_adjusted_to_utc || timezone == nullptr || + remove_nullable(column_schema.type)->get_primitive_type() == TYPE_TIMESTAMPTZ) { + // TIMESTAMPTZ keeps the original UTC ordering, so local civil-time rollback does not make + // its converted min/max non-monotonic. return true; } return format::utc_timestamp_range_is_monotonic( diff --git a/be/src/format_v2/timestamp_statistics.h b/be/src/format_v2/timestamp_statistics.h index 6bfd861c05a9d7..2bbe7b99b908f7 100644 --- a/be/src/format_v2/timestamp_statistics.h +++ b/be/src/format_v2/timestamp_statistics.h @@ -51,7 +51,10 @@ inline bool utc_timestamp_range_is_monotonic(int64_t min_seconds, int64_t max_se if (transition.to < transition.from) { return false; } - current = transition_time; + // Move past the transition that was just inspected. Some cctz implementations return the + // same transition again when queried at its exact instant, which would otherwise prevent + // us from seeing a later rollback in the requested range. + current = transition_time + cctz::seconds(1); } return true; } diff --git a/be/test/format_v2/orc/orc_reader_test.cpp b/be/test/format_v2/orc/orc_reader_test.cpp index 0e36498c41d9bf..a6f669e227410e 100644 --- a/be/test/format_v2/orc/orc_reader_test.cpp +++ b/be/test/format_v2/orc/orc_reader_test.cpp @@ -74,6 +74,7 @@ #include "runtime/runtime_state.h" #include "storage/segment/condition_cache.h" #include "storage/utils.h" +#include "util/timezone_utils.h" namespace doris { namespace { @@ -2455,6 +2456,49 @@ class NullableGreaterThanExpr final : public VExpr { const std::string _expr_name; }; +template +class NullableEqualsExpr final : public VExpr { +public: + using ColumnType = typename PrimitiveTypeTraits::ColumnType; + using ValueType = typename PrimitiveTypeTraits::CppType; + + NullableEqualsExpr(int column_id, DataTypePtr type, const Field& value, std::string column_name) + : VExpr(std::make_shared(), false), + _column_id(column_id), + _value(value.get()), + _expr_name("NullableEqualsExpr") { + _node_type = TExprNodeType::BINARY_PRED; + _opcode = TExprOpcode::EQ; + add_child(TableSlotRef::create_shared(column_id, column_id, -1, make_nullable(type), + std::move(column_name))); + add_child(TableLiteral::create_shared(std::move(type), value)); + } + + Status execute_column_impl(VExprContext* context, const Block* block, const Selector* selector, + size_t count, ColumnPtr& result_column) const override { + const auto& nullable_column = + assert_cast(*block->get_by_position(_column_id).column); + const auto& input = assert_cast(nullable_column.get_nested_column()); + auto result = ColumnUInt8::create(); + auto& result_data = result->get_data(); + result_data.resize(count); + for (size_t row = 0; row < count; ++row) { + const size_t input_row = selector == nullptr ? row : (*selector)[row]; + result_data[row] = !nullable_column.is_null_at(input_row) && + input.get_element(input_row) == _value; + } + result_column = std::move(result); + return Status::OK(); + } + + const std::string& expr_name() const override { return _expr_name; } + +private: + const int _column_id; + const ValueType _value; + const std::string _expr_name; +}; + template class NullableInExpr final : public VExpr { public: @@ -4036,7 +4080,9 @@ void write_multi_stripe_orc_sarg_types_file(const std::string& file_path) { out.write(memory_stream.getData(), static_cast(memory_stream.getLength())); } -void write_multi_stripe_orc_timestamp_instant_sarg_file(const std::string& file_path) { +void write_multi_stripe_orc_timestamp_instant_sarg_file( + const std::string& file_path, int64_t first_timestamp_second = 0, + int64_t second_timestamp_second = 1609459200) { auto type = std::unique_ptr<::orc::Type>(::orc::Type::buildTypeFromString( "struct")); @@ -4068,8 +4114,8 @@ void write_multi_stripe_orc_timestamp_instant_sarg_file(const std::string& file_ writer->add(*batch); }; - add_batch(0); - add_batch(1609459200); + add_batch(first_timestamp_second); + add_batch(second_timestamp_second); writer->close(); std::ofstream out(file_path, std::ios::binary); @@ -8327,6 +8373,49 @@ TEST_F(NewOrcReaderTest, SargTimestampInstantConjunctUsesSessionTimezone) { EXPECT_EQ(reader->reader_statistics().filtered_group_rows, 200); } +TEST_F(NewOrcReaderTest, SargTimestampInstantRepeatedCivilTimeDoesNotPruneStripes) { + const auto multi_stripe_file_path = + (_test_dir / "sarg_timestamp_instant_dst_rollback.orc").string(); + // These UTC instants both decode to 2021-11-07 01:30:00.123 in America/New_York, + // once before and once after the UTC-04:00 to UTC-05:00 rollback. + write_multi_stripe_orc_timestamp_instant_sarg_file(multi_stripe_file_path, 1636263000, + 1636266600); + ASSERT_EQ(get_orc_stripe_count(multi_stripe_file_path), 2); + + auto reader = create_reader_for_path(multi_stripe_file_path); + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + TimezoneUtils::load_timezones_to_cache(); + state.set_timezone("America/New_York"); + ASSERT_TRUE(reader->init(&state).ok()); + + std::vector schema; + ASSERT_TRUE(reader->get_schema(&schema).ok()); + ASSERT_EQ(schema.size(), 2); + + const auto literal = + Field::create_field(make_datetime_v2(2021, 11, 7, 1, 30, 0, 123000)); + auto request = std::make_shared(); + request->predicate_columns = {field_projection(0)}; + request->conjuncts.push_back( + VExprContext::create_shared(std::make_shared>( + 0, remove_nullable(schema[0].type), literal, "timestamp_instant_col"))); + ASSERT_TRUE(reader->open(request).ok()); + + bool eof = false; + size_t result_rows = 0; + while (!eof) { + Block block = build_file_block({schema[0]}); + size_t rows = 0; + ASSERT_TRUE(reader->get_block(&block, &rows, &eof).ok()); + result_rows += rows; + } + + EXPECT_EQ(result_rows, 2); + EXPECT_EQ(reader->reader_statistics().filtered_row_groups, 0); + EXPECT_EQ(reader->reader_statistics().filtered_row_groups_by_min_max, 0); + EXPECT_EQ(reader->reader_statistics().filtered_group_rows, 0); +} + TEST_F(NewOrcReaderTest, SargTimestampLowerPrecisionCastDoesNotPruneStripes) { const auto multi_stripe_file_path = (_test_dir / "sarg_timestamp_lower_precision_cast.orc").string(); diff --git a/be/test/format_v2/parquet/parquet_statistics_test.cpp b/be/test/format_v2/parquet/parquet_statistics_test.cpp index b56773e7786552..b4712181f94294 100644 --- a/be/test/format_v2/parquet/parquet_statistics_test.cpp +++ b/be/test/format_v2/parquet/parquet_statistics_test.cpp @@ -36,6 +36,7 @@ #include "core/data_type/data_type_number.h" #include "core/data_type/data_type_string.h" +#include "core/data_type/data_type_timestamptz.h" #include "core/field.h" #include "exprs/vexpr.h" #include "exprs/vexpr_context.h" @@ -420,6 +421,32 @@ TEST(ParquetStatisticsTransformTest, DisablesUtcTimestampMinMaxAcrossDstRollback EXPECT_LT(utc_stats.min_value, utc_stats.max_value); } +TEST(ParquetStatisticsTransformTest, KeepsTimestampTzMinMaxAcrossDstRollback) { + constexpr int64_t MICROS_PER_SECOND = 1000000; + auto table = arrow::Table::Make( + arrow::schema( + {arrow::field("ts", arrow::timestamp(arrow::TimeUnit::MICRO, "UTC"), false)}), + {timestamp_array({1636263000 * MICROS_PER_SECOND, 1636266600 * MICROS_PER_SECOND})}); + auto reader = make_reader(table, 2, false, true); + auto schema = build_file_schema(*reader); + auto statistics = reader->metadata()->RowGroup(0)->ColumnChunk(0)->statistics(); + + // This is the effective type produced by enable_mapping_timestamp_tz. The physical timestamp + // flags intentionally remain adjusted-to-UTC so decoding can preserve the source semantics. + schema[0]->type = std::make_shared(6); + schema[0]->type_descriptor.doris_type = schema[0]->type; + + cctz::time_zone new_york; + ASSERT_TRUE(cctz::load_time_zone("America/New_York", &new_york)); + const auto timestamp_tz_stats = + format::parquet::ParquetStatisticsUtils::TransformColumnStatistics( + *schema[0], statistics, &new_york); + EXPECT_TRUE(timestamp_tz_stats.has_min_max); + EXPECT_EQ(timestamp_tz_stats.min_value.get_type(), TYPE_TIMESTAMPTZ); + EXPECT_EQ(timestamp_tz_stats.max_value.get_type(), TYPE_TIMESTAMPTZ); + EXPECT_LT(timestamp_tz_stats.min_value, timestamp_tz_stats.max_value); +} + TEST(ParquetStatisticsTransformTest, HandlesMissingStatisticsAndAllNullChunks) { auto no_stats_table = arrow::Table::Make( arrow::schema({arrow::field("i", arrow::int32(), true)}), {int32_array({1, 2, 3})}); diff --git a/be/test/format_v2/timestamp_statistics_test.cpp b/be/test/format_v2/timestamp_statistics_test.cpp index 1eae798a13cbe8..8fd6e206086206 100644 --- a/be/test/format_v2/timestamp_statistics_test.cpp +++ b/be/test/format_v2/timestamp_statistics_test.cpp @@ -31,6 +31,10 @@ TEST(TimestampStatisticsTest, DetectsBackwardTimezoneTransitionsInUtcRange) { EXPECT_FALSE(utc_timestamp_range_is_monotonic(1636263000, 1636264800, new_york)); EXPECT_TRUE(utc_timestamp_range_is_monotonic(1636264800, 1636266600, new_york)); + // A range beginning before the spring-forward transition must continue scanning and find the + // later rollback in the same year. + EXPECT_FALSE(utc_timestamp_range_is_monotonic(1609477200, 1641013200, new_york)); + // The 2021 spring transition at 07:00 UTC skips civil values but preserves ordering. EXPECT_TRUE(utc_timestamp_range_is_monotonic(1615703400, 1615707000, new_york)); } From 3249db1cd6d27faf240806af878c085d0bb9136a Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 11:40:03 +0800 Subject: [PATCH 04/23] [fix](be) Scope Parquet bloom cache to row groups ### What problem does this PR solve? Issue Number: N/A Related PR: N/A Problem Summary: Parquet row-group pruning cached Bloom filters only by leaf column id. After the first row group loaded a column Bloom filter, later row groups reused that filter and could be incorrectly pruned when a matching value existed only outside the first row group. Scope cache entries by both row group and leaf column so absence checks always use the current row group's Bloom filter. Add a parquet-mr fixture with two row groups and real Bloom data plus an enabled/disabled differential unit test. ### Release note Fix incorrect Parquet Bloom filter pruning that could silently omit rows from later row groups. ### Check List (For Author) - Test: Unit Test - ParquetStatisticsPruningTest.BloomFilterCacheIsScopedToRowGroupAndColumn - Behavior changed: Yes, Parquet Bloom pruning now uses each row group's own filter - Does this need documentation: No --- .../format_v2/parquet/parquet_statistics.cpp | 17 ++++--- .../multi_row_group_bloom_filter.parquet | Bin 0 -> 534 bytes .../parquet/parquet_statistics_test.cpp | 44 ++++++++++++++++++ 3 files changed, 54 insertions(+), 7 deletions(-) create mode 100644 be/test/exec/test_data/parquet_scanner/multi_row_group_bloom_filter.parquet diff --git a/be/src/format_v2/parquet/parquet_statistics.cpp b/be/src/format_v2/parquet/parquet_statistics.cpp index 62be7ad649cd25..de5bb145c8490a 100644 --- a/be/src/format_v2/parquet/parquet_statistics.cpp +++ b/be/src/format_v2/parquet/parquet_statistics.cpp @@ -437,30 +437,33 @@ bool bloom_filter_excludes(const ParquetColumnSchema& column_schema, int slot_in } struct RowGroupBloomFilterCache { + using CacheKey = std::pair; + ::parquet::BloomFilterReader* bloom_filter_reader = nullptr; - std::map> column_bloom_filters; - std::set loaded_columns; + std::map> column_bloom_filters; + std::set loaded_columns; ::parquet::BloomFilter* get(int row_group_idx, int leaf_column_id, ParquetPruningStats* pruning_stats) { if (bloom_filter_reader == nullptr || leaf_column_id < 0) { return nullptr; } - if (loaded_columns.find(leaf_column_id) == loaded_columns.end()) { - loaded_columns.insert(leaf_column_id); + const CacheKey cache_key {row_group_idx, leaf_column_id}; + if (loaded_columns.find(cache_key) == loaded_columns.end()) { + loaded_columns.insert(cache_key); try { std::shared_ptr<::parquet::RowGroupBloomFilterReader> row_group_reader; if (pruning_stats != nullptr) { SCOPED_RAW_TIMER(&pruning_stats->bloom_filter_read_time); row_group_reader = bloom_filter_reader->RowGroup(row_group_idx); if (row_group_reader != nullptr) { - column_bloom_filters[leaf_column_id] = + column_bloom_filters[cache_key] = row_group_reader->GetColumnBloomFilter(leaf_column_id); } } else { row_group_reader = bloom_filter_reader->RowGroup(row_group_idx); if (row_group_reader != nullptr) { - column_bloom_filters[leaf_column_id] = + column_bloom_filters[cache_key] = row_group_reader->GetColumnBloomFilter(leaf_column_id); } } @@ -470,7 +473,7 @@ struct RowGroupBloomFilterCache { return nullptr; } } - auto it = column_bloom_filters.find(leaf_column_id); + auto it = column_bloom_filters.find(cache_key); return it == column_bloom_filters.end() ? nullptr : it->second.get(); } }; diff --git a/be/test/exec/test_data/parquet_scanner/multi_row_group_bloom_filter.parquet b/be/test/exec/test_data/parquet_scanner/multi_row_group_bloom_filter.parquet new file mode 100644 index 0000000000000000000000000000000000000000..6b9c77842582ad9b5b8b49055afb65863d3ac7be GIT binary patch literal 534 zcmZuv%SyvQ6g`O%X5k`YXGkE65U^CJp|RFzDWO6a>Z){G6q06&fws}KDk6f23thSM zBm4$;euw|z(mPsF(79aZKF*nYhMDeeFW}%N+56l5w!;m|7-xV6KqsGfA739{^k40r z8qg%S9l>}@5Rjq{k5t6ljstu<3^H0vXy0H_MZg_gwV669f8b0_Wg0XSgd%DiCd@V| z(qP^YT(_%x3%&e;>lClT(z6tT*;U&q!=dgfn5~K&!VoRdDzkluZb4v(XN@0`HgT)* z5~yjEnP`=i_vwY$Peh~UKx00M?qtqS7gW4e6+-lS}WKgdsGTGVKj>F+r a9Lh$RG=f^P8HRDNW~ #include #include +#include #include #include @@ -271,6 +272,14 @@ VExprContextSPtrs bloom_conjuncts(DataTypePtr data_type, std::vector valu std::make_shared(0, std::move(data_type), std::move(values)))}; } +format::FileScanRequest request_with_bloom_conjunct(DataTypePtr data_type, + std::vector values) { + format::FileScanRequest request; + request.local_positions.emplace(format::LocalColumnId(0), format::LocalIndex(0)); + request.conjuncts = bloom_conjuncts(std::move(data_type), std::move(values)); + return request; +} + void add_bloom_field(segment_v2::BlockSplitBloomFilter* bloom_filter, const Field& value, PrimitiveType type) { DORIS_CHECK(bloom_filter != nullptr); @@ -597,6 +606,41 @@ TEST(ParquetStatisticsPruningTest, VExprUsesDictionaryAndMissingBloomKeepsRows) EXPECT_EQ(pruning_stats.filtered_row_groups_by_bloom_filter, 0); } +TEST(ParquetStatisticsPruningTest, BloomFilterCacheIsScopedToRowGroupAndColumn) { + auto input = arrow::io::ReadableFile::Open( + "./be/test/exec/test_data/parquet_scanner/multi_row_group_bloom_filter.parquet"); + ASSERT_TRUE(input.ok()); + auto reader = ::parquet::ParquetFileReader::Open(*input); + ASSERT_EQ(reader->metadata()->num_row_groups(), 2); + auto& bloom_filter_reader = reader->GetBloomFilterReader(); + for (int row_group_idx = 0; row_group_idx < 2; ++row_group_idx) { + auto row_group_reader = bloom_filter_reader.RowGroup(row_group_idx); + ASSERT_NE(row_group_reader, nullptr); + ASSERT_NE(row_group_reader->GetColumnBloomFilter(0), nullptr); + } + auto schema = build_file_schema(*reader); + + std::vector selected; + format::parquet::ParquetPruningStats pruning_stats; + auto request = request_with_bloom_conjunct(std::make_shared(), + {Field::create_field(12345)}); + ASSERT_TRUE(format::parquet::select_row_groups_by_metadata(*reader->metadata(), reader.get(), + schema, request, nullptr, &selected, + false, &pruning_stats) + .ok()); + EXPECT_EQ(selected, std::vector({0, 1})); + EXPECT_EQ(pruning_stats.filtered_row_groups_by_bloom_filter, 0); + + selected.clear(); + pruning_stats = {}; + ASSERT_TRUE(format::parquet::select_row_groups_by_metadata(*reader->metadata(), reader.get(), + schema, request, nullptr, &selected, + true, &pruning_stats) + .ok()); + EXPECT_EQ(selected, std::vector({1})); + EXPECT_EQ(pruning_stats.filtered_row_groups_by_bloom_filter, 1); +} + TEST(ParquetBloomFilterPruningTest, VExprEqPrunesAbsentIntValue) { auto data_type = std::make_shared(); auto bloom_filter = bloom_filter_for_fields( From d26baa20c8dc8ae61301cf0f626f52d884a16633 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 13:20:12 +0800 Subject: [PATCH 05/23] [fix](be) Ignore NaN Parquet min max statistics Issue Number: N/A Related PR: N/A Problem Summary: Parquet FLOAT and DOUBLE min/max statistics containing NaN were treated as valid by FileScannerV2. Doris comparisons can then interpret NaN as equal to predicate bounds, causing range and inequality predicates to incorrectly prune row groups or pages that contain matching finite values. Reject footer and ColumnIndex min/max statistics when either floating-point bound is NaN, while preserving null-count information and finite statistics. Fix missing rows when Parquet floating-point min/max statistics contain NaN. - Test: Unit Test - ParquetStatisticsTransformTest targeted NaN footer and ColumnIndex tests - Behavior changed: Yes, NaN floating-point min/max statistics are ignored for pruning. - Does this need documentation: No --- .../format_v2/parquet/parquet_statistics.cpp | 49 +++++-- be/src/format_v2/parquet/parquet_statistics.h | 7 + .../parquet/parquet_statistics_test.cpp | 121 ++++++++++++++++++ 3 files changed, 163 insertions(+), 14 deletions(-) diff --git a/be/src/format_v2/parquet/parquet_statistics.cpp b/be/src/format_v2/parquet/parquet_statistics.cpp index de5bb145c8490a..fe291a2010a495 100644 --- a/be/src/format_v2/parquet/parquet_statistics.cpp +++ b/be/src/format_v2/parquet/parquet_statistics.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -152,6 +153,15 @@ bool timestamp_min_max_is_safe(const ParquetColumnSchema& column_schema, int64_t floor_timestamp_seconds(max_value, column_schema.type_descriptor.time_unit), *timezone); } +template +bool valid_min_max(const NativeType& min_value, const NativeType& max_value) { + if constexpr (std::is_floating_point_v) { + // Parquet requires readers to ignore min/max statistics if either bound is NaN. + return !std::isnan(min_value) && !std::isnan(max_value); + } + return true; +} + template bool set_decoded_min_max(const std::shared_ptr<::parquet::Statistics>& statistics, const ParquetColumnSchema& column_schema, DecodedValueKind value_kind, @@ -159,16 +169,18 @@ bool set_decoded_min_max(const std::shared_ptr<::parquet::Statistics>& statistic const cctz::time_zone* timezone) { auto typed_statistics = std::static_pointer_cast<::parquet::TypedStatistics>(statistics); + const auto& min_value = typed_statistics->min(); + const auto& max_value = typed_statistics->max(); if constexpr (std::is_same_v) { - if (!timestamp_min_max_is_safe(column_schema, typed_statistics->min(), - typed_statistics->max(), timezone)) { + if (!timestamp_min_max_is_safe(column_schema, min_value, max_value, timezone)) { return false; } } - if (!set_decoded_field(column_schema, value_kind, typed_statistics->min(), - &column_statistics->min_value, timezone) || - !set_decoded_field(column_schema, value_kind, typed_statistics->max(), - &column_statistics->max_value, timezone)) { + if (!valid_min_max(min_value, max_value) || + !set_decoded_field(column_schema, value_kind, min_value, &column_statistics->min_value, + timezone) || + !set_decoded_field(column_schema, value_kind, max_value, &column_statistics->max_value, + timezone)) { return false; } return true; @@ -1012,16 +1024,18 @@ bool set_page_decoded_min_max(const std::shared_ptr<::parquet::ColumnIndex>& col page_idx >= typed_index->max_values().size()) { return false; } + const auto& min_value = typed_index->min_values()[page_idx]; + const auto& max_value = typed_index->max_values()[page_idx]; if constexpr (std::is_same_v) { - if (!timestamp_min_max_is_safe(column_schema, typed_index->min_values()[page_idx], - typed_index->max_values()[page_idx], timezone)) { + if (!timestamp_min_max_is_safe(column_schema, min_value, max_value, timezone)) { return false; } } - if (!set_decoded_field(column_schema, value_kind, typed_index->min_values()[page_idx], - &page_statistics->min_value, timezone) || - !set_decoded_field(column_schema, value_kind, typed_index->max_values()[page_idx], - &page_statistics->max_value, timezone)) { + if (!valid_min_max(min_value, max_value) || + !set_decoded_field(column_schema, value_kind, min_value, &page_statistics->min_value, + timezone) || + !set_decoded_field(column_schema, value_kind, max_value, &page_statistics->max_value, + timezone)) { return false; } page_statistics->has_min_max = true; @@ -1255,8 +1269,8 @@ bool select_ranges_for_expr_zonemap( const auto page_count = offset_index->page_locations().size(); for (size_t page_idx = 0; page_idx < page_count; ++page_idx) { ParquetColumnStatistics page_statistics; - if (!build_page_statistics(column_index, *column_schema, page_idx, &page_statistics, - timezone)) { + if (!ParquetStatisticsUtils::TransformColumnIndexStatistics( + column_index, *column_schema, page_idx, &page_statistics, timezone)) { ranges->clear(); return false; } @@ -1405,6 +1419,13 @@ void build_page_skip_plans(const std::shared_ptr<::parquet::RowGroupPageIndexRea } // namespace +bool ParquetStatisticsUtils::TransformColumnIndexStatistics( + const std::shared_ptr<::parquet::ColumnIndex>& column_index, + const ParquetColumnSchema& column_schema, size_t page_idx, + ParquetColumnStatistics* page_statistics, const cctz::time_zone* timezone) { + return build_page_statistics(column_index, column_schema, page_idx, page_statistics, timezone); +} + Status select_row_group_ranges_by_page_index( ::parquet::ParquetFileReader* file_reader, const std::vector>& file_schema, diff --git a/be/src/format_v2/parquet/parquet_statistics.h b/be/src/format_v2/parquet/parquet_statistics.h index 818d645052ca5d..88f0dd2e2347ae 100644 --- a/be/src/format_v2/parquet/parquet_statistics.h +++ b/be/src/format_v2/parquet/parquet_statistics.h @@ -15,6 +15,7 @@ #pragma once +#include #include #include #include @@ -30,6 +31,7 @@ namespace parquet { class BloomFilter; +class ColumnIndex; class FileMetaData; class ParquetFileReader; class Statistics; @@ -124,6 +126,11 @@ struct ParquetStatisticsUtils { const std::shared_ptr<::parquet::Statistics>& statistics, const cctz::time_zone* timezone = nullptr); + static bool TransformColumnIndexStatistics( + const std::shared_ptr<::parquet::ColumnIndex>& column_index, + const ParquetColumnSchema& column_schema, size_t page_idx, + ParquetColumnStatistics* page_statistics, const cctz::time_zone* timezone = nullptr); + static bool BloomFilterExcludes(const ParquetColumnSchema& column_schema, int slot_index, const VExprContextSPtrs& conjuncts, const ::parquet::BloomFilter& bloom_filter); diff --git a/be/test/format_v2/parquet/parquet_statistics_test.cpp b/be/test/format_v2/parquet/parquet_statistics_test.cpp index 470ee2e98b98f0..cd931d544450f9 100644 --- a/be/test/format_v2/parquet/parquet_statistics_test.cpp +++ b/be/test/format_v2/parquet/parquet_statistics_test.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -77,6 +78,27 @@ std::shared_ptr uint32_array(const std::vector& values) return finish_array(&builder); } +std::shared_ptr float_array(const std::vector& values) { + arrow::FloatBuilder builder; + for (const auto value : values) { + EXPECT_TRUE(builder.Append(value).ok()); + } + return finish_array(&builder); +} + +std::shared_ptr double_array(const std::vector& values) { + arrow::DoubleBuilder builder; + for (const auto value : values) { + EXPECT_TRUE(builder.Append(value).ok()); + } + return finish_array(&builder); +} + +template +std::string encoded_value(const NativeType& value) { + return {reinterpret_cast(&value), sizeof(value)}; +} + std::shared_ptr string_array(const std::vector& values) { arrow::StringBuilder builder; for (const auto& value : values) { @@ -130,6 +152,37 @@ std::vector> build_file_sc return file_schema; } +template +class TestColumnIndex final : public ::parquet::TypedColumnIndex { +public: + using NativeType = typename ParquetDType::c_type; + + TestColumnIndex(NativeType min_value, NativeType max_value) + : _min_values {min_value}, _max_values {max_value} {} + + const std::vector& null_pages() const override { return _null_pages; } + const std::vector& encoded_min_values() const override { return _encoded_values; } + const std::vector& encoded_max_values() const override { return _encoded_values; } + ::parquet::BoundaryOrder::type boundary_order() const override { + return ::parquet::BoundaryOrder::Unordered; + } + bool has_null_counts() const override { return true; } + const std::vector& null_counts() const override { return _null_counts; } + const std::vector& non_null_page_indices() const override { + return _non_null_page_indices; + } + const std::vector& min_values() const override { return _min_values; } + const std::vector& max_values() const override { return _max_values; } + +private: + const std::vector _null_pages {false}; + const std::vector _encoded_values; + const std::vector _null_counts {0}; + const std::vector _non_null_page_indices {0}; + const std::vector _min_values; + const std::vector _max_values; +}; + class Int32ZoneMapExpr final : public VExpr { public: enum class Op { GE, GT, IS_NULL, IS_NOT_NULL }; @@ -500,6 +553,74 @@ TEST(ParquetStatisticsTransformTest, MissingNullCountConservativelyReportsPossib EXPECT_EQ(statistics.max_value.get(), 3); } +TEST(ParquetStatisticsTransformTest, IgnoresNaNFloatAndDoubleMinMax) { + auto table = arrow::Table::Make(arrow::schema({arrow::field("f", arrow::float32(), false), + arrow::field("d", arrow::float64(), false)}), + {float_array({1.0F, 2.0F}), double_array({1.0, 2.0})}); + auto reader = make_reader(table, 2, false, true); + auto schema = build_file_schema(*reader); + + const float float_nan = std::numeric_limits::quiet_NaN(); + const float float_max = 2.0F; + auto float_stats = ::parquet::MakeStatistics<::parquet::FloatType>( + schema[0]->descriptor, encoded_value(float_nan), encoded_value(float_max), 2, 0, 0, + true, true, false); + const auto converted_float = format::parquet::ParquetStatisticsUtils::TransformColumnStatistics( + *schema[0], float_stats); + EXPECT_FALSE(converted_float.has_min_max); + EXPECT_TRUE(converted_float.has_not_null); + + const double double_nan = std::numeric_limits::quiet_NaN(); + const double double_min = 1.0; + auto double_stats = ::parquet::MakeStatistics<::parquet::DoubleType>( + schema[1]->descriptor, encoded_value(double_min), encoded_value(double_nan), 2, 0, 0, + true, true, false); + const auto converted_double = + format::parquet::ParquetStatisticsUtils::TransformColumnStatistics(*schema[1], + double_stats); + EXPECT_FALSE(converted_double.has_min_max); + EXPECT_TRUE(converted_double.has_not_null); + + const double double_max = 2.0; + auto finite_stats = ::parquet::MakeStatistics<::parquet::DoubleType>( + schema[1]->descriptor, encoded_value(double_min), encoded_value(double_max), 2, 0, 0, + true, true, false); + const auto converted_finite = + format::parquet::ParquetStatisticsUtils::TransformColumnStatistics(*schema[1], + finite_stats); + EXPECT_TRUE(converted_finite.has_min_max); +} + +TEST(ParquetStatisticsTransformTest, IgnoresNaNFloatAndDoubleColumnIndexMinMax) { + auto table = arrow::Table::Make(arrow::schema({arrow::field("f", arrow::float32(), false), + arrow::field("d", arrow::float64(), false)}), + {float_array({1.0F, 2.0F}), double_array({1.0, 2.0})}); + auto reader = make_reader(table, 2, false, true); + auto schema = build_file_schema(*reader); + + auto float_index = std::make_shared>( + 1.0F, std::numeric_limits::quiet_NaN()); + format::parquet::ParquetColumnStatistics float_page_stats; + EXPECT_FALSE(format::parquet::ParquetStatisticsUtils::TransformColumnIndexStatistics( + float_index, *schema[0], 0, &float_page_stats)); + EXPECT_FALSE(float_page_stats.has_min_max); + EXPECT_TRUE(float_page_stats.has_not_null); + + auto double_index = std::make_shared>( + std::numeric_limits::quiet_NaN(), 2.0); + format::parquet::ParquetColumnStatistics double_page_stats; + EXPECT_FALSE(format::parquet::ParquetStatisticsUtils::TransformColumnIndexStatistics( + double_index, *schema[1], 0, &double_page_stats)); + EXPECT_FALSE(double_page_stats.has_min_max); + EXPECT_TRUE(double_page_stats.has_not_null); + + auto finite_index = std::make_shared>(1.0, 2.0); + format::parquet::ParquetColumnStatistics finite_page_stats; + EXPECT_TRUE(format::parquet::ParquetStatisticsUtils::TransformColumnIndexStatistics( + finite_index, *schema[1], 0, &finite_page_stats)); + EXPECT_TRUE(finite_page_stats.has_min_max); +} + TEST(ParquetStatisticsPruningTest, ExprZonemapPredicatesAndNullPredicatesPruneRowGroups) { auto table = arrow::Table::Make(arrow::schema({arrow::field("i", arrow::int32(), true)}), {int32_array({std::nullopt, std::nullopt, 3, 4, 5, 6})}); From b5a75acfadd8d409a17303d9c863f7c0d65c670d Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 14:25:18 +0800 Subject: [PATCH 06/23] [fix](be) Keep page index pruning after NaN statistics Issue Number: N/A Related PR: #65493 Problem Summary: A NaN min/max in one Parquet ColumnIndex page was reported as a transform failure, which disabled page-index pruning for the entire slot. Treat NaN bounds as unavailable only for that page so it is kept conservatively while later pages with finite bounds remain eligible for pruning. Preserve Parquet page-index pruning for finite pages when another page has NaN statistics. - Test: Unit Test - ParquetStatisticsTransformTest.* (4 tests passed) - Behavior changed: Yes. NaN page bounds no longer disable pruning for other pages in the slot. - Does this need documentation: No --- .../format_v2/parquet/parquet_statistics.cpp | 9 ++++- .../parquet/parquet_statistics_test.cpp | 40 ++++++++++++++++--- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/be/src/format_v2/parquet/parquet_statistics.cpp b/be/src/format_v2/parquet/parquet_statistics.cpp index fe291a2010a495..31f5fca2048151 100644 --- a/be/src/format_v2/parquet/parquet_statistics.cpp +++ b/be/src/format_v2/parquet/parquet_statistics.cpp @@ -1031,8 +1031,13 @@ bool set_page_decoded_min_max(const std::shared_ptr<::parquet::ColumnIndex>& col return false; } } - if (!valid_min_max(min_value, max_value) || - !set_decoded_field(column_schema, value_kind, min_value, &page_statistics->min_value, + if (!valid_min_max(min_value, max_value)) { + // A NaN invalidates only this page's bounds, not the ColumnIndex itself. Keep the page + // conservatively by returning usable null-count statistics with has_min_max=false, while + // allowing later pages with finite bounds to remain eligible for pruning. + return true; + } + if (!set_decoded_field(column_schema, value_kind, min_value, &page_statistics->min_value, timezone) || !set_decoded_field(column_schema, value_kind, max_value, &page_statistics->max_value, timezone)) { diff --git a/be/test/format_v2/parquet/parquet_statistics_test.cpp b/be/test/format_v2/parquet/parquet_statistics_test.cpp index cd931d544450f9..d3cdd3bf32c365 100644 --- a/be/test/format_v2/parquet/parquet_statistics_test.cpp +++ b/be/test/format_v2/parquet/parquet_statistics_test.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include "core/data_type/data_type_number.h" @@ -158,7 +159,19 @@ class TestColumnIndex final : public ::parquet::TypedColumnIndex { using NativeType = typename ParquetDType::c_type; TestColumnIndex(NativeType min_value, NativeType max_value) - : _min_values {min_value}, _max_values {max_value} {} + : TestColumnIndex(std::vector {min_value}, + std::vector {max_value}) {} + + TestColumnIndex(std::vector min_values, std::vector max_values) + : _null_pages(min_values.size(), false), + _null_counts(min_values.size(), 0), + _min_values(std::move(min_values)), + _max_values(std::move(max_values)) { + EXPECT_EQ(_min_values.size(), _max_values.size()); + for (size_t page_idx = 0; page_idx < _min_values.size(); ++page_idx) { + _non_null_page_indices.push_back(static_cast(page_idx)); + } + } const std::vector& null_pages() const override { return _null_pages; } const std::vector& encoded_min_values() const override { return _encoded_values; } @@ -175,10 +188,10 @@ class TestColumnIndex final : public ::parquet::TypedColumnIndex { const std::vector& max_values() const override { return _max_values; } private: - const std::vector _null_pages {false}; + const std::vector _null_pages; const std::vector _encoded_values; - const std::vector _null_counts {0}; - const std::vector _non_null_page_indices {0}; + const std::vector _null_counts; + std::vector _non_null_page_indices; const std::vector _min_values; const std::vector _max_values; }; @@ -601,7 +614,7 @@ TEST(ParquetStatisticsTransformTest, IgnoresNaNFloatAndDoubleColumnIndexMinMax) auto float_index = std::make_shared>( 1.0F, std::numeric_limits::quiet_NaN()); format::parquet::ParquetColumnStatistics float_page_stats; - EXPECT_FALSE(format::parquet::ParquetStatisticsUtils::TransformColumnIndexStatistics( + EXPECT_TRUE(format::parquet::ParquetStatisticsUtils::TransformColumnIndexStatistics( float_index, *schema[0], 0, &float_page_stats)); EXPECT_FALSE(float_page_stats.has_min_max); EXPECT_TRUE(float_page_stats.has_not_null); @@ -609,7 +622,7 @@ TEST(ParquetStatisticsTransformTest, IgnoresNaNFloatAndDoubleColumnIndexMinMax) auto double_index = std::make_shared>( std::numeric_limits::quiet_NaN(), 2.0); format::parquet::ParquetColumnStatistics double_page_stats; - EXPECT_FALSE(format::parquet::ParquetStatisticsUtils::TransformColumnIndexStatistics( + EXPECT_TRUE(format::parquet::ParquetStatisticsUtils::TransformColumnIndexStatistics( double_index, *schema[1], 0, &double_page_stats)); EXPECT_FALSE(double_page_stats.has_min_max); EXPECT_TRUE(double_page_stats.has_not_null); @@ -619,6 +632,21 @@ TEST(ParquetStatisticsTransformTest, IgnoresNaNFloatAndDoubleColumnIndexMinMax) EXPECT_TRUE(format::parquet::ParquetStatisticsUtils::TransformColumnIndexStatistics( finite_index, *schema[1], 0, &finite_page_stats)); EXPECT_TRUE(finite_page_stats.has_min_max); + + auto mixed_index = std::make_shared>( + std::vector {std::numeric_limits::quiet_NaN(), 1.0}, + std::vector {std::numeric_limits::quiet_NaN(), 2.0}); + format::parquet::ParquetColumnStatistics nan_page_stats; + EXPECT_TRUE(format::parquet::ParquetStatisticsUtils::TransformColumnIndexStatistics( + mixed_index, *schema[1], 0, &nan_page_stats)); + EXPECT_FALSE(nan_page_stats.has_min_max); + + format::parquet::ParquetColumnStatistics following_page_stats; + EXPECT_TRUE(format::parquet::ParquetStatisticsUtils::TransformColumnIndexStatistics( + mixed_index, *schema[1], 1, &following_page_stats)); + EXPECT_TRUE(following_page_stats.has_min_max); + EXPECT_EQ(following_page_stats.min_value.get(), 1.0); + EXPECT_EQ(following_page_stats.max_value.get(), 2.0); } TEST(ParquetStatisticsPruningTest, ExprZonemapPredicatesAndNullPredicatesPruneRowGroups) { From 6f4be2243b4fa9c3fe509b46daf961d7652fc932 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 14:49:18 +0800 Subject: [PATCH 07/23] [fix](be) Preserve null pruning with invalid Parquet bounds ### What problem does this PR solve? Issue Number: N/A Related PR: #65493 Problem Summary: When NaN invalidated Parquet min/max bounds, the statistics adapter returned no zonemap and discarded otherwise valid null-count metadata. Build a pass-all zonemap instead so range predicates remain conservative while IS NULL and IS NOT NULL can still prune from has_null and has_not_null for both footer and page-index statistics. ### Release note Preserve Parquet null-predicate pruning when floating-point min/max bounds contain NaN. ### Check List (For Author) - Test: Unit Test - ParquetStatisticsTransformTest.* (5 tests passed) - Behavior changed: Yes. Invalid range bounds no longer disable valid null-count pruning. - Does this need documentation: No --- .../format_v2/parquet/parquet_statistics.cpp | 17 +++++-- be/src/format_v2/parquet/parquet_statistics.h | 6 +++ .../parquet/parquet_statistics_test.cpp | 45 +++++++++++++++++++ 3 files changed, 64 insertions(+), 4 deletions(-) diff --git a/be/src/format_v2/parquet/parquet_statistics.cpp b/be/src/format_v2/parquet/parquet_statistics.cpp index 31f5fca2048151..d2dde4c6f29af9 100644 --- a/be/src/format_v2/parquet/parquet_statistics.cpp +++ b/be/src/format_v2/parquet/parquet_statistics.cpp @@ -721,7 +721,11 @@ std::shared_ptr make_zonemap_from_statistics( return std::make_shared(std::move(zone_map)); } if (!statistics.has_min_max) { - return nullptr; + // Null counts remain trustworthy when min/max decoding fails (for example, because a + // floating-point bound is NaN). pass_all prevents range pruning without discarding the + // has_null/has_not_null flags needed by IS NULL and IS NOT NULL predicates. + zone_map.pass_all = true; + return std::make_shared(std::move(zone_map)); } zone_map.min_value = statistics.min_value; zone_map.max_value = statistics.max_value; @@ -748,6 +752,11 @@ void accumulate_zonemap_stats(const ZoneMapEvalContext& ctx, ParquetPruningStats } // namespace +std::shared_ptr ParquetStatisticsUtils::MakeZoneMap( + const ParquetColumnStatistics& statistics) { + return make_zonemap_from_statistics(statistics); +} + ParquetColumnStatistics ParquetStatisticsUtils::TransformColumnStatistics( const ParquetColumnSchema& column_schema, const std::shared_ptr<::parquet::Statistics>& statistics, const cctz::time_zone* timezone) { @@ -916,8 +925,8 @@ bool check_statistics(const ::parquet::RowGroupMetaData& row_group, DCHECK_LT(column_schema->leaf_column_id, row_group.num_columns()); auto column_chunk = row_group.ColumnChunk(column_schema->leaf_column_id); if (column_chunk != nullptr) { - zone_map = - make_zonemap_from_statistics(ParquetStatisticsUtils::TransformColumnStatistics( + zone_map = ParquetStatisticsUtils::MakeZoneMap( + ParquetStatisticsUtils::TransformColumnStatistics( *column_schema, column_chunk->statistics(), timezone)); } add_slot_zonemap(&ctx, slot_index, column_schema->type, std::move(zone_map)); @@ -1282,7 +1291,7 @@ bool select_ranges_for_expr_zonemap( ZoneMapEvalContext ctx; add_slot_zonemap(&ctx, slot_index, column_schema->type, - make_zonemap_from_statistics(page_statistics)); + ParquetStatisticsUtils::MakeZoneMap(page_statistics)); const auto result = VExprContext::evaluate_zonemap_filter(conjuncts, ctx); page_stats.merge_page_eval_stats(ctx.stats); if (result == ZoneMapFilterResult::kNoMatch) { diff --git a/be/src/format_v2/parquet/parquet_statistics.h b/be/src/format_v2/parquet/parquet_statistics.h index 88f0dd2e2347ae..9e94562bf2d37b 100644 --- a/be/src/format_v2/parquet/parquet_statistics.h +++ b/be/src/format_v2/parquet/parquet_statistics.h @@ -43,6 +43,9 @@ class time_zone; namespace doris { class RuntimeState; +namespace segment_v2 { +struct ZoneMap; +} // namespace segment_v2 } // namespace doris namespace doris::format::parquet { @@ -121,6 +124,9 @@ struct ParquetColumnStatistics { // -> bloom filter(evaluate_bloom_filter) // ============================================================================ struct ParquetStatisticsUtils { + static std::shared_ptr MakeZoneMap( + const ParquetColumnStatistics& statistics); + static ParquetColumnStatistics TransformColumnStatistics( const ParquetColumnSchema& column_schema, const std::shared_ptr<::parquet::Statistics>& statistics, diff --git a/be/test/format_v2/parquet/parquet_statistics_test.cpp b/be/test/format_v2/parquet/parquet_statistics_test.cpp index d3cdd3bf32c365..5552c93b35a829 100644 --- a/be/test/format_v2/parquet/parquet_statistics_test.cpp +++ b/be/test/format_v2/parquet/parquet_statistics_test.cpp @@ -41,6 +41,7 @@ #include "core/data_type/data_type_string.h" #include "core/data_type/data_type_timestamptz.h" #include "core/field.h" +#include "exprs/expr_zonemap_filter.h" #include "exprs/vexpr.h" #include "exprs/vexpr_context.h" #include "exprs/vslot_ref.h" @@ -649,6 +650,50 @@ TEST(ParquetStatisticsTransformTest, IgnoresNaNFloatAndDoubleColumnIndexMinMax) EXPECT_EQ(following_page_stats.max_value.get(), 2.0); } +TEST(ParquetStatisticsTransformTest, PreservesNullCountWhenNaNInvalidatesMinMax) { + auto table = arrow::Table::Make(arrow::schema({arrow::field("f", arrow::float64(), false)}), + {double_array({1.0, 2.0})}); + auto reader = make_reader(table, 2, false, true); + auto schema = build_file_schema(*reader); + + const double nan = std::numeric_limits::quiet_NaN(); + const double max_value = 2.0; + auto footer_stats = ::parquet::MakeStatistics<::parquet::DoubleType>( + schema[0]->descriptor, encoded_value(nan), encoded_value(max_value), 2, 0, 0, true, + true, false); + const auto converted_footer = + format::parquet::ParquetStatisticsUtils::TransformColumnStatistics(*schema[0], + footer_stats); + auto footer_zone_map = format::parquet::ParquetStatisticsUtils::MakeZoneMap(converted_footer); + ASSERT_NE(footer_zone_map, nullptr); + EXPECT_TRUE(footer_zone_map->pass_all); + EXPECT_FALSE(footer_zone_map->has_null); + EXPECT_TRUE(footer_zone_map->has_not_null); + EXPECT_FALSE(expr_zonemap::range_stats_usable_for_zonemap(*footer_zone_map, schema[0]->type)); + + ZoneMapEvalContext footer_ctx; + footer_ctx.slots.emplace(0, ZoneMapEvalContext::SlotZoneMap {.data_type = schema[0]->type, + .zone_map = footer_zone_map}); + Int32ZoneMapExpr is_null_expr(0, Int32ZoneMapExpr::Op::IS_NULL); + EXPECT_EQ(is_null_expr.evaluate_zonemap_filter(footer_ctx), ZoneMapFilterResult::kNoMatch); + + auto column_index = std::make_shared>(nan, max_value); + format::parquet::ParquetColumnStatistics page_stats; + ASSERT_TRUE(format::parquet::ParquetStatisticsUtils::TransformColumnIndexStatistics( + column_index, *schema[0], 0, &page_stats)); + auto page_zone_map = format::parquet::ParquetStatisticsUtils::MakeZoneMap(page_stats); + ASSERT_NE(page_zone_map, nullptr); + EXPECT_TRUE(page_zone_map->pass_all); + EXPECT_FALSE(page_zone_map->has_null); + EXPECT_TRUE(page_zone_map->has_not_null); + EXPECT_FALSE(expr_zonemap::range_stats_usable_for_zonemap(*page_zone_map, schema[0]->type)); + + ZoneMapEvalContext page_ctx; + page_ctx.slots.emplace(0, ZoneMapEvalContext::SlotZoneMap {.data_type = schema[0]->type, + .zone_map = page_zone_map}); + EXPECT_EQ(is_null_expr.evaluate_zonemap_filter(page_ctx), ZoneMapFilterResult::kNoMatch); +} + TEST(ParquetStatisticsPruningTest, ExprZonemapPredicatesAndNullPredicatesPruneRowGroups) { auto table = arrow::Table::Make(arrow::schema({arrow::field("i", arrow::int32(), true)}), {int32_array({std::nullopt, std::nullopt, 3, 4, 5, 6})}); From 1488f8f424728eeccfeb93fff29f62d025c79f9c Mon Sep 17 00:00:00 2001 From: Gabriel Date: Fri, 10 Jul 2026 21:13:17 +0800 Subject: [PATCH 08/23] [fix](be) Skip unsafe predicates in partition pruning ### What problem does this PR solve? Issue Number: None Related PR: #65449 Problem Summary: FileScannerV2 split-level partition pruning classified predicates only by their referenced slots. A predicate could therefore reference only partition columns while also containing a zero-slot non-deterministic or error-preserving expression. Evaluating such a predicate once before reading the split could change its normal row-level semantics and incorrectly prune the whole file. Check the unwrapped predicate's existing safe-to-reorder contract before evaluating it at split level, and add unit coverage proving an unsafe runtime-filter predicate is neither executed nor used to prune a split. ### Release note Prevent FileScannerV2 partition pruning from pre-evaluating non-deterministic or error-preserving predicates. ### Check List (For Author) - Test: Unit Test - `TableReaderTest.PrepareSplit*` (2 tests passed) - `build-support/clang-format.sh` - `build-support/check-format.sh` - `git diff --check` - Behavior changed: Yes. Unsafe predicates remain on the normal row-level evaluation path instead of being evaluated once per split. - Does this need documentation: No --- be/src/format_v2/table_reader.cpp | 9 ++++ be/test/format_v2/table_reader_test.cpp | 60 +++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/be/src/format_v2/table_reader.cpp b/be/src/format_v2/table_reader.cpp index 11e2b30df6de23..765e5dc8872d88 100644 --- a/be/src/format_v2/table_reader.cpp +++ b/be/src/format_v2/table_reader.cpp @@ -803,6 +803,15 @@ Status TableReader::_evaluate_partition_prune_conjuncts(const VExprContextSPtrs& for (const auto& conjunct : conjuncts) { DORIS_CHECK(conjunct != nullptr); DORIS_CHECK(conjunct->root() != nullptr); + const auto root = conjunct->root(); + const auto impl = root->get_impl(); + const auto predicate = impl != nullptr ? impl : root; + // Split pruning evaluates a predicate once before any file rows are read. Reordering + // non-deterministic or error-preserving expressions can change their row-level semantics, + // even when every referenced slot is a partition column. + if (!predicate->is_safe_to_execute_on_selected_rows()) { + continue; + } std::set global_indices; collect_global_indices(conjunct->root(), &global_indices); if (global_indices.empty()) { diff --git a/be/test/format_v2/table_reader_test.cpp b/be/test/format_v2/table_reader_test.cpp index 252bfa5c04d73f..9b0b92d8446fdb 100644 --- a/be/test/format_v2/table_reader_test.cpp +++ b/be/test/format_v2/table_reader_test.cpp @@ -265,6 +265,29 @@ VExprSPtr runtime_filter_wrapper_expr(VExprSPtr impl) { return RuntimeFilterExpr::create_shared(node, std::move(impl), 0, false, /*filter_id=*/1); } +class NonDeterministicPartitionPredicate final : public VExpr { +public: + explicit NonDeterministicPartitionPredicate(bool* executed) + : VExpr(std::make_shared(), false), _executed(executed) {} + + Status execute_column_impl(VExprContext*, const Block*, const Selector*, size_t count, + ColumnPtr& result_column) const override { + DORIS_CHECK(_executed != nullptr); + *_executed = true; + auto result = ColumnUInt8::create(); + result->get_data().resize_fill(count, 0); + result_column = std::move(result); + return Status::OK(); + } + + const std::string& expr_name() const override { return _expr_name; } + bool is_deterministic() const override { return false; } + +private: + bool* const _executed; + const std::string _expr_name = "NonDeterministicPartitionPredicate"; +}; + class NullableArrayBigintDefaultExpr final : public VExpr { public: explicit NullableArrayBigintDefaultExpr(DataTypePtr data_type) @@ -1215,6 +1238,43 @@ TEST(TableReaderTest, PrepareSplitPrunesPartitionRuntimeFilter) { EXPECT_FALSE(reader.current_split_pruned()); } +TEST(TableReaderTest, PrepareSplitDoesNotEvaluateNonDeterministicPartitionPredicate) { + std::vector projected_columns; + auto partition_column = make_table_column(0, "part", std::make_shared()); + partition_column.is_partition_key = true; + projected_columns.push_back(std::move(partition_column)); + set_name_identifiers(&projected_columns); + + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + RuntimeProfile profile("scanner"); + TableReader reader; + ASSERT_TRUE(reader.init({ + .projected_columns = projected_columns, + .conjuncts = {}, + .format = FileFormat::PARQUET, + .scan_params = nullptr, + .io_ctx = nullptr, + .runtime_state = &state, + .scanner_profile = &profile, + }) + .ok()); + + bool predicate_executed = false; + auto predicate = std::make_shared(&predicate_executed); + predicate->add_child(table_int32_slot_ref(0, 0, "part")); + SplitReadOptions split; + split.current_range.__set_path("unused-nondeterministic-file"); + split.partition_values.emplace("part", Field::create_field(7)); + split.partition_prune_conjuncts.push_back( + VExprContext::create_shared(runtime_filter_wrapper_expr(std::move(predicate)))); + + ASSERT_TRUE(reader.prepare_split(split).ok()); + EXPECT_FALSE(predicate_executed); + EXPECT_FALSE(reader.current_split_pruned()); + ASSERT_NE(profile.get_counter("RuntimeFilterPartitionPrunedRangeNum"), nullptr); + EXPECT_EQ(profile.get_counter("RuntimeFilterPartitionPrunedRangeNum")->value(), 0); +} + TEST(TableReaderTest, CanUseInjectedFileReaderForStandaloneUnitTest) { std::vector file_schema; file_schema.push_back(make_file_column(0, "id", std::make_shared())); From 2a9a7baa421518588b88b7226c856c5159eec27e Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sat, 11 Jul 2026 13:25:18 +0800 Subject: [PATCH 09/23] [fix](be) Preserve predicate order in split pruning Issue Number: None Related PR: #65449 Problem Summary: Split pruning could skip an unsafe conjunct and still pre-execute later predicates. A later predicate could then prune a split before the earlier non-deterministic or error-preserving predicate reached its normal row-level evaluation point, changing conjunct ordering semantics. Preserve only the safe prefix for V2 partition pruning, V2 constant pruning, and the legacy FileScanner runtime-filter partition pruning path. Add unit coverage for all three paths. Split pruning now preserves row-level conjunct order when predicates are unsafe to pre-execute. - Test: Unit Test - ASAN BE unit tests: TableReaderTest.PrepareSplit*, TableReaderTest.ConstantPruningStopsAtUnsafePredicate, FileScannerTest.PartitionPruningStopsAtUnsafePredicate - C++ format check - Behavior changed: Yes. Split pruning stops at the first predicate that is unsafe to pre-execute. - Does this need documentation: No --- be/src/exec/scan/file_scanner.cpp | 5 ++ be/src/exec/scan/file_scanner.h | 13 ++++++ be/src/format_v2/table_reader.cpp | 25 ++++++---- be/src/format_v2/table_reader.h | 21 ++++++--- be/test/exec/scan/file_scanner_v2_test.cpp | 51 +++++++++++++++++++++ be/test/format_v2/table_reader_test.cpp | 53 ++++++++++++++++++++++ 6 files changed, 154 insertions(+), 14 deletions(-) diff --git a/be/src/exec/scan/file_scanner.cpp b/be/src/exec/scan/file_scanner.cpp index 78d6421adb5054..fcd0e8e863ded0 100644 --- a/be/src/exec/scan/file_scanner.cpp +++ b/be/src/exec/scan/file_scanner.cpp @@ -352,6 +352,11 @@ void FileScanner::_init_runtime_filter_partition_prune_ctxs() { auto impl = conjunct->root()->get_impl(); // If impl is not null, which means this a conjuncts from runtime filter. auto expr = impl ? impl : conjunct->root(); + // Preserve a safe prefix of the row-level conjunct order. Considering later predicates + // after an unsafe one could prune the split before the unsafe predicate is evaluated. + if (!expr->is_safe_to_execute_on_selected_rows()) { + break; + } if (_check_partition_prune_expr(expr)) { _runtime_filter_partition_prune_ctxs.emplace_back(conjunct); } diff --git a/be/src/exec/scan/file_scanner.h b/be/src/exec/scan/file_scanner.h index 3675fd2449711e..f05f4d5903dcba 100644 --- a/be/src/exec/scan/file_scanner.h +++ b/be/src/exec/scan/file_scanner.h @@ -68,6 +68,19 @@ class FileScanner : public Scanner { static const std::string FileReadBytesProfile; static const std::string FileReadTimeProfile; +#ifdef BE_TEST + void TEST_init_runtime_filter_partition_prune_ctxs( + const VExprContextSPtrs& conjuncts, + const std::unordered_map& partition_slot_index_map) { + _conjuncts = conjuncts; + _partition_slot_index_map = partition_slot_index_map; + _init_runtime_filter_partition_prune_ctxs(); + } + const VExprContextSPtrs& TEST_runtime_filter_partition_prune_ctxs() const { + return _runtime_filter_partition_prune_ctxs; + } +#endif + FileScanner(RuntimeState* state, FileScanLocalState* parent, int64_t limit, std::shared_ptr split_source, RuntimeProfile* profile, ShardedKVCache* kv_cache, diff --git a/be/src/format_v2/table_reader.cpp b/be/src/format_v2/table_reader.cpp index 765e5dc8872d88..00aa3d33998a5a 100644 --- a/be/src/format_v2/table_reader.cpp +++ b/be/src/format_v2/table_reader.cpp @@ -803,14 +803,11 @@ Status TableReader::_evaluate_partition_prune_conjuncts(const VExprContextSPtrs& for (const auto& conjunct : conjuncts) { DORIS_CHECK(conjunct != nullptr); DORIS_CHECK(conjunct->root() != nullptr); - const auto root = conjunct->root(); - const auto impl = root->get_impl(); - const auto predicate = impl != nullptr ? impl : root; - // Split pruning evaluates a predicate once before any file rows are read. Reordering - // non-deterministic or error-preserving expressions can change their row-level semantics, - // even when every referenced slot is a partition column. - if (!predicate->is_safe_to_execute_on_selected_rows()) { - continue; + // Keep only the safe prefix of the original conjunct order. If an unsafe conjunct is + // skipped, a later predicate could prune the split before the unsafe one reaches its + // normal row-level evaluation point. + if (!_is_safe_to_pre_execute(conjunct)) { + break; } std::set global_indices; collect_global_indices(conjunct->root(), &global_indices); @@ -845,6 +842,18 @@ Status TableReader::_evaluate_partition_prune_conjuncts(const VExprContextSPtrs& can_filter_all); } +bool TableReader::_is_safe_to_pre_execute(const VExprContextSPtr& conjunct) { + DORIS_CHECK(conjunct != nullptr); + DORIS_CHECK(conjunct->root() != nullptr); + const auto root = conjunct->root(); + const auto impl = root->get_impl(); + const auto predicate = impl != nullptr ? impl : root; + // Split pruning evaluates a predicate once before any file rows are read. Reordering + // non-deterministic or error-preserving expressions can change their row-level semantics, + // even when every referenced slot is a partition column or maps to a constant entry. + return predicate->is_safe_to_execute_on_selected_rows(); +} + Status TableReader::_build_partition_prune_block(Block* block) const { DORIS_CHECK(block != nullptr); DORIS_CHECK(!_projected_columns.empty()); diff --git a/be/src/format_v2/table_reader.h b/be/src/format_v2/table_reader.h index da2cb351ff68ce..a091fe1cf5b327 100644 --- a/be/src/format_v2/table_reader.h +++ b/be/src/format_v2/table_reader.h @@ -413,6 +413,7 @@ class TableReader { Status _build_table_filters_from_conjuncts(); Status _evaluate_partition_prune_conjuncts(const VExprContextSPtrs& conjuncts, bool* can_filter_all); + static bool _is_safe_to_pre_execute(const VExprContextSPtr& conjunct); Status _build_partition_prune_block(Block* block) const; Status _open_local_filter_exprs(const FileScanRequest& file_request); Status _init_reader_condition_cache(const FileScanRequest& file_request); @@ -423,12 +424,20 @@ class TableReader { DORIS_CHECK(can_filter_all != nullptr); *can_filter_all = false; for (const auto& table_filter : _table_filters) { - if (table_filter.conjunct == nullptr || - // RuntimeFilterExpr does not implement execute_column_impl(); it is evaluated by - // the row-level filter path through execute_filter(). Constant split pruning uses - // VExprContext::execute() on a one-row synthetic block, so runtime filters must not - // be pre-executed here even when their referenced slot maps to a constant value. - table_filter.conjunct->root()->is_rf_wrapper() || + if (table_filter.conjunct == nullptr) { + continue; + } + // Constant pruning must preserve a safe prefix of the row-level conjunct order. Once + // an unsafe predicate is reached, evaluating any later constant predicate could hide + // its error or other row-level behavior by pruning the split first. + if (!_is_safe_to_pre_execute(table_filter.conjunct)) { + break; + } + // RuntimeFilterExpr does not implement execute_column_impl(); it is evaluated by the + // row-level filter path through execute_filter(). Constant split pruning uses + // VExprContext::execute() on a one-row synthetic block, so runtime filters must not be + // pre-executed here even when their referenced slot maps to a constant value. + if (table_filter.conjunct->root()->is_rf_wrapper() || !_table_filter_has_only_constant_entries(table_filter)) { continue; } diff --git a/be/test/exec/scan/file_scanner_v2_test.cpp b/be/test/exec/scan/file_scanner_v2_test.cpp index 9df8dcb54f4934..2ab86072327e5c 100644 --- a/be/test/exec/scan/file_scanner_v2_test.cpp +++ b/be/test/exec/scan/file_scanner_v2_test.cpp @@ -29,9 +29,13 @@ #include "common/consts.h" #include "core/assert_cast.h" +#include "core/block/block.h" +#include "core/column/column_vector.h" #include "core/data_type/data_type_number.h" #include "core/data_type/data_type_string.h" #include "exec/operator/file_scan_operator.h" +#include "exec/scan/file_scanner.h" +#include "exec/scan/split_source_connector.h" #include "exprs/runtime_filter_expr.h" #include "exprs/vdirect_in_predicate.h" #include "exprs/vslot_ref.h" @@ -63,6 +67,33 @@ VExprSPtr slot_ref(int slot_id, int column_id, DataTypePtr type, const std::stri return VSlotRef::create_shared(slot_id, column_id, -1, std::move(type), name); } +TExprNode bool_in_pred_node(); + +class UnsafePartitionPredicate final : public VExpr { +public: + UnsafePartitionPredicate() : VExpr(std::make_shared(), false) {} + + Status execute_column_impl(VExprContext*, const Block*, const Selector*, size_t count, + ColumnPtr& result_column) const override { + auto result = ColumnUInt8::create(); + result->get_data().resize_fill(count, 1); + result_column = std::move(result); + return Status::OK(); + } + + const std::string& expr_name() const override { return _expr_name; } + bool is_safe_to_execute_on_selected_rows() const override { return false; } + +private: + const std::string _expr_name = "UnsafePartitionPredicate"; +}; + +VExprContextSPtr runtime_filter_context(VExprSPtr impl, int filter_id) { + const auto node = bool_in_pred_node(); + return VExprContext::create_shared( + RuntimeFilterExpr::create_shared(node, std::move(impl), 0.4, false, filter_id)); +} + TExprNode bool_in_pred_node() { TTypeDesc bool_type; TTypeNode bool_node; @@ -512,4 +543,24 @@ TEST(FileScannerV2Test, RewriteSlotRefsToGlobalIndexMatrix) { } } +TEST(FileScannerTest, PartitionPruningStopsAtUnsafePredicate) { + const auto bool_type = std::make_shared(); + auto unsafe_predicate = std::make_shared(); + unsafe_predicate->add_child(slot_ref(1, 0, bool_type, "part")); + VExprContextSPtrs conjuncts { + runtime_filter_context(slot_ref(1, 0, bool_type, "part"), 1), + runtime_filter_context(std::move(unsafe_predicate), 2), + runtime_filter_context(slot_ref(1, 0, bool_type, "part"), 3), + }; + + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + RuntimeProfile profile("file_scanner"); + FileScanner scanner(&state, &profile, nullptr, nullptr, nullptr); + scanner.TEST_init_runtime_filter_partition_prune_ctxs(conjuncts, {{1, 0}}); + + const auto& partition_conjuncts = scanner.TEST_runtime_filter_partition_prune_ctxs(); + ASSERT_EQ(partition_conjuncts.size(), 1); + EXPECT_EQ(partition_conjuncts[0], conjuncts[0]); +} + } // namespace doris diff --git a/be/test/format_v2/table_reader_test.cpp b/be/test/format_v2/table_reader_test.cpp index 9b0b92d8446fdb..9c5c37235b0399 100644 --- a/be/test/format_v2/table_reader_test.cpp +++ b/be/test/format_v2/table_reader_test.cpp @@ -283,6 +283,12 @@ class NonDeterministicPartitionPredicate final : public VExpr { const std::string& expr_name() const override { return _expr_name; } bool is_deterministic() const override { return false; } + Status clone_node(VExprSPtr* cloned_expr) const override { + DORIS_CHECK(cloned_expr != nullptr); + *cloned_expr = std::make_shared(_executed); + return Status::OK(); + } + private: bool* const _executed; const std::string _expr_name = "NonDeterministicPartitionPredicate"; @@ -1267,6 +1273,8 @@ TEST(TableReaderTest, PrepareSplitDoesNotEvaluateNonDeterministicPartitionPredic split.partition_values.emplace("part", Field::create_field(7)); split.partition_prune_conjuncts.push_back( VExprContext::create_shared(runtime_filter_wrapper_expr(std::move(predicate)))); + split.partition_prune_conjuncts.push_back(VExprContext::create_shared( + runtime_filter_wrapper_expr(table_int32_greater_than_expr(0, 0, 10)))); ASSERT_TRUE(reader.prepare_split(split).ok()); EXPECT_FALSE(predicate_executed); @@ -1275,6 +1283,51 @@ TEST(TableReaderTest, PrepareSplitDoesNotEvaluateNonDeterministicPartitionPredic EXPECT_EQ(profile.get_counter("RuntimeFilterPartitionPrunedRangeNum")->value(), 0); } +TEST(TableReaderTest, ConstantPruningStopsAtUnsafePredicate) { + std::vector projected_columns; + auto partition_column = make_table_column(0, "part", std::make_shared()); + partition_column.is_partition_key = true; + projected_columns.push_back(std::move(partition_column)); + set_name_identifiers(&projected_columns); + + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + bool predicate_executed = false; + auto unsafe_predicate = + std::make_shared(&predicate_executed); + unsafe_predicate->add_child(table_int32_slot_ref(0, 0, "part")); + auto fake_state = std::make_shared(); + FakeTableReader reader({}, fake_state); + ASSERT_TRUE(reader.init({ + .projected_columns = projected_columns, + .conjuncts = + { + prepared_conjunct(&state, unsafe_predicate), + prepared_conjunct(&state, + table_int32_greater_than_expr( + 0, 0, 10)), + }, + .format = FileFormat::PARQUET, + .scan_params = nullptr, + .io_ctx = nullptr, + .runtime_state = &state, + .scanner_profile = nullptr, + }) + .ok()); + + SplitReadOptions split; + split.current_range.__set_path("fake-table-reader-input"); + split.partition_values.emplace("part", Field::create_field(7)); + ASSERT_TRUE(reader.prepare_split(split).ok()); + + Block block = build_table_block(projected_columns); + bool eos = false; + ASSERT_TRUE(reader.get_block(&block, &eos).ok()); + EXPECT_FALSE(predicate_executed); + EXPECT_FALSE(eos); + EXPECT_EQ(fake_state->open_count, 1); + ASSERT_TRUE(reader.close().ok()); +} + TEST(TableReaderTest, CanUseInjectedFileReaderForStandaloneUnitTest) { std::vector file_schema; file_schema.push_back(make_file_column(0, "id", std::make_shared())); From 947744f7ccabfc6b0a980d71c25bc90938718794 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 12:55:41 +0800 Subject: [PATCH 10/23] [fix](be) Reject lossy file predicate literal conversion ### What problem does this PR solve? Issue Number: N/A Related PR: N/A Problem Summary: FileScannerV2 localized predicate literals to the file column type whenever conversion succeeded, even when the conversion changed the literal value. For example, localizing a DOUBLE predicate value of 1.5 to an INT file column changed the boundary to 1 and could make file-level filtering silently drop matching rows. Require an exact round trip through the file type before localizing a literal; otherwise, preserve table semantics by casting the file slot. Exact conversions such as BIGINT 1 to INT 1 remain localized. ### Release note Fix FileScannerV2 predicate localization to avoid missing rows after lossy literal conversion. ### Check List (For Author) - Test: Unit Test - ColumnMapperCastTest targeted literal localization tests - Behavior changed: Yes, lossy literal conversions now fall back to casting the file slot. - Does this need documentation: No --- be/src/format_v2/column_mapper.cpp | 15 +++++ be/test/format_v2/column_mapper_test.cpp | 80 ++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/be/src/format_v2/column_mapper.cpp b/be/src/format_v2/column_mapper.cpp index e60e22b85e7fdf..fd922d333e238a 100644 --- a/be/src/format_v2/column_mapper.cpp +++ b/be/src/format_v2/column_mapper.cpp @@ -553,6 +553,21 @@ static VExprSPtr rewrite_literal_to_file_type(const VExprSPtr& literal_expr, if (file_field.get_type() != remove_nullable(rewrite_info.file_type)->get_primitive_type()) { return nullptr; } + Field round_trip_field; + try { + convert_field_to_type(file_field, *original_literal->data_type(), &round_trip_field, + rewrite_info.file_type.get()); + } catch (const Exception&) { + return nullptr; + } + // Only localize a literal when converting it to the file type is lossless. For example, + // BIGINT 1 -> INT 1 -> BIGINT 1 succeeds. However, for a table predicate `value < 1.5` + // with DOUBLE table type and INT file type, rewriting 1.5 to INT 1 would make a file value + // of 1 fail `value < 1` and silently drop a matching row. Its round trip + // DOUBLE 1.5 -> INT 1 -> DOUBLE 1.0 fails here and keeps the table-typed predicate instead. + if (round_trip_field != original_field) { + return nullptr; + } auto literal = std::make_shared( rewrite_info.file_type, file_field, original_literal->data_type(), original_field); rewrite_context->add_created_expr(literal); diff --git a/be/test/format_v2/column_mapper_test.cpp b/be/test/format_v2/column_mapper_test.cpp index 4e815d6cbf3c5e..0e5e51c62cf020 100644 --- a/be/test/format_v2/column_mapper_test.cpp +++ b/be/test/format_v2/column_mapper_test.cpp @@ -3486,6 +3486,44 @@ TEST_F(ColumnMapperCastTest, ColumnMapperCastsLiteralForLiteralSlotPredicateType file_request.conjuncts[0]->close(); } +// Scenario: a fractional table literal cannot be localized to an integral file type without +// changing the predicate boundary, so the mapper must cast the file slot instead. +TEST_F(ColumnMapperCastTest, ColumnMapperRejectsLossyBinaryLiteralConversion) { + TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_NAME}); + auto table_column = name_col("value", f64()); + std::vector projected_columns {table_column}; + + auto file_field = name_col("value", i32(), 0); + std::vector file_schema {file_field}; + + auto status = mapper.create_mapping(projected_columns, {}, file_schema); + ASSERT_TRUE(status.ok()) << status; + + auto predicate = binary_predicate( + TExprOpcode::LT, VSlotRef::create_shared(0, 0, -1, table_column.type, "value"), + VLiteral::create_shared(table_column.type, Field::create_field(1.5))); + TableFilter table_filter; + table_filter.conjunct = VExprContext::create_shared(predicate); + table_filter.global_indices = {GlobalIndex(0)}; + + FileScanRequest file_request; + ASSERT_TRUE(mapper.create_scan_request({table_filter}, projected_columns, &file_request, &state) + .ok()); + ASSERT_EQ(file_request.conjuncts.size(), 1); + const auto& localized_expr = file_request.conjuncts[0]->root(); + ASSERT_EQ(localized_expr->get_num_children(), 2); + const auto& localized_slot_cast = localized_expr->children()[0]; + ASSERT_NE(dynamic_cast(localized_slot_cast.get()), nullptr); + EXPECT_TRUE(localized_slot_cast->data_type()->equals(*table_column.type)); + ASSERT_EQ(localized_slot_cast->get_num_children(), 1); + const auto* localized_slot = + assert_cast(localized_slot_cast->children()[0].get()); + EXPECT_EQ(localized_slot->column_id(), 0); + EXPECT_TRUE(localized_slot->data_type()->equals(*file_field.type)); + EXPECT_TRUE(localized_expr->children()[1]->is_literal()); + EXPECT_TRUE(localized_expr->children()[1]->data_type()->equals(*table_column.type)); +} + // Scenario: IN predicate literals are all rewritten to file type when every literal conversion is safe. TEST_F(ColumnMapperCastTest, ColumnMapperCastsInPredicateLiteralsForTypeMismatch) { TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_NAME}); @@ -3524,6 +3562,48 @@ TEST_F(ColumnMapperCastTest, ColumnMapperCastsInPredicateLiteralsForTypeMismatch EXPECT_TRUE(localized_expr->children()[2]->data_type()->equals(*file_field.type)); } +// Scenario: one lossy IN literal prevents the entire predicate from being localized to file type. +TEST_F(ColumnMapperCastTest, ColumnMapperRejectsLossyInPredicateLiteralConversion) { + TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_NAME}); + auto table_column = name_col("value", f64()); + std::vector projected_columns {table_column}; + + auto file_field = name_col("value", i32(), 0); + std::vector file_schema {file_field}; + + auto status = mapper.create_mapping(projected_columns, {}, file_schema); + ASSERT_TRUE(status.ok()) << status; + + auto predicate = create_in_predicate(); + predicate->add_child(VSlotRef::create_shared(0, 0, -1, table_column.type, "value")); + predicate->add_child( + VLiteral::create_shared(table_column.type, Field::create_field(1.0))); + predicate->add_child( + VLiteral::create_shared(table_column.type, Field::create_field(1.5))); + TableFilter table_filter; + table_filter.conjunct = VExprContext::create_shared(predicate); + table_filter.global_indices = {GlobalIndex(0)}; + + FileScanRequest file_request; + ASSERT_TRUE(mapper.create_scan_request({table_filter}, projected_columns, &file_request, &state) + .ok()); + ASSERT_EQ(file_request.conjuncts.size(), 1); + const auto& localized_expr = file_request.conjuncts[0]->root(); + ASSERT_EQ(localized_expr->get_num_children(), 3); + const auto& localized_slot_cast = localized_expr->children()[0]; + ASSERT_NE(dynamic_cast(localized_slot_cast.get()), nullptr); + EXPECT_TRUE(localized_slot_cast->data_type()->equals(*table_column.type)); + ASSERT_EQ(localized_slot_cast->get_num_children(), 1); + const auto* localized_slot = + assert_cast(localized_slot_cast->children()[0].get()); + EXPECT_EQ(localized_slot->column_id(), 0); + EXPECT_TRUE(localized_slot->data_type()->equals(*file_field.type)); + EXPECT_TRUE(localized_expr->children()[1]->is_literal()); + EXPECT_TRUE(localized_expr->children()[1]->data_type()->equals(*table_column.type)); + EXPECT_TRUE(localized_expr->children()[2]->is_literal()); + EXPECT_TRUE(localized_expr->children()[2]->data_type()->equals(*table_column.type)); +} + // Scenario: IN predicate falls back to casting the file slot when any literal cannot be converted safely. TEST_F(ColumnMapperCastTest, ColumnMapperFallsBackToSlotCastWhenInPredicateLiteralRewriteFails) { TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_NAME}); From 407c385cc0a9b3988e9c88111fe1b096f4dcdc5d Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 13:51:46 +0800 Subject: [PATCH 11/23] [fix](be) Restrict file literal localization to lossless casts ### What problem does this PR solve? Issue Number: N/A Related PR: #65490 Problem Summary: Round-trip equality alone can accept unsafe predicate localization when file values lose information during table materialization or when complex Field equality ignores nested contents. Restrict localization to scalar numeric file-to-table casts that preserve every file value, retain the exact literal round-trip check, and fall back to the table predicate for complex or unsupported casts. ### Release note Prevent file predicate localization from dropping rows for lossy schema-evolution casts. ### Check List (For Author) - Test: Unit Test - ColumnMapperCastTest (19 tests passed) - Behavior changed: Yes. Unsafe literal localization now falls back to evaluating the table-typed predicate after materialization. - Does this need documentation: No --- be/src/format_v2/column_mapper.cpp | 83 ++++++++++++++++++++++-- be/test/format_v2/column_mapper_test.cpp | 67 +++++++++++++++++++ 2 files changed, 145 insertions(+), 5 deletions(-) diff --git a/be/src/format_v2/column_mapper.cpp b/be/src/format_v2/column_mapper.cpp index fd922d333e238a..895e2fe2c91270 100644 --- a/be/src/format_v2/column_mapper.cpp +++ b/be/src/format_v2/column_mapper.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -530,6 +531,71 @@ static void collect_top_level_slot_columns(const VExprSPtr& expr, } } +static std::optional signed_integer_width(PrimitiveType type) { + switch (type) { + case TYPE_TINYINT: + return 8; + case TYPE_SMALLINT: + return 16; + case TYPE_INT: + return 32; + case TYPE_BIGINT: + return 64; + case TYPE_LARGEINT: + return 128; + default: + return std::nullopt; + } +} + +static std::optional floating_width(PrimitiveType type) { + switch (type) { + case TYPE_FLOAT: + return 32; + case TYPE_DOUBLE: + return 64; + default: + return std::nullopt; + } +} + +static std::optional floating_exact_integer_width(PrimitiveType type) { + switch (type) { + case TYPE_FLOAT: + return 24; + case TYPE_DOUBLE: + return 53; + default: + return std::nullopt; + } +} + +static bool is_lossless_file_to_table_numeric_cast(const DataTypePtr& file_type, + const DataTypePtr& table_type) { + const auto file_nested_type = remove_nullable(file_type); + const auto table_nested_type = remove_nullable(table_type); + if (file_nested_type->equals(*table_nested_type)) { + return true; + } + + const auto file_primitive_type = file_nested_type->get_primitive_type(); + const auto table_primitive_type = table_nested_type->get_primitive_type(); + if (const auto file_width = signed_integer_width(file_primitive_type)) { + if (const auto table_width = signed_integer_width(table_primitive_type)) { + return *table_width >= *file_width; + } + if (const auto table_width = floating_exact_integer_width(table_primitive_type)) { + return *table_width >= *file_width; + } + return false; + } + if (const auto file_width = floating_width(file_primitive_type)) { + const auto table_width = floating_width(table_primitive_type); + return table_width.has_value() && *table_width >= *file_width; + } + return false; +} + static VExprSPtr rewrite_literal_to_file_type(const VExprSPtr& literal_expr, const FileSlotRewriteInfo& rewrite_info, RewriteContext* rewrite_context) { @@ -540,6 +606,16 @@ static VExprSPtr rewrite_literal_to_file_type(const VExprSPtr& literal_expr, if (rewrite_info.file_type->equals(*original_literal->data_type())) { return original_literal; } + // A literal round trip alone cannot prove that file-local evaluation is safe: the file slot + // itself may lose information when materialized as the table type. For example, DOUBLE 1.5 + // becomes BIGINT 1, so table predicate `value = 1` is true while file predicate + // `value = 1.0` is false. Complex Field equality also does not compare nested contents. + // Restrict localization to scalar numeric casts that preserve every file value; unsupported + // and complex casts keep the table predicate and evaluate after materialization. + if (!is_lossless_file_to_table_numeric_cast(rewrite_info.file_type, + original_literal->data_type())) { + return nullptr; + } Field file_field; try { convert_field_to_type(original_field, *rewrite_info.file_type, &file_field, @@ -560,11 +636,8 @@ static VExprSPtr rewrite_literal_to_file_type(const VExprSPtr& literal_expr, } catch (const Exception&) { return nullptr; } - // Only localize a literal when converting it to the file type is lossless. For example, - // BIGINT 1 -> INT 1 -> BIGINT 1 succeeds. However, for a table predicate `value < 1.5` - // with DOUBLE table type and INT file type, rewriting 1.5 to INT 1 would make a file value - // of 1 fail `value < 1` and silently drop a matching row. Its round trip - // DOUBLE 1.5 -> INT 1 -> DOUBLE 1.0 fails here and keeps the table-typed predicate instead. + // The file-to-table type check protects every possible file value. This round trip separately + // proves that the specific predicate boundary is exactly representable in the file type. if (round_trip_field != original_field) { return nullptr; } diff --git a/be/test/format_v2/column_mapper_test.cpp b/be/test/format_v2/column_mapper_test.cpp index 0e5e51c62cf020..90d4e7b8d552d8 100644 --- a/be/test/format_v2/column_mapper_test.cpp +++ b/be/test/format_v2/column_mapper_test.cpp @@ -3524,6 +3524,73 @@ TEST_F(ColumnMapperCastTest, ColumnMapperRejectsLossyBinaryLiteralConversion) { EXPECT_TRUE(localized_expr->children()[1]->data_type()->equals(*table_column.type)); } +// Scenario: an exactly representable literal is still unsafe to localize when arbitrary file +// values lose information during materialization to the table type. +TEST_F(ColumnMapperCastTest, ColumnMapperRejectsLossyFileToTableConversion) { + TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_NAME}); + auto table_column = name_col("value", i64()); + std::vector projected_columns {table_column}; + + auto file_field = name_col("value", f64(), 0); + std::vector file_schema {file_field}; + + auto status = mapper.create_mapping(projected_columns, {}, file_schema); + ASSERT_TRUE(status.ok()) << status; + + auto predicate = binary_predicate( + TExprOpcode::EQ, VSlotRef::create_shared(0, 0, -1, table_column.type, "value"), + VLiteral::create_shared(table_column.type, Field::create_field(1))); + TableFilter table_filter; + table_filter.conjunct = VExprContext::create_shared(predicate); + table_filter.global_indices = {GlobalIndex(0)}; + + FileScanRequest file_request; + ASSERT_TRUE(mapper.create_scan_request({table_filter}, projected_columns, &file_request, &state) + .ok()); + ASSERT_EQ(file_request.conjuncts.size(), 1); + const auto& localized_expr = file_request.conjuncts[0]->root(); + ASSERT_EQ(localized_expr->get_num_children(), 2); + const auto& localized_slot_cast = localized_expr->children()[0]; + ASSERT_NE(dynamic_cast(localized_slot_cast.get()), nullptr); + EXPECT_TRUE(localized_slot_cast->data_type()->equals(*table_column.type)); + ASSERT_EQ(localized_slot_cast->get_num_children(), 1); + const auto* localized_slot = + assert_cast(localized_slot_cast->children()[0].get()); + EXPECT_EQ(localized_slot->column_id(), 0); + EXPECT_TRUE(localized_slot->data_type()->equals(*file_field.type)); + EXPECT_TRUE(localized_expr->children()[1]->data_type()->equals(*table_column.type)); +} + +// Scenario: complex Field equality does not compare nested values, so complex literals must not +// use the scalar round-trip guard. +TEST_F(ColumnMapperCastTest, ColumnMapperRejectsComplexLiteralLocalization) { + TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_NAME}); + auto table_column = array_col("value", -1, name_col("element", f64())); + set_name_identifiers(&table_column, -1); + const auto& table_type = table_column.type; + std::vector projected_columns {table_column}; + + auto file_field = array_col("value", -1, name_col("element", i32()), 0); + set_name_identifiers(&file_field, 0); + std::vector file_schema {file_field}; + + auto status = mapper.create_mapping(projected_columns, {}, file_schema); + ASSERT_TRUE(status.ok()) << status; + + Array literal_values {Field::create_field(1.5)}; + auto predicate = binary_predicate( + TExprOpcode::EQ, VSlotRef::create_shared(0, 0, -1, table_type, "value"), + VLiteral::create_shared(table_type, Field::create_field(literal_values))); + TableFilter table_filter; + table_filter.conjunct = VExprContext::create_shared(predicate); + table_filter.global_indices = {GlobalIndex(0)}; + + FileScanRequest file_request; + ASSERT_TRUE(mapper.create_scan_request({table_filter}, projected_columns, &file_request, &state) + .ok()); + EXPECT_TRUE(file_request.conjuncts.empty()); +} + // Scenario: IN predicate literals are all rewritten to file type when every literal conversion is safe. TEST_F(ColumnMapperCastTest, ColumnMapperCastsInPredicateLiteralsForTypeMismatch) { TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_NAME}); From 4a6bfaabd632ba262a0e48680b2761c4a613b983 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 13:36:16 +0800 Subject: [PATCH 12/23] [fix](be) Disable Parquet binary min max pushdown ### What problem does this PR solve? Issue Number: N/A Related PR: N/A Problem Summary: FileScannerV2 returned Parquet footer min/max statistics as exact aggregate values. For BYTE_ARRAY and FIXED_LEN_BYTE_ARRAY columns, Parquet permits truncated lower and upper bounds, and Arrow 17 does not expose the exactness flags. MIN/MAX pushdown could therefore return a synthetic bound that does not occur in the data. Reject aggregate pushdown for these physical types so the upper reader falls back to a normal scan, while retaining exact numeric pushdown and safe statistics pruning. ### Release note Fix incorrect Parquet MIN/MAX aggregate results for truncated binary statistics. ### Check List (For Author) - Test: Unit Test - ParquetScanTest targeted binary rejection and numeric success tests - Behavior changed: Yes, Parquet binary MIN/MAX aggregate pushdown now falls back to scanning. - Does this need documentation: No --- be/src/format_v2/parquet/parquet_reader.cpp | 17 +++++++ .../format_v2/parquet/parquet_scan_test.cpp | 46 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/be/src/format_v2/parquet/parquet_reader.cpp b/be/src/format_v2/parquet/parquet_reader.cpp index 82ad3f78891c80..01ad3333818b27 100644 --- a/be/src/format_v2/parquet/parquet_reader.cpp +++ b/be/src/format_v2/parquet/parquet_reader.cpp @@ -287,6 +287,22 @@ static Status find_projected_minmax_leaf(const ParquetColumnSchema& column_schem child_projection.local_id(), column_schema.name); } +static Status validate_minmax_aggregate_statistics(const ParquetColumnSchema& column_schema) { + DORIS_CHECK(column_schema.descriptor != nullptr); + switch (column_schema.descriptor->physical_type()) { + case ::parquet::Type::BYTE_ARRAY: + case ::parquet::Type::FIXED_LEN_BYTE_ARRAY: + // Arrow 17 does not expose Parquet's min/max exactness flags. Binary statistics may be + // truncated bounds rather than values present in the file, so they are safe for pruning + // but cannot be returned as exact aggregate results. + return Status::NotSupported( + "Parquet MIN/MAX aggregate pushdown requires exact statistics for column {}", + column_schema.name); + default: + return Status::OK(); + } +} + void ParquetReader::_fill_column_definition(const ParquetColumnSchema& column_schema, format::ColumnDefinition* field) const { if (column_schema.parquet_field_id >= 0) { @@ -671,6 +687,7 @@ Status ParquetReader::get_aggregate_result(const format::FileAggregateRequest& r RETURN_IF_ERROR(find_projected_minmax_leaf( *column_schema, request.columns[request_column_idx].projection, &leaf_schema)); DORIS_CHECK(leaf_schema != nullptr); + RETURN_IF_ERROR(validate_minmax_aggregate_statistics(*leaf_schema)); auto& aggregate_column = result->columns[request_column_idx]; aggregate_column.projection = request.columns[request_column_idx].projection; diff --git a/be/test/format_v2/parquet/parquet_scan_test.cpp b/be/test/format_v2/parquet/parquet_scan_test.cpp index edd51c4520307a..d73e08235c8b4e 100644 --- a/be/test/format_v2/parquet/parquet_scan_test.cpp +++ b/be/test/format_v2/parquet/parquet_scan_test.cpp @@ -222,6 +222,25 @@ std::shared_ptr build_int32_array(const std::vector& valu return finish_array(&builder); } +std::shared_ptr build_string_array(const std::vector& values) { + arrow::StringBuilder builder; + for (const auto& value : values) { + EXPECT_TRUE(builder.Append(value).ok()); + } + return finish_array(&builder); +} + +std::shared_ptr build_fixed_binary_array(const std::vector& values, + int byte_width) { + auto type = arrow::fixed_size_binary(byte_width); + arrow::FixedSizeBinaryBuilder builder(type, arrow::default_memory_pool()); + for (const auto& value : values) { + EXPECT_EQ(value.size(), byte_width); + EXPECT_TRUE(builder.Append(reinterpret_cast(value.data())).ok()); + } + return finish_array(&builder); +} + std::shared_ptr build_struct_array(const std::vector& ids, const std::vector& names) { auto struct_type = arrow::struct_({arrow::field("id", arrow::int32(), false), @@ -295,6 +314,16 @@ void write_int_pair_parquet_file(const std::string& file_path, int64_t row_group write_table(file_path, table, row_group_size, false, false, enable_statistics); } +void write_binary_minmax_parquet_file(const std::string& file_path) { + auto schema = arrow::schema({ + arrow::field("text", arrow::utf8(), false), + arrow::field("fixed", arrow::fixed_size_binary(4), false), + }); + auto table = arrow::Table::Make(schema, {build_string_array({"alpha", "omega"}), + build_fixed_binary_array({"aaaa", "zzzz"}, 4)}); + write_table(file_path, table, 2); +} + void write_struct_parquet_file(const std::string& file_path) { auto struct_type = arrow::struct_({arrow::field("id", arrow::int32(), false), arrow::field("name", arrow::utf8(), false)}); @@ -692,6 +721,23 @@ TEST_F(ParquetScanTest, AggregateCountAndMinMaxUseAllSelectedRowGroups) { EXPECT_EQ(minmax_result.columns[1].max_value.get(), 60); } +TEST_F(ParquetScanTest, AggregateMinMaxRejectsInexactBinaryStatistics) { + write_binary_minmax_parquet_file(_file_path); + auto reader = create_reader(); + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + ASSERT_TRUE(reader->init(&state).ok()); + open_all_row_groups(reader.get()); + + for (int32_t column_id = 0; column_id < 2; ++column_id) { + format::FileAggregateRequest request; + request.agg_type = TPushAggOp::MINMAX; + request.columns.push_back({.projection = field_projection(column_id)}); + format::FileAggregateResult result; + const auto status = reader->get_aggregate_result(request, &result); + EXPECT_TRUE(status.is()) << status; + } +} + TEST_F(ParquetScanTest, AggregateRespectsStatisticsPrunedRowGroups) { write_int_pair_parquet_file(_file_path); auto reader = create_reader(); From bbddc01752e2e1472ce902147f32da74e0cf3d02 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 21:12:47 +0800 Subject: [PATCH 13/23] [fix](be) Preserve conservative split pruning ### What problem does this PR solve? Issue Number: None Related PR: #65500 Problem Summary: Corrupt external timestamp statistics with inverted bounds could terminate the BE, and a slotless unsafe conjunct could be omitted from localized table filters so a later constant predicate incorrectly pruned the split. Treat inverted statistics as unusable and track the constant-pruning safe prefix from the original conjunct order, including slotless ordering barriers. ### Release note Corrupt timestamp statistics and unsafe slotless predicates now fall back to conservative scan behavior. ### Check List (For Author) - Test: Regression test / Unit Test - TimestampStatisticsTest.* - TableReaderTest.ConstantPruningStopsAtUnsafePredicate - TableReaderTest.ConstantPruningStopsAtUnsafeSlotlessPredicate - Remote BE build - Behavior changed: Yes (split pruning now falls back conservatively for corrupt timestamp statistics and unsafe slotless predicates) - Does this need documentation: No --- be/src/format_v2/table_reader.cpp | 14 ++++++ be/src/format_v2/table_reader.h | 19 ++++--- be/src/format_v2/timestamp_statistics.h | 7 ++- be/test/format_v2/table_reader_test.cpp | 50 +++++++++++++++++++ .../format_v2/timestamp_statistics_test.cpp | 3 ++ 5 files changed, 85 insertions(+), 8 deletions(-) diff --git a/be/src/format_v2/table_reader.cpp b/be/src/format_v2/table_reader.cpp index 00aa3d33998a5a..1b48127e608a99 100644 --- a/be/src/format_v2/table_reader.cpp +++ b/be/src/format_v2/table_reader.cpp @@ -523,9 +523,23 @@ Status TableReader::init(TableReadOptions&& options) { Status TableReader::_build_table_filters_from_conjuncts() { _table_filters.clear(); + _constant_pruning_safe_filter_count = 0; + bool in_safe_prefix = true; for (const auto& conjunct : _conjuncts) { + DORIS_CHECK(conjunct != nullptr); + DORIS_CHECK(conjunct->root() != nullptr); + // `_table_filters` omits expressions without slot references, but such an expression still + // occupies a position in the row-level conjunct order. Record how many localized filters + // precede the first unsafe original conjunct so constant pruning cannot jump over a + // slotless non-deterministic/error-preserving barrier. + if (in_safe_prefix && !_is_safe_to_pre_execute(conjunct)) { + in_safe_prefix = false; + } RETURN_IF_ERROR( build_table_filters_from_conjunct(conjunct, _runtime_state, &_table_filters)); + if (in_safe_prefix) { + _constant_pruning_safe_filter_count = _table_filters.size(); + } } return Status::OK(); } diff --git a/be/src/format_v2/table_reader.h b/be/src/format_v2/table_reader.h index a091fe1cf5b327..44fdc9c16a73dc 100644 --- a/be/src/format_v2/table_reader.h +++ b/be/src/format_v2/table_reader.h @@ -422,17 +422,17 @@ class TableReader { Status _evaluate_constant_filters(bool* can_filter_all) { DORIS_CHECK(can_filter_all != nullptr); + DORIS_CHECK_LE(_constant_pruning_safe_filter_count, _table_filters.size()); *can_filter_all = false; - for (const auto& table_filter : _table_filters) { + // The bound was derived from the original `_conjuncts` order, which includes slotless + // expressions omitted from `_table_filters`. Iterating only this prefix therefore cannot + // skip an unsafe row-level predicate and pre-execute a later constant predicate. + for (size_t i = 0; i < _constant_pruning_safe_filter_count; ++i) { + const auto& table_filter = _table_filters[i]; if (table_filter.conjunct == nullptr) { continue; } - // Constant pruning must preserve a safe prefix of the row-level conjunct order. Once - // an unsafe predicate is reached, evaluating any later constant predicate could hide - // its error or other row-level behavior by pruning the split first. - if (!_is_safe_to_pre_execute(table_filter.conjunct)) { - break; - } + DORIS_CHECK(_is_safe_to_pre_execute(table_filter.conjunct)); // RuntimeFilterExpr does not implement execute_column_impl(); it is evaluated by the // row-level filter path through execute_filter(). Constant split pruning uses // VExprContext::execute() on a one-row synthetic block, so runtime filters must not be @@ -619,6 +619,7 @@ class TableReader { _data_reader.column_mapper.reset(); } _table_filters.clear(); + _constant_pruning_safe_filter_count = 0; _data_reader.file_schema.clear(); _data_reader.file_block_layout.clear(); _data_reader.block_template.clear(); @@ -1525,6 +1526,10 @@ class TableReader { std::map _partition_values; // Predicates built from scan conjuncts before file-level localization. std::vector _table_filters; + // Number of localized filters before the first unsafe conjunct in the original row-level + // order. This differs from scanning `_table_filters` for safety because slotless predicates are + // intentionally absent from that vector but must still act as ordering barriers. + size_t _constant_pruning_safe_filter_count = 0; VExprContextSPtrs _conjuncts; ReadProfile _profile; // Parsed from row-position based delete files, including position delete and deletion vector. diff --git a/be/src/format_v2/timestamp_statistics.h b/be/src/format_v2/timestamp_statistics.h index 2bbe7b99b908f7..6b13f44ab16220 100644 --- a/be/src/format_v2/timestamp_statistics.h +++ b/be/src/format_v2/timestamp_statistics.h @@ -39,7 +39,12 @@ inline int64_t floor_epoch_seconds(int64_t value, int64_t units_per_second) { // safely form a ZoneMap: the true civil minimum or maximum may occur inside the UTC interval. inline bool utc_timestamp_range_is_monotonic(int64_t min_seconds, int64_t max_seconds, const cctz::time_zone& timezone) { - DORIS_CHECK(min_seconds <= max_seconds); + // These endpoints come from external file metadata. An inverted range is corrupt rather than + // a valid precondition violation, so report it as unusable statistics and let every caller + // take its conservative scan/fallback path instead of terminating the BE. + if (min_seconds > max_seconds) { + return false; + } auto current = cctz::time_point(cctz::seconds(min_seconds)); const auto range_end = cctz::time_point(cctz::seconds(max_seconds)); cctz::time_zone::civil_transition transition; diff --git a/be/test/format_v2/table_reader_test.cpp b/be/test/format_v2/table_reader_test.cpp index 9c5c37235b0399..7f85ca53d9837e 100644 --- a/be/test/format_v2/table_reader_test.cpp +++ b/be/test/format_v2/table_reader_test.cpp @@ -1328,6 +1328,56 @@ TEST(TableReaderTest, ConstantPruningStopsAtUnsafePredicate) { ASSERT_TRUE(reader.close().ok()); } +TEST(TableReaderTest, ConstantPruningStopsAtUnsafeSlotlessPredicate) { + std::vector projected_columns; + auto partition_column = make_table_column(0, "part", std::make_shared()); + partition_column.is_partition_key = true; + projected_columns.push_back(std::move(partition_column)); + set_name_identifiers(&projected_columns); + + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + bool predicate_executed = false; + auto unsafe_slotless_predicate = + std::make_shared(&predicate_executed); + auto fake_state = std::make_shared(); + FakeTableReader reader({}, fake_state); + ASSERT_TRUE( + reader + .init({ + .projected_columns = projected_columns, + .conjuncts = + { + prepared_conjunct(&state, unsafe_slotless_predicate), + prepared_conjunct(&state, table_int32_greater_than_expr( + 0, 0, 10)), + }, + .format = FileFormat::PARQUET, + .scan_params = nullptr, + .io_ctx = nullptr, + .runtime_state = &state, + .scanner_profile = nullptr, + }) + .ok()); + + SplitReadOptions split; + split.current_range.__set_path("fake-table-reader-input"); + split.partition_values.emplace("part", Field::create_field(7)); + ASSERT_TRUE(reader.prepare_split(split).ok()); + + Block block = build_table_block(projected_columns); + bool eos = false; + ASSERT_TRUE(reader.get_block(&block, &eos).ok()); + // The unsafe expression must reach normal row-level evaluation; it must not be hidden by the + // later false constant predicate pruning the split first. + EXPECT_TRUE(predicate_executed); + EXPECT_FALSE(eos); + // The later partition predicate is false for part=7. Opening the file proves constant pruning + // stopped at the earlier unsafe expression even though that expression had no slot and thus no + // entry in `_table_filters`. + EXPECT_EQ(fake_state->open_count, 1); + ASSERT_TRUE(reader.close().ok()); +} + TEST(TableReaderTest, CanUseInjectedFileReaderForStandaloneUnitTest) { std::vector file_schema; file_schema.push_back(make_file_column(0, "id", std::make_shared())); diff --git a/be/test/format_v2/timestamp_statistics_test.cpp b/be/test/format_v2/timestamp_statistics_test.cpp index 8fd6e206086206..6c27b680dc2896 100644 --- a/be/test/format_v2/timestamp_statistics_test.cpp +++ b/be/test/format_v2/timestamp_statistics_test.cpp @@ -37,6 +37,9 @@ TEST(TimestampStatisticsTest, DetectsBackwardTimezoneTransitionsInUtcRange) { // The 2021 spring transition at 07:00 UTC skips civil values but preserves ordering. EXPECT_TRUE(utc_timestamp_range_is_monotonic(1615703400, 1615707000, new_york)); + + // Corrupt external statistics must disable pruning instead of asserting in the BE. + EXPECT_FALSE(utc_timestamp_range_is_monotonic(2, 1, new_york)); } TEST(TimestampStatisticsTest, FloorsNegativeEpochFractions) { From 3bdcb98952be25df1f63db823e427e19ebab3617 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 22:25:49 +0800 Subject: [PATCH 14/23] [fix](be) Reject unsafe file statistics and pushdown ### What problem does this PR solve? Issue Number: None Related PR: #65500 Problem Summary: File-local predicate localization could cross a slotless unsafe conjunct, inverted Parquet and ORC statistics could still drive pruning or aggregate results, and MIN/MAX pushdown could use endpoints from a non-order-preserving file-to-table projection. Stop localization at the original conjunct-order barrier, reject inverted decoded and raw timestamp bounds, and require a direct mapping without a projection for MIN/MAX pushdown. Add focused coverage for inverted statistics, ordering barriers, and cast fallback. ### Release note Prevent incorrect external file pruning and MIN/MAX results from corrupt statistics, unsafe predicate reordering, and non-order-preserving schema mappings. ### Check List (For Author) - Test: Unit Test - Remote ASAN BE unit tests: 4 focused predicate/statistics/mapping tests passed - Remote ASAN BE unit tests: 13 Parquet, ORC, and timestamp statistics tests passed - Remote clang-tidy on the six modified files - Behavior changed: Yes (unsafe metadata and mappings now fall back to normal scanning) - Does this need documentation: No --- be/src/format_v2/orc/orc_reader.cpp | 7 +++ .../format_v2/parquet/parquet_statistics.cpp | 26 ++++++-- be/src/format_v2/table_reader.cpp | 7 ++- be/src/format_v2/table_reader.h | 2 +- .../parquet/parquet_statistics_test.cpp | 60 +++++++++++++++++++ be/test/format_v2/table_reader_test.cpp | 22 +++++-- 6 files changed, 111 insertions(+), 13 deletions(-) diff --git a/be/src/format_v2/orc/orc_reader.cpp b/be/src/format_v2/orc/orc_reader.cpp index 6f9ea2e10aaaca..7d48b7eaacb463 100644 --- a/be/src/format_v2/orc/orc_reader.cpp +++ b/be/src/format_v2/orc/orc_reader.cpp @@ -525,6 +525,13 @@ bool set_timestamp_zone_map(const ::orc::ColumnStatistics& statistics, !timestamp_statistics->hasMaximum()) { return false; } + const auto min_endpoint = std::pair(timestamp_statistics->getMinimum(), + timestamp_statistics->getMinimumNanos()); + const auto max_endpoint = std::pair(timestamp_statistics->getMaximum(), + timestamp_statistics->getMaximumNanos()); + if (min_endpoint > max_endpoint) { + return false; + } if (use_timestamp_tz) { zone_map->min_value = Field::create_field(timestamp_tz_from_orc_millis( timestamp_statistics->getMinimum(), timestamp_statistics->getMinimumNanos())); diff --git a/be/src/format_v2/parquet/parquet_statistics.cpp b/be/src/format_v2/parquet/parquet_statistics.cpp index d2dde4c6f29af9..b1eba0cf74fe93 100644 --- a/be/src/format_v2/parquet/parquet_statistics.cpp +++ b/be/src/format_v2/parquet/parquet_statistics.cpp @@ -141,6 +141,9 @@ int64_t floor_timestamp_seconds(int64_t value, ParquetTimeUnit time_unit) { bool timestamp_min_max_is_safe(const ParquetColumnSchema& column_schema, int64_t min_value, int64_t max_value, const cctz::time_zone* timezone) { + if (min_value > max_value) { + return false; + } if (!column_schema.type_descriptor.is_timestamp || !column_schema.type_descriptor.timestamp_is_adjusted_to_utc || timezone == nullptr || remove_nullable(column_schema.type)->get_primitive_type() == TYPE_TIMESTAMPTZ) { @@ -157,11 +160,17 @@ template bool valid_min_max(const NativeType& min_value, const NativeType& max_value) { if constexpr (std::is_floating_point_v) { // Parquet requires readers to ignore min/max statistics if either bound is NaN. - return !std::isnan(min_value) && !std::isnan(max_value); + if (std::isnan(min_value) || std::isnan(max_value)) { + return false; + } } return true; } +bool decoded_min_max_is_ordered(const ParquetColumnStatistics& column_statistics) { + return !(column_statistics.max_value < column_statistics.min_value); +} + template bool set_decoded_min_max(const std::shared_ptr<::parquet::Statistics>& statistics, const ParquetColumnSchema& column_schema, DecodedValueKind value_kind, @@ -183,7 +192,7 @@ bool set_decoded_min_max(const std::shared_ptr<::parquet::Statistics>& statistic timezone)) { return false; } - return true; + return decoded_min_max_is_ordered(*column_statistics); } bool set_decoded_binary_field(const ParquetColumnSchema& column_schema, DecodedValueKind value_kind, @@ -215,7 +224,7 @@ bool set_string_min_max(const std::shared_ptr<::parquet::Statistics>& statistics &column_statistics->max_value, timezone)) { return false; } - return true; + return decoded_min_max_is_ordered(*column_statistics); } case ::parquet::Type::FIXED_LEN_BYTE_ARRAY: { if (column_schema.descriptor == nullptr || column_schema.descriptor->type_length() <= 0) { @@ -237,7 +246,7 @@ bool set_string_min_max(const std::shared_ptr<::parquet::Statistics>& statistics &column_statistics->max_value, timezone)) { return false; } - return true; + return decoded_min_max_is_ordered(*column_statistics); } default: return false; @@ -1052,6 +1061,9 @@ bool set_page_decoded_min_max(const std::shared_ptr<::parquet::ColumnIndex>& col timezone)) { return false; } + if (!decoded_min_max_is_ordered(*page_statistics)) { + return true; + } page_statistics->has_min_max = true; return true; } @@ -1078,6 +1090,9 @@ bool set_page_string_min_max(const std::shared_ptr<::parquet::ColumnIndex>& colu &page_statistics->max_value, timezone)) { return false; } + if (!decoded_min_max_is_ordered(*page_statistics)) { + return true; + } page_statistics->has_min_max = true; return true; } @@ -1105,6 +1120,9 @@ bool set_page_string_min_max(const std::shared_ptr<::parquet::ColumnIndex>& colu &page_statistics->max_value, timezone)) { return false; } + if (!decoded_min_max_is_ordered(*page_statistics)) { + return true; + } page_statistics->has_min_max = true; return true; } diff --git a/be/src/format_v2/table_reader.cpp b/be/src/format_v2/table_reader.cpp index 1b48127e608a99..fb446e653fa243 100644 --- a/be/src/format_v2/table_reader.cpp +++ b/be/src/format_v2/table_reader.cpp @@ -535,11 +535,12 @@ Status TableReader::_build_table_filters_from_conjuncts() { if (in_safe_prefix && !_is_safe_to_pre_execute(conjunct)) { in_safe_prefix = false; } + if (!in_safe_prefix) { + continue; + } RETURN_IF_ERROR( build_table_filters_from_conjunct(conjunct, _runtime_state, &_table_filters)); - if (in_safe_prefix) { - _constant_pruning_safe_filter_count = _table_filters.size(); - } + _constant_pruning_safe_filter_count = _table_filters.size(); } return Status::OK(); } diff --git a/be/src/format_v2/table_reader.h b/be/src/format_v2/table_reader.h index 44fdc9c16a73dc..2a4ffdafed1e35 100644 --- a/be/src/format_v2/table_reader.h +++ b/be/src/format_v2/table_reader.h @@ -1568,7 +1568,7 @@ class TableReader { static bool _can_push_down_minmax_for_mapping(const ColumnMapping& mapping) { if (mapping.child_mappings.empty()) { - return true; + return mapping.is_trivial && mapping.projection == nullptr; } const auto primitive_type = remove_nullable(mapping.file_type)->get_primitive_type(); if (primitive_type != TYPE_STRUCT) { diff --git a/be/test/format_v2/parquet/parquet_statistics_test.cpp b/be/test/format_v2/parquet/parquet_statistics_test.cpp index 5552c93b35a829..7998fbe0b20736 100644 --- a/be/test/format_v2/parquet/parquet_statistics_test.cpp +++ b/be/test/format_v2/parquet/parquet_statistics_test.cpp @@ -650,6 +650,66 @@ TEST(ParquetStatisticsTransformTest, IgnoresNaNFloatAndDoubleColumnIndexMinMax) EXPECT_EQ(following_page_stats.max_value.get(), 2.0); } +TEST(ParquetStatisticsTransformTest, IgnoresInvertedFooterAndColumnIndexMinMax) { + auto table = arrow::Table::Make( + arrow::schema({arrow::field("i", arrow::int32(), false), + arrow::field("s", arrow::utf8(), false), + arrow::field("ts", arrow::timestamp(arrow::TimeUnit::MICRO, "UTC"), + false)}), + {int32_array({1, 2}), string_array({"a", "z"}), timestamp_array({1000000, 2000000})}); + auto reader = make_reader(table, 2, false, true); + auto schema = build_file_schema(*reader); + + const int32_t inverted_min = 10; + const int32_t inverted_max = 1; + auto int_stats = ::parquet::MakeStatistics<::parquet::Int32Type>( + schema[0]->descriptor, encoded_value(inverted_min), encoded_value(inverted_max), 2, 0, + 0, true, true, false); + const auto converted_int = format::parquet::ParquetStatisticsUtils::TransformColumnStatistics( + *schema[0], int_stats); + EXPECT_TRUE(converted_int.has_not_null); + EXPECT_FALSE(converted_int.has_min_max); + + const std::string inverted_string_min = "z"; + const std::string inverted_string_max = "a"; + auto string_stats = ::parquet::MakeStatistics<::parquet::ByteArrayType>( + schema[1]->descriptor, inverted_string_min, inverted_string_max, 2, 0, 0, true, true, + false); + const auto converted_string = + format::parquet::ParquetStatisticsUtils::TransformColumnStatistics(*schema[1], + string_stats); + EXPECT_TRUE(converted_string.has_not_null); + EXPECT_FALSE(converted_string.has_min_max); + + auto int_index = std::make_shared>(inverted_min, + inverted_max); + format::parquet::ParquetColumnStatistics page_stats; + EXPECT_TRUE(format::parquet::ParquetStatisticsUtils::TransformColumnIndexStatistics( + int_index, *schema[0], 0, &page_stats)); + EXPECT_TRUE(page_stats.has_not_null); + EXPECT_FALSE(page_stats.has_min_max); + + // These endpoints are inverted within one second. Whole-second validation alone would miss + // the corruption, and TIMESTAMPTZ must reject the same raw inversion before its UTC shortcut. + constexpr int64_t timestamp_min = 1500000; + constexpr int64_t timestamp_max = 1000000; + auto timestamp_stats = ::parquet::MakeStatistics<::parquet::Int64Type>( + schema[2]->descriptor, encoded_value(timestamp_min), encoded_value(timestamp_max), 2, + 0, 0, true, true, false); + auto utc = cctz::utc_time_zone(); + const auto converted_timestamp = + format::parquet::ParquetStatisticsUtils::TransformColumnStatistics( + *schema[2], timestamp_stats, &utc); + EXPECT_FALSE(converted_timestamp.has_min_max); + + schema[2]->type = std::make_shared(6); + schema[2]->type_descriptor.doris_type = schema[2]->type; + const auto converted_timestamp_tz = + format::parquet::ParquetStatisticsUtils::TransformColumnStatistics( + *schema[2], timestamp_stats, &utc); + EXPECT_FALSE(converted_timestamp_tz.has_min_max); +} + TEST(ParquetStatisticsTransformTest, PreservesNullCountWhenNaNInvalidatesMinMax) { auto table = arrow::Table::Make(arrow::schema({arrow::field("f", arrow::float64(), false)}), {double_array({1.0, 2.0})}); diff --git a/be/test/format_v2/table_reader_test.cpp b/be/test/format_v2/table_reader_test.cpp index 7f85ca53d9837e..e92a75ff616aab 100644 --- a/be/test/format_v2/table_reader_test.cpp +++ b/be/test/format_v2/table_reader_test.cpp @@ -1367,14 +1367,17 @@ TEST(TableReaderTest, ConstantPruningStopsAtUnsafeSlotlessPredicate) { Block block = build_table_block(projected_columns); bool eos = false; ASSERT_TRUE(reader.get_block(&block, &eos).ok()); - // The unsafe expression must reach normal row-level evaluation; it must not be hidden by the - // later false constant predicate pruning the split first. EXPECT_TRUE(predicate_executed); EXPECT_FALSE(eos); // The later partition predicate is false for part=7. Opening the file proves constant pruning // stopped at the earlier unsafe expression even though that expression had no slot and thus no // entry in `_table_filters`. EXPECT_EQ(fake_state->open_count, 1); + ASSERT_NE(fake_state->last_request, nullptr); + // A slotless unsafe conjunct is an ordering barrier even though it has no TableFilter entry. + // The later predicate must stay on the scanner's row-level path instead of running inside the + // file reader before the unsafe conjunct. + EXPECT_TRUE(fake_state->last_request->conjuncts.empty()); ASSERT_TRUE(reader.close().ok()); } @@ -2229,7 +2232,7 @@ TEST(TableReaderTest, PushDownMinMaxFromNewParquetReader) { std::filesystem::remove_all(test_dir); } -TEST(TableReaderTest, PushDownMinMaxCastsFileValueToTableType) { +TEST(TableReaderTest, PushDownMinMaxFallsBackForFileToTableCast) { const auto test_dir = std::filesystem::temp_directory_path() / "doris_table_reader_minmax_cast_test"; std::filesystem::remove_all(test_dir); @@ -2265,8 +2268,17 @@ TEST(TableReaderTest, PushDownMinMaxCastsFileValueToTableType) { ASSERT_FALSE(eos); ASSERT_EQ(block.rows(), 2); const auto& id_column = assert_cast(expect_not_null_table_column(block, 0)); - EXPECT_EQ(id_column.get_element(0), 1); - EXPECT_EQ(id_column.get_element(1), 5); + EXPECT_EQ(id_column.get_element(0), 3); + EXPECT_EQ(id_column.get_element(1), 1); + + block = build_table_block(projected_columns); + ASSERT_TRUE(reader.get_block(&block, &eos).ok()); + ASSERT_FALSE(eos); + ASSERT_EQ(block.rows(), 2); + const auto& second_id_column = + assert_cast(expect_not_null_table_column(block, 0)); + EXPECT_EQ(second_id_column.get_element(0), 5); + EXPECT_EQ(second_id_column.get_element(1), 2); ASSERT_TRUE(reader.close().ok()); std::filesystem::remove_all(test_dir); From f5da27fff5332530f334a86e8a53a12692993799 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 22:43:00 +0800 Subject: [PATCH 15/23] update --- be/src/format_v2/orc/orc_reader.cpp | 8 ++++---- .../parquet/parquet_statistics_test.cpp | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/be/src/format_v2/orc/orc_reader.cpp b/be/src/format_v2/orc/orc_reader.cpp index 7d48b7eaacb463..fd10a777388995 100644 --- a/be/src/format_v2/orc/orc_reader.cpp +++ b/be/src/format_v2/orc/orc_reader.cpp @@ -525,10 +525,10 @@ bool set_timestamp_zone_map(const ::orc::ColumnStatistics& statistics, !timestamp_statistics->hasMaximum()) { return false; } - const auto min_endpoint = std::pair(timestamp_statistics->getMinimum(), - timestamp_statistics->getMinimumNanos()); - const auto max_endpoint = std::pair(timestamp_statistics->getMaximum(), - timestamp_statistics->getMaximumNanos()); + const auto min_endpoint = + std::pair(timestamp_statistics->getMinimum(), timestamp_statistics->getMinimumNanos()); + const auto max_endpoint = + std::pair(timestamp_statistics->getMaximum(), timestamp_statistics->getMaximumNanos()); if (min_endpoint > max_endpoint) { return false; } diff --git a/be/test/format_v2/parquet/parquet_statistics_test.cpp b/be/test/format_v2/parquet/parquet_statistics_test.cpp index 7998fbe0b20736..dd2138279b11d1 100644 --- a/be/test/format_v2/parquet/parquet_statistics_test.cpp +++ b/be/test/format_v2/parquet/parquet_statistics_test.cpp @@ -652,10 +652,10 @@ TEST(ParquetStatisticsTransformTest, IgnoresNaNFloatAndDoubleColumnIndexMinMax) TEST(ParquetStatisticsTransformTest, IgnoresInvertedFooterAndColumnIndexMinMax) { auto table = arrow::Table::Make( - arrow::schema({arrow::field("i", arrow::int32(), false), - arrow::field("s", arrow::utf8(), false), - arrow::field("ts", arrow::timestamp(arrow::TimeUnit::MICRO, "UTC"), - false)}), + arrow::schema( + {arrow::field("i", arrow::int32(), false), + arrow::field("s", arrow::utf8(), false), + arrow::field("ts", arrow::timestamp(arrow::TimeUnit::MICRO, "UTC"), false)}), {int32_array({1, 2}), string_array({"a", "z"}), timestamp_array({1000000, 2000000})}); auto reader = make_reader(table, 2, false, true); auto schema = build_file_schema(*reader); @@ -681,8 +681,8 @@ TEST(ParquetStatisticsTransformTest, IgnoresInvertedFooterAndColumnIndexMinMax) EXPECT_TRUE(converted_string.has_not_null); EXPECT_FALSE(converted_string.has_min_max); - auto int_index = std::make_shared>(inverted_min, - inverted_max); + auto int_index = + std::make_shared>(inverted_min, inverted_max); format::parquet::ParquetColumnStatistics page_stats; EXPECT_TRUE(format::parquet::ParquetStatisticsUtils::TransformColumnIndexStatistics( int_index, *schema[0], 0, &page_stats)); @@ -694,8 +694,8 @@ TEST(ParquetStatisticsTransformTest, IgnoresInvertedFooterAndColumnIndexMinMax) constexpr int64_t timestamp_min = 1500000; constexpr int64_t timestamp_max = 1000000; auto timestamp_stats = ::parquet::MakeStatistics<::parquet::Int64Type>( - schema[2]->descriptor, encoded_value(timestamp_min), encoded_value(timestamp_max), 2, - 0, 0, true, true, false); + schema[2]->descriptor, encoded_value(timestamp_min), encoded_value(timestamp_max), 2, 0, + 0, true, true, false); auto utc = cctz::utc_time_zone(); const auto converted_timestamp = format::parquet::ParquetStatisticsUtils::TransformColumnStatistics( From 108e69bfa46f240441c50c6cc66d81819ad706bf Mon Sep 17 00:00:00 2001 From: Gabriel Date: Mon, 13 Jul 2026 10:01:04 +0800 Subject: [PATCH 16/23] [fix](be) Keep VARBINARY filters above file readers ### What problem does this PR solve? Issue Number: None Related PR: #65500 Problem Summary: Direct VARBINARY mappings localized late runtime filters into external file readers even though those readers intentionally do not support VARBINARY predicate pushdown. This could reject matching Iceberg BINARY partition rows. Keep VARBINARY filters for scanner-level evaluation after materialization. ### Release note Fix incorrect results for runtime filters on external VARBINARY columns, including Iceberg BINARY partition columns. ### Check List (For Author) - Test: Unit Test - Remote ASAN BE unit tests: ColumnMapperLocalizeFiltersTest.* (5 passed) - Behavior changed: Yes (VARBINARY runtime filters stay on scanner-level evaluation path) - Does this need documentation: No --- be/src/format_v2/column_mapper.cpp | 15 ++++++++++-- be/test/format_v2/column_mapper_test.cpp | 30 ++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/be/src/format_v2/column_mapper.cpp b/be/src/format_v2/column_mapper.cpp index 895e2fe2c91270..baa156c44dfe6b 100644 --- a/be/src/format_v2/column_mapper.cpp +++ b/be/src/format_v2/column_mapper.cpp @@ -1078,6 +1078,18 @@ static bool mapping_can_use_file_column_directly(const ColumnMapping& mapping) { return !needs_complex_rematerialize(mapping); } +static FilterConversionType direct_filter_conversion(const ColumnMapping& mapping) { + DORIS_CHECK(mapping.table_type != nullptr); + // FileScanOperator deliberately keeps VARBINARY predicates above external readers. Their + // physical binary representations are not uniformly supported by reader-side expression and + // metadata filtering, so localizing a late runtime filter here can incorrectly reject rows. + if (remove_nullable(mapping.table_type)->get_primitive_type() == TYPE_VARBINARY) { + return FilterConversionType::FINALIZE_ONLY; + } + return mapping.is_trivial ? FilterConversionType::COPY_DIRECTLY + : FilterConversionType::CAST_FILTER; +} + static const ColumnDefinition* find_file_child_for_mapping(const ColumnDefinition& table_child, const ColumnDefinition& file_parent, TableColumnMappingMode mode, @@ -1958,8 +1970,7 @@ Status TableColumnMapper::_create_direct_mapping(const ColumnDefinition& table_c mapping->projected_file_children = file_field.children; mapping->file_type = file_field.type; mapping->is_trivial = mapping_can_use_file_column_directly(*mapping); - mapping->filter_conversion = mapping->is_trivial ? FilterConversionType::COPY_DIRECTLY - : FilterConversionType::CAST_FILTER; + mapping->filter_conversion = direct_filter_conversion(*mapping); mapping->child_mappings.clear(); auto table_children = table_column.children; diff --git a/be/test/format_v2/column_mapper_test.cpp b/be/test/format_v2/column_mapper_test.cpp index 90d4e7b8d552d8..29b522aee6ccbb 100644 --- a/be/test/format_v2/column_mapper_test.cpp +++ b/be/test/format_v2/column_mapper_test.cpp @@ -37,6 +37,7 @@ #include "core/data_type/data_type_string.h" #include "core/data_type/data_type_struct.h" #include "core/data_type/data_type_timestamptz.h" +#include "core/data_type/data_type_varbinary.h" #include "exprs/vexpr.h" #include "exprs/vexpr_context.h" #include "exprs/vin_predicate.h" @@ -78,6 +79,10 @@ DataTypePtr str() { return std::make_shared(); } +DataTypePtr varbinary() { + return std::make_shared(); +} + DataTypePtr timestamptz(uint32_t scale) { return std::make_shared(scale); } @@ -2157,6 +2162,31 @@ TEST(ColumnMapperLocalizeFiltersTest, VisibleLocalFilterAddsPredicateColumnAndCo EXPECT_TRUE(localized_slot->data_type()->equals(*int_type)); } +TEST(ColumnMapperLocalizeFiltersTest, VarbinaryFilterStaysAboveFileReader) { + const auto binary_type = varbinary(); + const auto table_column = name_col("partition_key", binary_type); + const auto file_column = name_col("partition_key", binary_type, 7); + + TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_NAME}); + ASSERT_TRUE(mapper.create_mapping({table_column}, {}, {file_column}).ok()); + ASSERT_EQ(mapper.mappings().size(), 1); + EXPECT_TRUE(mapper.mappings()[0].is_trivial); + EXPECT_EQ(mapper.mappings()[0].filter_conversion, FilterConversionType::FINALIZE_ONLY); + + const auto value = Field::create_field(StringView("binary-value")); + TableFilter filter {.conjunct = VExprContext::create_shared(binary_predicate( + TExprOpcode::EQ, table_slot(0, 0, binary_type, "partition_key"), + literal(binary_type, value))), + .global_indices = {GlobalIndex(0)}}; + + FileScanRequest request; + ASSERT_TRUE(mapper.create_scan_request({filter}, {table_column}, &request).ok()); + EXPECT_TRUE(request.predicate_columns.empty()); + ASSERT_EQ(request.non_predicate_columns.size(), 1); + EXPECT_EQ(request.non_predicate_columns[0].column_id(), LocalColumnId(7)); + EXPECT_TRUE(request.conjuncts.empty()); +} + TEST(ColumnMapperLocalizeFiltersTest, ConstantFilterBuildsEntryWithoutFileScanColumn) { auto partition_column = name_col("part", i32()); partition_column.is_partition_key = true; From 92e77db5110b3526e83c73c85d278eea1136d2d1 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 15:06:40 +0800 Subject: [PATCH 17/23] [fix](be) Refresh V2 file predicates for each split ### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: FileScannerV2 cloned its table-level conjuncts only when TableReader was initialized. Runtime filters arriving after scanner open were visible to the scanner but never reached subsequent split readers, so Parquet and ORC metadata pruning, native file filtering, and JNI block filtering could not benefit from late runtime filters. Pass a freshly cloned conjunct snapshot into every split, replace TableReader's initial snapshot during split preparation, and prepare JNI conjuncts per split. ### Release note Enable late runtime filters to participate in FileScannerV2 file-level filtering for subsequent splits. ### Check List (For Author) - Test: Unit Test - `TableReaderTest.PrepareSplitReplacesInitialConjunctSnapshot` - `FileScannerV2Test.RewriteSlotRefsToGlobalIndexMatrix` - Behavior changed: Yes. Subsequent FileScannerV2 splits use the latest runtime-filter snapshot for native and JNI filtering. - Does this need documentation: No --- be/src/exec/scan/file_scanner_v2.cpp | 3 ++ be/src/format_v2/jni/jni_table_reader.cpp | 15 +++++---- be/src/format_v2/table_reader.cpp | 3 ++ be/src/format_v2/table_reader.h | 8 +++-- be/test/format_v2/table_reader_test.cpp | 38 +++++++++++++++++++++++ 5 files changed, 57 insertions(+), 10 deletions(-) diff --git a/be/src/exec/scan/file_scanner_v2.cpp b/be/src/exec/scan/file_scanner_v2.cpp index 10b5f850ea36f7..f3231ecd7d420b 100644 --- a/be/src/exec/scan/file_scanner_v2.cpp +++ b/be/src/exec/scan/file_scanner_v2.cpp @@ -492,12 +492,15 @@ Status FileScannerV2::_prepare_table_reader_split(const TFileRangeDesc& range, std::map partition_values) { format::FileFormat current_split_format; RETURN_IF_ERROR(_to_file_format(get_range_format_type(*_params, range), ¤t_split_format)); + VExprContextSPtrs conjuncts; + RETURN_IF_ERROR(_build_table_conjuncts(&conjuncts)); VExprContextSPtrs partition_prune_conjuncts; if (_state->query_options().enable_runtime_filter_partition_prune) { RETURN_IF_ERROR(_build_table_conjuncts(&partition_prune_conjuncts)); } RETURN_IF_ERROR(_table_reader->prepare_split({ .partition_values = std::move(partition_values), + .conjuncts = std::move(conjuncts), .partition_prune_conjuncts = std::move(partition_prune_conjuncts), .cache = _kv_cache, .current_range = range, diff --git a/be/src/format_v2/jni/jni_table_reader.cpp b/be/src/format_v2/jni/jni_table_reader.cpp index 7cec22a7c5999f..9416659b8f5bfc 100644 --- a/be/src/format_v2/jni/jni_table_reader.cpp +++ b/be/src/format_v2/jni/jni_table_reader.cpp @@ -32,14 +32,6 @@ namespace doris::format { Status JniTableReader::init(TableReadOptions&& options) { RETURN_IF_ERROR(TableReader::init(std::move(options))); _init_profile(); - - // JNI readers do not go through TableReader::open_reader(), where file-local filters are - // prepared for file readers. They execute table-level conjuncts directly on the JNI block. - RowDescriptor row_desc; - for (const auto& conjunct : _conjuncts) { - RETURN_IF_ERROR(conjunct->prepare(_runtime_state, row_desc)); - RETURN_IF_ERROR(conjunct->open(_runtime_state)); - } return Status::OK(); } @@ -55,6 +47,13 @@ Status JniTableReader::prepare_split(const SplitReadOptions& options) { if (_is_table_level_count_active()) { return Status::OK(); } + // JNI readers do not go through TableReader::open_reader(), where native readers prepare + // file-local filters. Prepare the fresh per-split snapshot before it filters JNI blocks. + RowDescriptor row_desc; + for (const auto& conjunct : _conjuncts) { + RETURN_IF_ERROR(conjunct->prepare(_runtime_state, row_desc)); + RETURN_IF_ERROR(conjunct->open(_runtime_state)); + } // Subclasses populate split-specific scanner params before calling this method, so the Java // scanner can be opened here instead of being lazily opened by the first get_block() call. return _open_jni_scanner(); diff --git a/be/src/format_v2/table_reader.cpp b/be/src/format_v2/table_reader.cpp index fb446e653fa243..f71393f9988588 100644 --- a/be/src/format_v2/table_reader.cpp +++ b/be/src/format_v2/table_reader.cpp @@ -765,6 +765,9 @@ std::unique_ptr create_file_description(const TFileRangeDes Status TableReader::prepare_split(const SplitReadOptions& options) { SCOPED_TIMER(_profile.prepare_split_timer); _current_split_pruned = false; + if (options.conjuncts.has_value()) { + _conjuncts = *options.conjuncts; + } // Update to current split format to handle ORC/PARQUET files in one table. _format = options.current_split_format; _partition_values = std::move(options.partition_values); diff --git a/be/src/format_v2/table_reader.h b/be/src/format_v2/table_reader.h index 2a4ffdafed1e35..c65ebeec5d7df9 100644 --- a/be/src/format_v2/table_reader.h +++ b/be/src/format_v2/table_reader.h @@ -142,8 +142,12 @@ struct TableReadOptions { struct SplitReadOptions { // Split-level information for reader initialization, which may include file path, partition values, delete file info, etc. The content is table format specific and opaque to table reader base class; it's the responsibility of the concrete table reader implementation to parse necessary information for reader initialization and filter pushdown. std::map partition_values; - // Latest scanner conjuncts rewritten to table/global column indices. Runtime filters can - // arrive after TableReader::init(), so split preparation must receive a fresh snapshot. + // Latest scanner conjuncts rewritten to table/global column indices. Runtime filters may + // arrive after TableReader::init(), so scanner-driven splits replace the initial snapshot. + // nullopt preserves the initial snapshot for standalone TableReader callers. + std::optional conjuncts; + // Independent clones used for partition pruning because evaluation prepares and opens them + // against a synthetic partition block before the file reader opens its row-level conjuncts. VExprContextSPtrs partition_prune_conjuncts; ShardedKVCache* cache; TFileRangeDesc current_range; diff --git a/be/test/format_v2/table_reader_test.cpp b/be/test/format_v2/table_reader_test.cpp index e92a75ff616aab..d59ffd3f20468e 100644 --- a/be/test/format_v2/table_reader_test.cpp +++ b/be/test/format_v2/table_reader_test.cpp @@ -1439,6 +1439,44 @@ TEST(TableReaderTest, CanUseInjectedFileReaderForStandaloneUnitTest) { EXPECT_TRUE(eos); } +TEST(TableReaderTest, PrepareSplitReplacesInitialConjunctSnapshot) { + std::vector file_schema; + file_schema.push_back(make_file_column(0, "id", std::make_shared())); + + std::vector projected_columns; + projected_columns.push_back(make_table_column(0, "id", std::make_shared())); + set_name_identifiers(&projected_columns); + + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + auto fake_state = std::make_shared(); + FakeTableReader reader(file_schema, fake_state); + ASSERT_TRUE(reader.init({ + .projected_columns = projected_columns, + .conjuncts = {VExprContext::create_shared( + table_int32_greater_than_expr(0, 0, 0))}, + .format = FileFormat::PARQUET, + .scan_params = nullptr, + .io_ctx = nullptr, + .runtime_state = &state, + .scanner_profile = nullptr, + }) + .ok()); + + SplitReadOptions split_options; + split_options.current_range.__set_path("fake-table-reader-input"); + split_options.conjuncts = VExprContextSPtrs {VExprContext::create_shared( + runtime_filter_wrapper_expr(table_int32_greater_than_expr(0, 0, 1)))}; + ASSERT_TRUE(reader.prepare_split(split_options).ok()); + + Block block = build_table_block(projected_columns); + bool eos = false; + ASSERT_TRUE(reader.get_block(&block, &eos).ok()); + ASSERT_NE(fake_state->last_request, nullptr); + ASSERT_EQ(fake_state->last_request->conjuncts.size(), 1); + EXPECT_TRUE(fake_state->last_request->conjuncts.front()->root()->is_rf_wrapper()); + ASSERT_TRUE(reader.close().ok()); +} + TEST(TableReaderTest, AbortSplitClearsReaderAfterIgnorableNotFound) { std::vector file_schema; file_schema.push_back(make_file_column(0, "id", std::make_shared())); From 10bd67e9763c3664fbe62a3c9e9b230f9e028761 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 20:43:28 +0800 Subject: [PATCH 18/23] [fix](be) Bound nested Parquet skip memory ### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Complex Parquet column readers materialized every discarded row in a selection gap into one scratch column, so memory and decoding work grew with the entire gap. In addition, a refreshed split predicate could be bypassed by the table-level COUNT shortcut. Materialize skipped nested rows in bounded batches with per-batch scratch columns, and only use metadata row counts when the split has no active conjuncts. ### Release note Bound memory usage while skipping nested Parquet columns and preserve late runtime-filter semantics for table-level COUNT. ### Check List (For Author) - Test: Unit Test - `ParquetColumnReaderControlTest.*` (16 tests) - `TableReaderTest.PrepareSplitReplacesInitialConjunctSnapshot` - `TableReaderTest.RefreshedConjunctDisablesTableLevelCount` - Behavior changed: Yes. Nested Parquet skip work is processed in bounded batches, and table-level COUNT no longer bypasses active split predicates. - Does this need documentation: No --- .../parquet/reader/column_reader.cpp | 28 ++++++++ .../format_v2/parquet/reader/column_reader.h | 4 ++ .../parquet/reader/list_column_reader.cpp | 14 +--- .../parquet/reader/map_column_reader.cpp | 14 +--- .../parquet/reader/struct_column_reader.cpp | 14 +--- be/src/format_v2/table_reader.cpp | 5 +- .../parquet/parquet_reader_control_test.cpp | 72 +++++++++++++++++++ be/test/format_v2/table_reader_test.cpp | 41 +++++++++++ 8 files changed, 152 insertions(+), 40 deletions(-) diff --git a/be/src/format_v2/parquet/reader/column_reader.cpp b/be/src/format_v2/parquet/reader/column_reader.cpp index 1b6e66beefe860..80a95cf17ee171 100644 --- a/be/src/format_v2/parquet/reader/column_reader.cpp +++ b/be/src/format_v2/parquet/reader/column_reader.cpp @@ -617,6 +617,34 @@ Status ParquetColumnReader::skip_nested_column(int64_t rows) { return Status::OK(); } +Status ParquetColumnReader::skip_nested_rows(int64_t rows) { + if (rows <= 0) { + return Status::OK(); + } + + // A nested parent row may expand to many child values. Capping the number of parent rows per + // scratch column bounds the amplification for large holes while preserving the same nested + // level decoding and validation path as a normal read. Recreate the scratch column per batch + // so its child buffers are released before decoding the next batch. + constexpr int64_t MAX_NESTED_SKIP_BATCH_SIZE = 4096; + int64_t remaining_rows = rows; + while (remaining_rows > 0) { + const int64_t batch_rows = std::min(remaining_rows, MAX_NESTED_SKIP_BATCH_SIZE); + auto scratch_column = _type->create_column(); + RETURN_IF_ERROR(load_nested_batch(batch_rows)); + int64_t rows_read = 0; + RETURN_IF_ERROR(build_nested_column(batch_rows, scratch_column, &rows_read)); + if (rows_read != batch_rows) { + return Status::Corruption( + "Failed to skip nested parquet column {}: skipped {} of {} rows in batch", + _name, rows_read, batch_rows); + } + remaining_rows -= batch_rows; + } + update_reader_skip_rows(rows); + return Status::OK(); +} + const std::vector& ParquetColumnReader::nested_definition_levels() const { static const std::vector empty; return empty; diff --git a/be/src/format_v2/parquet/reader/column_reader.h b/be/src/format_v2/parquet/reader/column_reader.h index 1d63e03ca9cf43..7072a8c7f28bad 100644 --- a/be/src/format_v2/parquet/reader/column_reader.h +++ b/be/src/format_v2/parquet/reader/column_reader.h @@ -109,6 +109,10 @@ class ParquetColumnReader { ParquetColumnReader(const ParquetColumnSchema& schema, const DataTypePtr type, ParquetColumnReaderProfile profile = {}); ParquetColumnReader() = default; + // Complex readers cannot advance their child streams without rebuilding parent boundaries + // from definition/repetition levels. Materialize that discarded shape in bounded batches so + // a large selection gap does not allocate a scratch column for the entire gap at once. + Status skip_nested_rows(int64_t rows); void update_reader_read_rows(int64_t rows) const; void update_reader_skip_rows(int64_t rows) const; diff --git a/be/src/format_v2/parquet/reader/list_column_reader.cpp b/be/src/format_v2/parquet/reader/list_column_reader.cpp index aaf8f6635f1af0..3caac5b2ac6d6a 100644 --- a/be/src/format_v2/parquet/reader/list_column_reader.cpp +++ b/be/src/format_v2/parquet/reader/list_column_reader.cpp @@ -47,19 +47,7 @@ Status ListColumnReader::read(int64_t rows, MutableColumnPtr& column, int64_t* r } Status ListColumnReader::skip(int64_t rows) { - if (rows <= 0) { - return Status::OK(); - } - auto scratch_column = _type->create_column(); - RETURN_IF_ERROR(load_nested_batch(rows)); - int64_t rows_read = 0; - RETURN_IF_ERROR(build_nested_column(rows, scratch_column, &rows_read)); - if (rows_read != rows) { - return Status::Corruption("Failed to skip parquet LIST column {}: skipped {} of {} rows", - _name, rows_read, rows); - } - update_reader_skip_rows(rows); - return Status::OK(); + return skip_nested_rows(rows); } Status ListColumnReader::load_nested_batch(int64_t rows) { diff --git a/be/src/format_v2/parquet/reader/map_column_reader.cpp b/be/src/format_v2/parquet/reader/map_column_reader.cpp index 90d4a867331190..c5a7e0e47f9e13 100644 --- a/be/src/format_v2/parquet/reader/map_column_reader.cpp +++ b/be/src/format_v2/parquet/reader/map_column_reader.cpp @@ -49,19 +49,7 @@ Status MapColumnReader::read(int64_t rows, MutableColumnPtr& column, int64_t* ro } Status MapColumnReader::skip(int64_t rows) { - if (rows <= 0) { - return Status::OK(); - } - auto scratch_column = _type->create_column(); - RETURN_IF_ERROR(load_nested_batch(rows)); - int64_t rows_read = 0; - RETURN_IF_ERROR(build_nested_column(rows, scratch_column, &rows_read)); - if (rows_read != rows) { - return Status::Corruption("Failed to skip parquet MAP column {}: skipped {} of {} rows", - _name, rows_read, rows); - } - update_reader_skip_rows(rows); - return Status::OK(); + return skip_nested_rows(rows); } Status MapColumnReader::load_nested_batch(int64_t rows) { diff --git a/be/src/format_v2/parquet/reader/struct_column_reader.cpp b/be/src/format_v2/parquet/reader/struct_column_reader.cpp index 66e450c567133a..8d69f75f8d3431 100644 --- a/be/src/format_v2/parquet/reader/struct_column_reader.cpp +++ b/be/src/format_v2/parquet/reader/struct_column_reader.cpp @@ -90,19 +90,7 @@ Status StructColumnReader::read(int64_t rows, MutableColumnPtr& column, int64_t* } Status StructColumnReader::skip(int64_t rows) { - if (rows <= 0) { - return Status::OK(); - } - auto scratch_column = _type->create_column(); - RETURN_IF_ERROR(load_nested_batch(rows)); - int64_t rows_read = 0; - RETURN_IF_ERROR(build_nested_column(rows, scratch_column, &rows_read)); - if (rows_read != rows) { - return Status::Corruption("Failed to skip parquet STRUCT column {}: skipped {} of {} rows", - _name, rows_read, rows); - } - update_reader_skip_rows(rows); - return Status::OK(); + return skip_nested_rows(rows); } Status StructColumnReader::load_nested_batch(int64_t rows) { diff --git a/be/src/format_v2/table_reader.cpp b/be/src/format_v2/table_reader.cpp index f71393f9988588..f5e478d8d6f45d 100644 --- a/be/src/format_v2/table_reader.cpp +++ b/be/src/format_v2/table_reader.cpp @@ -795,7 +795,10 @@ Status TableReader::prepare_split(const SplitReadOptions& options) { _current_task = std::make_unique(); _current_task->data_file = create_file_description(options.current_range); _current_file_description = *_current_task->data_file; - if (_push_down_agg_type == TPushAggOp::type::COUNT && + // A table-level row count is only equivalent to scanning the split when no row predicate is + // active. In particular, a runtime filter may arrive after init() and replace `_conjuncts` + // above. Returning synthetic rows here would bypass that fresh split snapshot completely. + if (_push_down_agg_type == TPushAggOp::type::COUNT && _conjuncts.empty() && options.current_range.__isset.table_format_params && options.current_range.table_format_params.__isset.table_level_row_count) { DORIS_CHECK(options.current_range.table_format_params.table_level_row_count >= -1); diff --git a/be/test/format_v2/parquet/parquet_reader_control_test.cpp b/be/test/format_v2/parquet/parquet_reader_control_test.cpp index c7d430350d1b26..ca19f096924e90 100644 --- a/be/test/format_v2/parquet/parquet_reader_control_test.cpp +++ b/be/test/format_v2/parquet/parquet_reader_control_test.cpp @@ -315,6 +315,64 @@ class ScriptedNestedReader final : public ParquetColumnReader { std::vector _build_lengths; }; +class ChunkedNestedLeafReader final : public ParquetColumnReader { +public: + ChunkedNestedLeafReader() + : ParquetColumnReader(nested_int64_schema("element", 0, 1, 1, 1), + std::make_shared()) {} + + Status read(int64_t, MutableColumnPtr&, int64_t*) override { + return Status::NotSupported("unused"); + } + + Status load_nested_batch(int64_t rows) override { + _load_lengths.push_back(rows); + _def_levels.assign(static_cast(rows), 1); + _rep_levels.assign(static_cast(rows), 0); + return Status::OK(); + } + + Status build_nested_column(int64_t length_upper_bound, MutableColumnPtr& column, + int64_t* values_read) override { + DORIS_CHECK(column.get() != nullptr); + DORIS_CHECK(values_read != nullptr); + _initial_column_sizes.push_back(column->size()); + _build_lengths.push_back(length_upper_bound); + if (auto* nullable = check_and_get_column(*column); nullable != nullptr) { + auto& values = assert_cast(nullable->get_nested_column()); + for (int64_t row = 0; row < length_upper_bound; ++row) { + values.insert_value(row); + nullable->get_null_map_data().push_back(0); + } + } else { + auto* values = assert_cast(column.get()); + for (int64_t row = 0; row < length_upper_bound; ++row) { + values->insert_value(row); + } + } + *values_read = length_upper_bound; + return Status::OK(); + } + + const std::vector& nested_definition_levels() const override { return _def_levels; } + const std::vector& nested_repetition_levels() const override { return _rep_levels; } + int64_t nested_levels_written() const override { + return static_cast(_def_levels.size()); + } + bool is_or_has_repeated_child() const override { return true; } + + const std::vector& load_lengths() const { return _load_lengths; } + const std::vector& build_lengths() const { return _build_lengths; } + const std::vector& initial_column_sizes() const { return _initial_column_sizes; } + +private: + std::vector _def_levels; + std::vector _rep_levels; + std::vector _load_lengths; + std::vector _build_lengths; + std::vector _initial_column_sizes; +}; + } // namespace struct ScalarColumnReaderTestAccess { @@ -464,6 +522,20 @@ TEST(ParquetColumnReaderControlTest, BaseNestedDefaultsAndSkipNested) { EXPECT_FALSE(short_reader.skip_nested_column(3).ok()); } +TEST(ParquetColumnReaderControlTest, NestedSkipMaterializesBoundedBatches) { + auto element_reader = std::make_unique(); + auto* element_reader_ptr = element_reader.get(); + ListColumnReader reader(bare_repeated_int64_list_schema(), + bare_repeated_int64_list_schema().type, std::move(element_reader)); + + ASSERT_TRUE(reader.skip(8193).ok()); + EXPECT_EQ(element_reader_ptr->load_lengths(), std::vector({4096, 4096, 1})); + EXPECT_EQ(element_reader_ptr->build_lengths(), std::vector({4096, 4096, 1})); + // Each child build sees an empty destination, proving the parent scratch column from the + // previous batch was destroyed instead of retaining all skipped nested values. + EXPECT_EQ(element_reader_ptr->initial_column_sizes(), std::vector({0, 0, 0})); +} + TEST(ParquetColumnReaderControlTest, NestedMaterializerHelpersAppendOffsetsAndParentNulls) { ColumnArray::Offsets64 offsets; append_offsets(offsets, {3, 0, 2}); diff --git a/be/test/format_v2/table_reader_test.cpp b/be/test/format_v2/table_reader_test.cpp index d59ffd3f20468e..2f85fc470414f4 100644 --- a/be/test/format_v2/table_reader_test.cpp +++ b/be/test/format_v2/table_reader_test.cpp @@ -1477,6 +1477,47 @@ TEST(TableReaderTest, PrepareSplitReplacesInitialConjunctSnapshot) { ASSERT_TRUE(reader.close().ok()); } +TEST(TableReaderTest, RefreshedConjunctDisablesTableLevelCount) { + std::vector file_schema; + file_schema.push_back(make_file_column(0, "id", std::make_shared())); + + std::vector projected_columns; + projected_columns.push_back(make_table_column(0, "id", std::make_shared())); + set_name_identifiers(&projected_columns); + + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + auto fake_state = std::make_shared(); + FakeTableReader reader(file_schema, fake_state); + ASSERT_TRUE(reader.init({ + .projected_columns = projected_columns, + .conjuncts = {}, + .format = FileFormat::PARQUET, + .scan_params = nullptr, + .io_ctx = nullptr, + .runtime_state = &state, + .scanner_profile = nullptr, + .push_down_agg_type = TPushAggOp::type::COUNT, + }) + .ok()); + + SplitReadOptions split_options; + split_options.current_range.__set_path("fake-table-reader-input"); + split_options.conjuncts = VExprContextSPtrs {VExprContext::create_shared( + runtime_filter_wrapper_expr(table_int32_greater_than_expr(0, 0, 1)))}; + set_table_level_row_count(&split_options, 5); + ASSERT_TRUE(reader.prepare_split(split_options).ok()); + + Block block = build_table_block(projected_columns); + bool eos = false; + ASSERT_TRUE(reader.get_block(&block, &eos).ok()); + // The metadata count advertises five rows, while the fake reader contains two. Opening the + // reader and returning its rows proves the fresh runtime filter did not take the synthetic + // table-level COUNT path that would bypass all row predicates. + EXPECT_EQ(fake_state->open_count, 1); + EXPECT_EQ(block.rows(), 2); + ASSERT_TRUE(reader.close().ok()); +} + TEST(TableReaderTest, AbortSplitClearsReaderAfterIgnorableNotFound) { std::vector file_schema; file_schema.push_back(make_file_column(0, "id", std::make_shared())); From bf482f13cfc20f2d5cf908d3eb719cca4747b3a4 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 21:31:31 +0800 Subject: [PATCH 19/23] [fix](be) Guard metadata count against pending filters ### What problem does this PR solve? Issue Number: None Related PR: #65498 Problem Summary: A table-level metadata COUNT split can emit several synthetic batches across scheduler turns. If a runtime filter was still pending when the shortcut started, it could arrive after an unfiltered batch had already escaped, and the active split could not recover the corresponding real file rows. Pass the scanner runtime-filter completion state into split preparation and allow metadata COUNT only after every assigned runtime filter has arrived. ### Release note Table-level metadata COUNT now falls back to reading file rows while runtime filters are pending. ### Check List (For Author) - Test: Unit Test - TableReaderTest.PrepareSplitReplacesInitialConjunctSnapshot - TableReaderTest.RefreshedConjunctDisablesTableLevelCount - TableReaderTest.PendingRuntimeFilterDisablesTableLevelCount - Remote BE build - Behavior changed: Yes (metadata COUNT waits for all scanner runtime filters before using synthetic rows) - Does this need documentation: No --- be/src/exec/scan/file_scanner_v2.cpp | 3 ++ be/src/format_v2/table_reader.cpp | 9 +++--- be/src/format_v2/table_reader.h | 5 ++++ be/test/format_v2/table_reader_test.cpp | 40 +++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 4 deletions(-) diff --git a/be/src/exec/scan/file_scanner_v2.cpp b/be/src/exec/scan/file_scanner_v2.cpp index f3231ecd7d420b..519237ea85c252 100644 --- a/be/src/exec/scan/file_scanner_v2.cpp +++ b/be/src/exec/scan/file_scanner_v2.cpp @@ -502,6 +502,9 @@ Status FileScannerV2::_prepare_table_reader_split(const TFileRangeDesc& range, .partition_values = std::move(partition_values), .conjuncts = std::move(conjuncts), .partition_prune_conjuncts = std::move(partition_prune_conjuncts), + // A metadata COUNT split may span scheduler turns. Do not enter that irreversible + // synthetic-row path while a runtime filter can still arrive between batches. + .all_runtime_filters_applied = _applied_rf_num == _total_rf_num, .cache = _kv_cache, .current_range = range, .current_split_format = current_split_format, diff --git a/be/src/format_v2/table_reader.cpp b/be/src/format_v2/table_reader.cpp index f5e478d8d6f45d..71ab072768f645 100644 --- a/be/src/format_v2/table_reader.cpp +++ b/be/src/format_v2/table_reader.cpp @@ -796,10 +796,11 @@ Status TableReader::prepare_split(const SplitReadOptions& options) { _current_task->data_file = create_file_description(options.current_range); _current_file_description = *_current_task->data_file; // A table-level row count is only equivalent to scanning the split when no row predicate is - // active. In particular, a runtime filter may arrive after init() and replace `_conjuncts` - // above. Returning synthetic rows here would bypass that fresh split snapshot completely. - if (_push_down_agg_type == TPushAggOp::type::COUNT && _conjuncts.empty() && - options.current_range.__isset.table_format_params && + // active and no predicate can arrive later. The metadata path can return several batches for + // one split; after its first synthetic batch there is no way to recover the real rows if a + // runtime filter arrives before the next scheduler turn. + if (_push_down_agg_type == TPushAggOp::type::COUNT && options.all_runtime_filters_applied && + _conjuncts.empty() && options.current_range.__isset.table_format_params && options.current_range.table_format_params.__isset.table_level_row_count) { DORIS_CHECK(options.current_range.table_format_params.table_level_row_count >= -1); _remaining_table_level_count = diff --git a/be/src/format_v2/table_reader.h b/be/src/format_v2/table_reader.h index c65ebeec5d7df9..9be1e7cee41b03 100644 --- a/be/src/format_v2/table_reader.h +++ b/be/src/format_v2/table_reader.h @@ -149,6 +149,11 @@ struct SplitReadOptions { // Independent clones used for partition pruning because evaluation prepares and opens them // against a synthetic partition block before the file reader opens its row-level conjuncts. VExprContextSPtrs partition_prune_conjuncts; + // Table-level COUNT may emit one metadata-derived batch and resume on a later scheduler turn. + // It is safe only after every runtime filter assigned to the scanner has arrived; otherwise a + // filter could arrive after synthetic rows have already been returned and those rows cannot be + // retracted. Standalone TableReader callers have no scanner runtime-filter lifecycle. + bool all_runtime_filters_applied = true; ShardedKVCache* cache; TFileRangeDesc current_range; FileFormat current_split_format = FileFormat::PARQUET; diff --git a/be/test/format_v2/table_reader_test.cpp b/be/test/format_v2/table_reader_test.cpp index 2f85fc470414f4..cfd30d85803680 100644 --- a/be/test/format_v2/table_reader_test.cpp +++ b/be/test/format_v2/table_reader_test.cpp @@ -1518,6 +1518,46 @@ TEST(TableReaderTest, RefreshedConjunctDisablesTableLevelCount) { ASSERT_TRUE(reader.close().ok()); } +TEST(TableReaderTest, PendingRuntimeFilterDisablesTableLevelCount) { + std::vector file_schema; + file_schema.push_back(make_file_column(0, "id", std::make_shared())); + + std::vector projected_columns; + projected_columns.push_back(make_table_column(0, "id", std::make_shared())); + set_name_identifiers(&projected_columns); + + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + auto fake_state = std::make_shared(); + FakeTableReader reader(file_schema, fake_state); + ASSERT_TRUE(reader.init({ + .projected_columns = projected_columns, + .conjuncts = {}, + .format = FileFormat::PARQUET, + .scan_params = nullptr, + .io_ctx = nullptr, + .runtime_state = &state, + .scanner_profile = nullptr, + .push_down_agg_type = TPushAggOp::type::COUNT, + }) + .ok()); + + SplitReadOptions split_options; + split_options.current_range.__set_path("fake-table-reader-input"); + // A pending runtime filter makes metadata COUNT ineligible before its first synthetic batch. + // This prevents the filter from arriving between scheduler reads after unfiltered rows have + // already escaped. + split_options.all_runtime_filters_applied = false; + set_table_level_row_count(&split_options, state.batch_size() + 5); + ASSERT_TRUE(reader.prepare_split(split_options).ok()); + + Block block = build_table_block(projected_columns); + bool eos = false; + ASSERT_TRUE(reader.get_block(&block, &eos).ok()); + EXPECT_EQ(fake_state->open_count, 1); + EXPECT_EQ(block.rows(), 2); + ASSERT_TRUE(reader.close().ok()); +} + TEST(TableReaderTest, AbortSplitClearsReaderAfterIgnorableNotFound) { std::vector file_schema; file_schema.push_back(make_file_column(0, "id", std::make_shared())); From 679a397502351caf666ff0b1839f21ec7708e1ed Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 23:22:24 +0800 Subject: [PATCH 20/23] [improvement](be) Avoid materializing skipped nested values ### What problem does this PR solve? Issue Number: close #65498 Related PR: #65498 Problem Summary: Nested Parquet skips rebuilt parent boundaries into scratch Columns and decoded leaf payloads that were immediately discarded. In addition, pending runtime filters disabled table-level metadata COUNT but could still allow normal COUNT or MIN/MAX aggregate pushdown to emit irreversible synthetic rows before the filter arrived. Add a levels-only nested consume protocol for scalar, list, map, and struct readers, preserving nullability and stream-alignment validation without payload materialization. Gate every aggregate pushdown path on the split runtime-filter snapshot. ### Release note Improve nested Parquet skip efficiency and preserve runtime-filter correctness for aggregate pushdown. ### Check List (For Author) - Test: Regression test / Unit Test - ParquetColumnReaderTest, ParquetColumnReaderBaseTest, ParquetColumnReaderControlTest, and targeted TableReader tests - Remote ./build.sh --be - Behavior changed: Yes (nested skip avoids decoding discarded payloads; aggregate pushdown waits for pending runtime filters) - Does this need documentation: No --- .../parquet/reader/column_reader.cpp | 28 +-- .../format_v2/parquet/reader/column_reader.h | 33 +++- .../parquet/reader/list_column_reader.cpp | 77 ++++++-- .../parquet/reader/list_column_reader.h | 5 + .../parquet/reader/map_column_reader.cpp | 119 +++++++++--- .../parquet/reader/map_column_reader.h | 5 + .../parquet/reader/scalar_column_reader.cpp | 46 ++++- .../parquet/reader/scalar_column_reader.h | 2 + .../parquet/reader/struct_column_reader.cpp | 85 +++++--- .../parquet/reader/struct_column_reader.h | 4 + be/src/format_v2/table_reader.cpp | 1 + be/src/format_v2/table_reader.h | 11 ++ .../parquet/parquet_column_reader_test.cpp | 15 +- .../parquet/parquet_reader_control_test.cpp | 182 +++++++++++++----- be/test/format_v2/table_reader_test.cpp | 56 ++++++ 15 files changed, 518 insertions(+), 151 deletions(-) diff --git a/be/src/format_v2/parquet/reader/column_reader.cpp b/be/src/format_v2/parquet/reader/column_reader.cpp index 80a95cf17ee171..352fbbd7c3d215 100644 --- a/be/src/format_v2/parquet/reader/column_reader.cpp +++ b/be/src/format_v2/parquet/reader/column_reader.cpp @@ -606,15 +606,9 @@ Status ParquetColumnReader::build_nested_column(int64_t, MutableColumnPtr&, int6 _name); } -Status ParquetColumnReader::skip_nested_column(int64_t rows) { - auto scratch_column = _type->create_column(); - int64_t values_read = 0; - RETURN_IF_ERROR(build_nested_column(rows, scratch_column, &values_read)); - if (values_read != rows) { - return Status::Corruption("Failed to skip nested parquet column {}: skipped {} of {} rows", - _name, values_read, rows); - } - return Status::OK(); +Status ParquetColumnReader::consume_nested_column(int64_t, int64_t*) { + return Status::NotSupported("Parquet nested column consume is not supported for column {}", + _name); } Status ParquetColumnReader::skip_nested_rows(int64_t rows) { @@ -623,21 +617,19 @@ Status ParquetColumnReader::skip_nested_rows(int64_t rows) { } // A nested parent row may expand to many child values. Capping the number of parent rows per - // scratch column bounds the amplification for large holes while preserving the same nested - // level decoding and validation path as a normal read. Recreate the scratch column per batch - // so its child buffers are released before decoding the next batch. + // loaded batch bounds that amplification for large holes. The consume interface advances the + // loaded definition/repetition levels recursively without constructing a discarded Column. constexpr int64_t MAX_NESTED_SKIP_BATCH_SIZE = 4096; int64_t remaining_rows = rows; while (remaining_rows > 0) { const int64_t batch_rows = std::min(remaining_rows, MAX_NESTED_SKIP_BATCH_SIZE); - auto scratch_column = _type->create_column(); - RETURN_IF_ERROR(load_nested_batch(batch_rows)); - int64_t rows_read = 0; - RETURN_IF_ERROR(build_nested_column(batch_rows, scratch_column, &rows_read)); - if (rows_read != batch_rows) { + RETURN_IF_ERROR(load_nested_levels_batch(batch_rows)); + int64_t rows_consumed = 0; + RETURN_IF_ERROR(consume_nested_column(batch_rows, &rows_consumed)); + if (rows_consumed != batch_rows) { return Status::Corruption( "Failed to skip nested parquet column {}: skipped {} of {} rows in batch", - _name, rows_read, batch_rows); + _name, rows_consumed, batch_rows); } remaining_rows -= batch_rows; } diff --git a/be/src/format_v2/parquet/reader/column_reader.h b/be/src/format_v2/parquet/reader/column_reader.h index 7072a8c7f28bad..9416c4eb22739f 100644 --- a/be/src/format_v2/parquet/reader/column_reader.h +++ b/be/src/format_v2/parquet/reader/column_reader.h @@ -80,17 +80,32 @@ class ParquetColumnReader { virtual Status load_nested_batch(int64_t rows); - // Shape-only load interface for COUNT(col). Implementations only guarantee that - // nested_definition_levels(), nested_repetition_levels(), and nested_levels_written() are available; - // value_indices and values_column are not guaranteed, so callers must not call build_nested_column() afterwards. - // This protocol lets the V2 aggregation path avoid Doris-side value materialization even when - // the representative ARRAY/STRUCT leaf is STRING/BINARY; normal scans still use load_nested_batch(). + // Shape-only load interface for COUNT(col) and skip. Implementations guarantee only that + // nested_definition_levels(), nested_repetition_levels(), and nested_levels_written() are + // available; value indices and payload columns may be absent. Callers may inspect the levels or + // call consume_nested_column(), but must not call build_nested_column() afterwards. For example, + // skipping ARRAY uses this method to find ARRAY boundaries without decoding strings. + // Normal scans that need output values use load_nested_batch() instead. virtual Status load_nested_levels_batch(int64_t rows); virtual Status build_nested_column(int64_t length_upper_bound, MutableColumnPtr& column, int64_t* values_read); - virtual Status skip_nested_column(int64_t rows); + // Consume logical values from a batch previously loaded by load_nested_batch() or + // load_nested_levels_batch() without appending them to an output Column. Implementations must + // advance exactly the same nested level cursors and perform the same shape/null/alignment + // validation as build_nested_column(). The levels-only form is preferred for skip paths because + // it also avoids decoding leaf payloads that will be discarded. + // + // `length_upper_bound` is expressed at this reader's logical level, not in physical leaf + // values. For example, consuming two rows from ARRAY [[1, 2], []] consumes two parent ARRAY + // rows but only two element values. A MAP implementation must also consume key/value streams + // in lockstep, while a nullable STRUCT consumes no child value for a null parent. + // + // Callers must not use the ordinary skip() after either load call: the leaf stream has already + // advanced into an in-memory nested batch, and doing so would advance it twice. + // `values_consumed` may be smaller than the requested bound only when the loaded batch ends. + virtual Status consume_nested_column(int64_t length_upper_bound, int64_t* values_consumed); virtual const std::vector& nested_definition_levels() const; virtual const std::vector& nested_repetition_levels() const; @@ -109,9 +124,9 @@ class ParquetColumnReader { ParquetColumnReader(const ParquetColumnSchema& schema, const DataTypePtr type, ParquetColumnReaderProfile profile = {}); ParquetColumnReader() = default; - // Complex readers cannot advance their child streams without rebuilding parent boundaries - // from definition/repetition levels. Materialize that discarded shape in bounded batches so - // a large selection gap does not allocate a scratch column for the entire gap at once. + // Load shape levels and consume skipped parent rows in bounded batches. The bound limits level + // memory when a parent expands to many children; the levels-only load plus + // consume_nested_column() avoids both payload decoding and output Columns. Status skip_nested_rows(int64_t rows); void update_reader_read_rows(int64_t rows) const; void update_reader_skip_rows(int64_t rows) const; diff --git a/be/src/format_v2/parquet/reader/list_column_reader.cpp b/be/src/format_v2/parquet/reader/list_column_reader.cpp index 3caac5b2ac6d6a..c042fc99b512aa 100644 --- a/be/src/format_v2/parquet/reader/list_column_reader.cpp +++ b/be/src/format_v2/parquet/reader/list_column_reader.cpp @@ -56,27 +56,53 @@ Status ListColumnReader::load_nested_batch(int64_t rows) { return _element_reader->load_nested_batch(rows); } +Status ListColumnReader::load_nested_levels_batch(int64_t rows) { + DORIS_CHECK(_element_reader != nullptr); + reset_nested_build_level_cursor(); + return _element_reader->load_nested_levels_batch(rows); +} + Status ListColumnReader::build_nested_column(int64_t length_upper_bound, MutableColumnPtr& column, int64_t* values_read) { - if (column.get() == nullptr || values_read == nullptr) { + if (column.get() == nullptr) { return Status::InvalidArgument("Invalid parquet list build result pointer for column {}", _name); } + return _consume_or_build_nested_column(length_upper_bound, &column, values_read); +} + +Status ListColumnReader::consume_nested_column(int64_t length_upper_bound, + int64_t* values_consumed) { + return _consume_or_build_nested_column(length_upper_bound, nullptr, values_consumed); +} + +Status ListColumnReader::_consume_or_build_nested_column(int64_t length_upper_bound, + MutableColumnPtr* column, + int64_t* values_processed) { + if (values_processed == nullptr) { + return Status::InvalidArgument("Invalid parquet list process result pointer for column {}", + _name); + } DORIS_CHECK(_element_reader != nullptr); - auto* array_column = array_column_from_output(column); - DORIS_CHECK(array_column != nullptr); - auto* parent_null_map = null_map_from_nullable_output(column); - auto nested_column = array_column->get_data_ptr()->assert_mutable(); - const auto& element_output_type = - assert_cast(*remove_nullable(_type)).get_nested_type(); - remove_nullable_wrapper_if_not_expected(element_output_type, &nested_column); + ColumnArray* array_column = nullptr; + NullMap* parent_null_map = nullptr; + MutableColumnPtr nested_column; + if (column != nullptr) { + array_column = array_column_from_output(*column); + DORIS_CHECK(array_column != nullptr); + parent_null_map = null_map_from_nullable_output(*column); + nested_column = array_column->get_data_ptr()->assert_mutable(); + const auto& element_output_type = + assert_cast(*remove_nullable(_type)).get_nested_type(); + remove_nullable_wrapper_if_not_expected(element_output_type, &nested_column); + } const auto& def_levels = _element_reader->nested_definition_levels(); const auto& rep_levels = _element_reader->nested_repetition_levels(); const int64_t levels_written = _element_reader->nested_levels_written(); std::vector entry_counts; NullMap parent_nulls; - *values_read = 0; + *values_processed = 0; int64_t level_idx = nested_build_level_cursor(); const int16_t min_parent_definition_level = static_cast(_definition_level - 1 - (_type->is_nullable() ? 1 : 0)); @@ -84,7 +110,7 @@ Status ListColumnReader::build_nested_column(int64_t length_upper_bound, Mutable const int16_t def_level = def_levels[level_idx]; const int16_t rep_level = rep_levels[level_idx]; const bool starts_parent = rep_level < _repetition_level; - if (starts_parent && *values_read >= length_upper_bound) { + if (starts_parent && *values_processed >= length_upper_bound) { break; } ++level_idx; @@ -104,13 +130,13 @@ Status ListColumnReader::build_nested_column(int64_t length_upper_bound, Mutable } const bool parent_is_null = def_level < _definition_level - 1; - if (parent_is_null && parent_null_map == nullptr) { + if (parent_is_null && !_type->is_nullable()) { return Status::Corruption("Parquet LIST column {} contains null for non-nullable LIST", _name); } parent_nulls.push_back(parent_is_null); entry_counts.push_back(def_level >= _definition_level ? 1 : 0); - ++*values_read; + ++*values_processed; } set_nested_build_level_cursor(level_idx); @@ -120,8 +146,13 @@ Status ListColumnReader::build_nested_column(int64_t length_upper_bound, Mutable for (const auto entry_count : entry_counts) { total_entries += entry_count; } - RETURN_IF_ERROR(_element_reader->build_nested_column(static_cast(total_entries), - nested_column, &child_value_count)); + if (column != nullptr) { + RETURN_IF_ERROR(_element_reader->build_nested_column( + static_cast(total_entries), nested_column, &child_value_count)); + } else { + RETURN_IF_ERROR(_element_reader->consume_nested_column( + static_cast(total_entries), &child_value_count)); + } } else { uint64_t pending_entries = 0; auto flush_pending_entries = [&]() -> Status { @@ -129,8 +160,14 @@ Status ListColumnReader::build_nested_column(int64_t length_upper_bound, Mutable return Status::OK(); } int64_t span_child_value_count = 0; - RETURN_IF_ERROR(_element_reader->build_nested_column( - static_cast(pending_entries), nested_column, &span_child_value_count)); + if (column != nullptr) { + RETURN_IF_ERROR(_element_reader->build_nested_column( + static_cast(pending_entries), nested_column, + &span_child_value_count)); + } else { + RETURN_IF_ERROR(_element_reader->consume_nested_column( + static_cast(pending_entries), &span_child_value_count)); + } if (span_child_value_count != static_cast(pending_entries)) { return Status::Corruption( "Parquet LIST column {} built {} child values, expected {}", _name, @@ -156,9 +193,11 @@ Status ListColumnReader::build_nested_column(int64_t length_upper_bound, Mutable return Status::Corruption("Parquet LIST column {} built {} child values, expected {}", _name, child_value_count, total_entries); } - array_column->get_data_ptr() = std::move(nested_column); - append_offsets(array_column->get_offsets(), entry_counts); - append_parent_nulls(parent_null_map, parent_nulls); + if (column != nullptr) { + array_column->get_data_ptr() = std::move(nested_column); + append_offsets(array_column->get_offsets(), entry_counts); + append_parent_nulls(parent_null_map, parent_nulls); + } return Status::OK(); } diff --git a/be/src/format_v2/parquet/reader/list_column_reader.h b/be/src/format_v2/parquet/reader/list_column_reader.h index 5a60eecacb0e3e..d64be546e394d7 100644 --- a/be/src/format_v2/parquet/reader/list_column_reader.h +++ b/be/src/format_v2/parquet/reader/list_column_reader.h @@ -36,8 +36,10 @@ class ListColumnReader final : public ParquetColumnReader { Status read(int64_t rows, MutableColumnPtr& column, int64_t* rows_read) override; Status skip(int64_t rows) override; Status load_nested_batch(int64_t rows) override; + Status load_nested_levels_batch(int64_t rows) override; Status build_nested_column(int64_t length_upper_bound, MutableColumnPtr& column, int64_t* values_read) override; + Status consume_nested_column(int64_t length_upper_bound, int64_t* values_consumed) override; const std::vector& nested_definition_levels() const override; const std::vector& nested_repetition_levels() const override; int64_t nested_levels_written() const override; @@ -45,6 +47,9 @@ class ListColumnReader final : public ParquetColumnReader { void advance_nested_build_level_cursor_past_parent(int16_t parent_repetition_level) override; private: + Status _consume_or_build_nested_column(int64_t length_upper_bound, MutableColumnPtr* column, + int64_t* values_processed); + std::unique_ptr _element_reader; // element reader (recursive; may be Scalar/Struct/List/Map) }; diff --git a/be/src/format_v2/parquet/reader/map_column_reader.cpp b/be/src/format_v2/parquet/reader/map_column_reader.cpp index c5a7e0e47f9e13..8217d0c013abc0 100644 --- a/be/src/format_v2/parquet/reader/map_column_reader.cpp +++ b/be/src/format_v2/parquet/reader/map_column_reader.cpp @@ -60,22 +60,51 @@ Status MapColumnReader::load_nested_batch(int64_t rows) { return _value_reader->load_nested_batch(rows); } +Status MapColumnReader::load_nested_levels_batch(int64_t rows) { + DORIS_CHECK(_key_reader != nullptr); + DORIS_CHECK(_value_reader != nullptr); + reset_nested_build_level_cursor(); + RETURN_IF_ERROR(_key_reader->load_nested_levels_batch(rows)); + return _value_reader->load_nested_levels_batch(rows); +} + Status MapColumnReader::build_nested_column(int64_t length_upper_bound, MutableColumnPtr& column, int64_t* values_read) { - if (column.get() == nullptr || values_read == nullptr) { + if (column.get() == nullptr) { return Status::InvalidArgument("Invalid parquet map build result pointer for column {}", _name); } + return _consume_or_build_nested_column(length_upper_bound, &column, values_read); +} + +Status MapColumnReader::consume_nested_column(int64_t length_upper_bound, + int64_t* values_consumed) { + return _consume_or_build_nested_column(length_upper_bound, nullptr, values_consumed); +} + +Status MapColumnReader::_consume_or_build_nested_column(int64_t length_upper_bound, + MutableColumnPtr* column, + int64_t* values_processed) { + if (values_processed == nullptr) { + return Status::InvalidArgument("Invalid parquet map process result pointer for column {}", + _name); + } DORIS_CHECK(_key_reader != nullptr); DORIS_CHECK(_value_reader != nullptr); - auto* map_column = map_column_from_output(column); - DORIS_CHECK(map_column != nullptr); - auto* parent_null_map = null_map_from_nullable_output(column); - auto key_column = map_column->get_keys_ptr()->assert_mutable(); - auto value_column = map_column->get_values_ptr()->assert_mutable(); - const auto& map_output_type = assert_cast(*remove_nullable(_type)); - remove_nullable_wrapper_if_not_expected(map_output_type.get_key_type(), &key_column); - remove_nullable_wrapper_if_not_expected(map_output_type.get_value_type(), &value_column); + ColumnMap* map_column = nullptr; + NullMap* parent_null_map = nullptr; + MutableColumnPtr key_column; + MutableColumnPtr value_column; + if (column != nullptr) { + map_column = map_column_from_output(*column); + DORIS_CHECK(map_column != nullptr); + parent_null_map = null_map_from_nullable_output(*column); + key_column = map_column->get_keys_ptr()->assert_mutable(); + value_column = map_column->get_values_ptr()->assert_mutable(); + const auto& map_output_type = assert_cast(*remove_nullable(_type)); + remove_nullable_wrapper_if_not_expected(map_output_type.get_key_type(), &key_column); + remove_nullable_wrapper_if_not_expected(map_output_type.get_value_type(), &value_column); + } const auto& def_levels = _key_reader->nested_definition_levels(); const auto& rep_levels = _key_reader->nested_repetition_levels(); @@ -84,7 +113,7 @@ Status MapColumnReader::build_nested_column(int64_t length_upper_bound, MutableC std::vector entry_counts; std::vector map_level_indices; NullMap parent_nulls; - *values_read = 0; + *values_processed = 0; int64_t level_idx = nested_build_level_cursor(); const int16_t min_parent_definition_level = static_cast(_definition_level - 1 - (_type->is_nullable() ? 1 : 0)); @@ -92,7 +121,7 @@ Status MapColumnReader::build_nested_column(int64_t length_upper_bound, MutableC const int16_t def_level = def_levels[level_idx]; const int16_t rep_level = rep_levels[level_idx]; const bool starts_parent = rep_level < _repetition_level; - if (starts_parent && *values_read >= length_upper_bound) { + if (starts_parent && *values_processed >= length_upper_bound) { break; } const int64_t current_level_idx = level_idx; @@ -114,13 +143,13 @@ Status MapColumnReader::build_nested_column(int64_t length_upper_bound, MutableC } const bool parent_is_null = def_level < _definition_level - 1; - if (parent_is_null && parent_null_map == nullptr) { + if (parent_is_null && !_type->is_nullable()) { return Status::Corruption("Parquet MAP column {} contains null for non-nullable MAP", _name); } parent_nulls.push_back(parent_is_null); entry_counts.push_back(def_level >= _definition_level ? 1 : 0); - ++*values_read; + ++*values_processed; } set_nested_build_level_cursor(level_idx); @@ -128,18 +157,36 @@ Status MapColumnReader::build_nested_column(int64_t length_upper_bound, MutableC for (const auto entry_count : entry_counts) { total_entries += entry_count; } - const size_t key_start = key_column->size(); int64_t key_value_count = 0; - RETURN_IF_ERROR(_key_reader->build_nested_column(static_cast(total_entries), - key_column, &key_value_count)); + size_t key_start = 0; + if (column != nullptr) { + key_start = key_column->size(); + RETURN_IF_ERROR(_key_reader->build_nested_column(static_cast(total_entries), + key_column, &key_value_count)); + } else if (auto* scalar_key_reader = dynamic_cast(_key_reader.get())) { + // MAP keys are required even if a projected Doris key type is nullable. Validate each + // actual entry directly from the key level stream while advancing past empty/null maps. + for (const int64_t key_level_idx : map_level_indices) { + if (def_levels[key_level_idx] >= _definition_level) { + RETURN_IF_ERROR(scalar_key_reader->validate_nested_value(key_level_idx, true)); + ++key_value_count; + } + } + scalar_key_reader->set_nested_build_level_cursor(level_idx); + } else { + RETURN_IF_ERROR(_key_reader->consume_nested_column(static_cast(total_entries), + &key_value_count)); + } if (key_value_count != static_cast(total_entries)) { return Status::Corruption("Parquet MAP column {} built {} keys, expected {}", _name, key_value_count, total_entries); } - if (const auto* nullable_key_column = check_and_get_column(*key_column); - nullable_key_column != nullptr && - nullable_key_column->has_null(key_start, nullable_key_column->size())) { - return Status::Corruption("Parquet MAP column {} contains null key", _name); + if (column != nullptr) { + if (const auto* nullable_key_column = check_and_get_column(*key_column); + nullable_key_column != nullptr && + nullable_key_column->has_null(key_start, nullable_key_column->size())) { + return Status::Corruption("Parquet MAP column {} contains null key", _name); + } } int64_t value_count = 0; if (auto* scalar_value_reader = dynamic_cast(_value_reader.get())) { @@ -170,28 +217,40 @@ Status MapColumnReader::build_nested_column(int64_t length_upper_bound, MutableC _name); } if (def_levels[key_level_idx] >= _definition_level) { - RETURN_IF_ERROR( - scalar_value_reader->append_nested_value(value_level_idx, value_column)); + if (column != nullptr) { + RETURN_IF_ERROR(scalar_value_reader->append_nested_value(value_level_idx, + value_column)); + } else { + RETURN_IF_ERROR( + scalar_value_reader->validate_nested_value(value_level_idx, false)); + } ++value_count; } ++value_level_idx; } scalar_value_reader->set_nested_build_level_cursor(value_level_idx); } else { - // Complex MAP values own their nested shape below the entry slot, so they can recursively - // materialize exactly one child value for each MAP entry. - RETURN_IF_ERROR(_value_reader->build_nested_column(static_cast(total_entries), - value_column, &value_count)); + // Complex MAP values own their nested shape below the entry slot, so they recursively + // process exactly one child value for each MAP entry. + if (column != nullptr) { + RETURN_IF_ERROR(_value_reader->build_nested_column(static_cast(total_entries), + value_column, &value_count)); + } else { + RETURN_IF_ERROR(_value_reader->consume_nested_column( + static_cast(total_entries), &value_count)); + } } if (value_count != static_cast(total_entries)) { return Status::Corruption("Parquet MAP column {} built {} values, expected {}", _name, value_count, total_entries); } - map_column->get_keys_ptr() = std::move(key_column); - map_column->get_values_ptr() = std::move(value_column); - append_offsets(map_column->get_offsets(), entry_counts); - append_parent_nulls(parent_null_map, parent_nulls); + if (column != nullptr) { + map_column->get_keys_ptr() = std::move(key_column); + map_column->get_values_ptr() = std::move(value_column); + append_offsets(map_column->get_offsets(), entry_counts); + append_parent_nulls(parent_null_map, parent_nulls); + } return Status::OK(); } diff --git a/be/src/format_v2/parquet/reader/map_column_reader.h b/be/src/format_v2/parquet/reader/map_column_reader.h index 3e26a7a480a2a5..1a8ca9c70d8c5b 100644 --- a/be/src/format_v2/parquet/reader/map_column_reader.h +++ b/be/src/format_v2/parquet/reader/map_column_reader.h @@ -39,8 +39,10 @@ class MapColumnReader final : public ParquetColumnReader { Status read(int64_t rows, MutableColumnPtr& column, int64_t* rows_read) override; Status skip(int64_t rows) override; Status load_nested_batch(int64_t rows) override; + Status load_nested_levels_batch(int64_t rows) override; Status build_nested_column(int64_t length_upper_bound, MutableColumnPtr& column, int64_t* values_read) override; + Status consume_nested_column(int64_t length_upper_bound, int64_t* values_consumed) override; const std::vector& nested_definition_levels() const override; const std::vector& nested_repetition_levels() const override; int64_t nested_levels_written() const override; @@ -48,6 +50,9 @@ class MapColumnReader final : public ParquetColumnReader { void advance_nested_build_level_cursor_past_parent(int16_t parent_repetition_level) override; private: + Status _consume_or_build_nested_column(int64_t length_upper_bound, MutableColumnPtr* column, + int64_t* values_processed); + std::unique_ptr _key_reader; // key column reader (always read fully) std::unique_ptr _value_reader; // value column reader (can be pruned by projection) diff --git a/be/src/format_v2/parquet/reader/scalar_column_reader.cpp b/be/src/format_v2/parquet/reader/scalar_column_reader.cpp index 6e3b1c7f4d5d20..1e21c71b211acd 100644 --- a/be/src/format_v2/parquet/reader/scalar_column_reader.cpp +++ b/be/src/format_v2/parquet/reader/scalar_column_reader.cpp @@ -449,7 +449,11 @@ Status ScalarColumnReader::build_nested_column(int64_t length_upper_bound, Mutab } DORIS_CHECK(_nested_batch != nullptr); ParquetNestedScalarValueCursor value_cursor(_nested_batch.get()); - const int16_t materialized_slot_definition_level = _nested_batch->value_slot_definition_level; + // The levels-only loader intentionally does not populate value-slot metadata or payload + // buffers. Derive the logical slot threshold from the schema, exactly as load_nested_batch() + // does, so this consumer works for both loaded batch forms. + const int16_t materialized_slot_definition_level = + static_cast(_definition_level - (_type->is_nullable() ? 1 : 0)); *values_read = 0; int64_t level_idx = nested_build_level_cursor(); while (level_idx < _nested_batch->levels_written && *values_read < length_upper_bound) { @@ -476,6 +480,31 @@ Status ScalarColumnReader::build_nested_column(int64_t length_upper_bound, Mutab return Status::OK(); } +Status ScalarColumnReader::consume_nested_column(int64_t length_upper_bound, + int64_t* values_consumed) { + if (values_consumed == nullptr) { + return Status::InvalidArgument("Invalid parquet nested scalar consume result for column {}", + _name); + } + DORIS_CHECK(_nested_batch != nullptr); + const int16_t materialized_slot_definition_level = _nested_batch->value_slot_definition_level; + *values_consumed = 0; + int64_t level_idx = nested_build_level_cursor(); + while (level_idx < _nested_batch->levels_written && *values_consumed < length_upper_bound) { + const int64_t current_level_idx = level_idx; + const int16_t def_level = _nested_batch->def_levels[current_level_idx]; + const int16_t rep_level = _nested_batch->rep_levels[current_level_idx]; + ++level_idx; + if (def_level < materialized_slot_definition_level || rep_level > _repetition_level) { + continue; + } + RETURN_IF_ERROR(validate_nested_value(current_level_idx, false)); + ++*values_consumed; + } + set_nested_build_level_cursor(level_idx); + return Status::OK(); +} + Status ScalarColumnReader::append_nested_value(int64_t level_idx, MutableColumnPtr& column) const { if (column.get() == nullptr) { return Status::InvalidArgument("Invalid parquet nested scalar append result for column {}", @@ -497,6 +526,21 @@ Status ScalarColumnReader::append_nested_value(int64_t level_idx, MutableColumnP return Status::OK(); } +Status ScalarColumnReader::validate_nested_value(int64_t level_idx, bool require_non_null) const { + DORIS_CHECK(_nested_batch != nullptr); + DORIS_CHECK(level_idx >= 0); + DORIS_CHECK(level_idx < _nested_batch->levels_written); + const int16_t def_level = _nested_batch->def_levels[level_idx]; + if (def_level == _definition_level) { + return Status::OK(); + } + if (require_non_null || !_type->is_nullable()) { + return Status::Corruption("Parquet scalar column {} contains null for non-nullable field", + _name); + } + return Status::OK(); +} + const std::vector& ScalarColumnReader::nested_definition_levels() const { DORIS_CHECK(_nested_batch != nullptr); return _nested_batch->def_levels; diff --git a/be/src/format_v2/parquet/reader/scalar_column_reader.h b/be/src/format_v2/parquet/reader/scalar_column_reader.h index 99fec69fa3ab8a..5342baa803eca0 100644 --- a/be/src/format_v2/parquet/reader/scalar_column_reader.h +++ b/be/src/format_v2/parquet/reader/scalar_column_reader.h @@ -65,6 +65,7 @@ class ScalarColumnReader final : public ParquetColumnReader { Status load_nested_levels_batch(int64_t rows) override; Status build_nested_column(int64_t length_upper_bound, MutableColumnPtr& column, int64_t* values_read) override; + Status consume_nested_column(int64_t length_upper_bound, int64_t* values_consumed) override; const std::vector& nested_definition_levels() const override; const std::vector& nested_repetition_levels() const override; int64_t nested_levels_written() const override; @@ -72,6 +73,7 @@ class ScalarColumnReader final : public ParquetColumnReader { private: Status append_nested_value(int64_t level_idx, MutableColumnPtr& column) const; + Status validate_nested_value(int64_t level_idx, bool require_non_null) const; Status read_range_with_dictionary_filter(int64_t rows, const IColumn::Filter& dictionary_filter, MutableColumnPtr& column, IColumn::Filter* row_filter, int64_t* rows_read, bool* used_filter); diff --git a/be/src/format_v2/parquet/reader/struct_column_reader.cpp b/be/src/format_v2/parquet/reader/struct_column_reader.cpp index 8d69f75f8d3431..5abe7abe75e9a2 100644 --- a/be/src/format_v2/parquet/reader/struct_column_reader.cpp +++ b/be/src/format_v2/parquet/reader/struct_column_reader.cpp @@ -102,20 +102,50 @@ Status StructColumnReader::load_nested_batch(int64_t rows) { return Status::OK(); } +Status StructColumnReader::load_nested_levels_batch(int64_t rows) { + reset_nested_build_level_cursor(); + for (auto& child_reader : _children) { + DORIS_CHECK(child_reader != nullptr); + RETURN_IF_ERROR(child_reader->load_nested_levels_batch(rows)); + } + return Status::OK(); +} + Status StructColumnReader::build_nested_column(int64_t length_upper_bound, MutableColumnPtr& column, int64_t* values_read) { - if (column.get() == nullptr || values_read == nullptr) { + if (column.get() == nullptr) { return Status::InvalidArgument("Invalid parquet struct build result pointer for column {}", _name); } + return _consume_or_build_nested_column(length_upper_bound, &column, values_read); +} + +Status StructColumnReader::consume_nested_column(int64_t length_upper_bound, + int64_t* values_consumed) { + return _consume_or_build_nested_column(length_upper_bound, nullptr, values_consumed); +} + +Status StructColumnReader::_consume_or_build_nested_column(int64_t length_upper_bound, + MutableColumnPtr* column, + int64_t* values_processed) { + if (values_processed == nullptr) { + return Status::InvalidArgument( + "Invalid parquet struct process result pointer for column {}", _name); + } if (_children.empty()) { - column->resize(column->size() + static_cast(length_upper_bound)); - *values_read = length_upper_bound; + if (column != nullptr) { + (*column)->resize((*column)->size() + static_cast(length_upper_bound)); + } + *values_processed = length_upper_bound; return Status::OK(); } - auto* struct_column = struct_column_from_output(column); - DORIS_CHECK(struct_column != nullptr); - auto* parent_null_map = null_map_from_nullable_output(column); + ColumnStruct* struct_column = nullptr; + NullMap* parent_null_map = nullptr; + if (column != nullptr) { + struct_column = struct_column_from_output(*column); + DORIS_CHECK(struct_column != nullptr); + parent_null_map = null_map_from_nullable_output(*column); + } auto* shape_reader = shape_source_reader(); DORIS_CHECK(shape_reader != nullptr); const auto& def_levels = shape_reader->nested_definition_levels(); @@ -124,7 +154,7 @@ Status StructColumnReader::build_nested_column(int64_t length_upper_bound, Mutab NullMap parent_nulls; std::vector parent_level_indices; - *values_read = 0; + *values_processed = 0; int64_t level_idx = nested_build_level_cursor(); while (level_idx < levels_written) { const int64_t current_level_idx = level_idx; @@ -132,7 +162,7 @@ Status StructColumnReader::build_nested_column(int64_t length_upper_bound, Mutab const int16_t rep_level = rep_levels[level_idx]; const bool starts_parent = !shape_reader->is_or_has_repeated_child() || rep_level <= _repetition_level; - if (starts_parent && *values_read >= length_upper_bound) { + if (starts_parent && *values_processed >= length_upper_bound) { break; } ++level_idx; @@ -143,24 +173,26 @@ Status StructColumnReader::build_nested_column(int64_t length_upper_bound, Mutab continue; } const bool parent_is_null = def_level < _nullable_definition_level; - if (parent_is_null && parent_null_map == nullptr) { + if (parent_is_null && !_type->is_nullable()) { return Status::Corruption( "Parquet STRUCT column {} contains null for non-nullable struct", _name); } parent_nulls.push_back(parent_is_null); parent_level_indices.push_back(current_level_idx); - ++*values_read; + ++*values_processed; } set_nested_build_level_cursor(level_idx); std::vector child_columns; - child_columns.reserve(struct_column->get_columns().size()); - for (size_t child_idx = 0; child_idx < struct_column->get_columns().size(); ++child_idx) { - child_columns.push_back(struct_column->get_column_ptr(child_idx)->assert_mutable()); + if (column != nullptr) { + child_columns.reserve(struct_column->get_columns().size()); + for (size_t child_idx = 0; child_idx < struct_column->get_columns().size(); ++child_idx) { + child_columns.push_back(struct_column->get_column_ptr(child_idx)->assert_mutable()); + } } for (size_t child_idx = 0; child_idx < _children.size(); ++child_idx) { const int output_idx = _child_output_indices[child_idx]; - if (output_idx < 0) { + if (column != nullptr && output_idx < 0) { continue; } // STRUCT owns row alignment. Child readers consume only present parent rows from their @@ -174,8 +206,13 @@ Status StructColumnReader::build_nested_column(int64_t length_upper_bound, Mutab return Status::OK(); } int64_t child_rows = 0; - RETURN_IF_ERROR(_children[child_idx]->build_nested_column( - pending_present_rows, child_columns[output_idx], &child_rows)); + if (column != nullptr) { + RETURN_IF_ERROR(_children[child_idx]->build_nested_column( + pending_present_rows, child_columns[output_idx], &child_rows)); + } else { + RETURN_IF_ERROR(_children[child_idx]->consume_nested_column(pending_present_rows, + &child_rows)); + } if (child_rows != pending_present_rows) { return Status::Corruption( "Parquet STRUCT child {} built {} rows, expected {} for column {}", @@ -192,22 +229,26 @@ Status StructColumnReader::build_nested_column(int64_t length_upper_bound, Mutab continue; } RETURN_IF_ERROR(flush_present_rows()); - child_columns[output_idx]->insert_default(); + if (column != nullptr) { + child_columns[output_idx]->insert_default(); + } RETURN_IF_ERROR(advance_child_past_null_parent(_children[child_idx].get(), parent_level_indices[parent_idx])); ++total_child_rows; } RETURN_IF_ERROR(flush_present_rows()); - if (total_child_rows != *values_read) { + if (total_child_rows != *values_processed) { return Status::Corruption( "Parquet STRUCT child {} built {} rows, expected {} for column {}", - _children[child_idx]->name(), total_child_rows, *values_read, _name); + _children[child_idx]->name(), total_child_rows, *values_processed, _name); } } - for (size_t child_idx = 0; child_idx < child_columns.size(); ++child_idx) { - struct_column->get_column_ptr(child_idx) = std::move(child_columns[child_idx]); + if (column != nullptr) { + for (size_t child_idx = 0; child_idx < child_columns.size(); ++child_idx) { + struct_column->get_column_ptr(child_idx) = std::move(child_columns[child_idx]); + } + append_parent_nulls(parent_null_map, parent_nulls); } - append_parent_nulls(parent_null_map, parent_nulls); return Status::OK(); } diff --git a/be/src/format_v2/parquet/reader/struct_column_reader.h b/be/src/format_v2/parquet/reader/struct_column_reader.h index 3e88b75cede3d9..3c2d6904cb36f4 100644 --- a/be/src/format_v2/parquet/reader/struct_column_reader.h +++ b/be/src/format_v2/parquet/reader/struct_column_reader.h @@ -41,8 +41,10 @@ class StructColumnReader final : public ParquetColumnReader { Status read(int64_t rows, MutableColumnPtr& column, int64_t* rows_read) override; Status skip(int64_t rows) override; Status load_nested_batch(int64_t rows) override; + Status load_nested_levels_batch(int64_t rows) override; Status build_nested_column(int64_t length_upper_bound, MutableColumnPtr& column, int64_t* values_read) override; + Status consume_nested_column(int64_t length_upper_bound, int64_t* values_consumed) override; const std::vector& nested_definition_levels() const override; const std::vector& nested_repetition_levels() const override; int64_t nested_levels_written() const override; @@ -50,6 +52,8 @@ class StructColumnReader final : public ParquetColumnReader { void advance_nested_build_level_cursor_past_parent(int16_t parent_repetition_level) override; private: + Status _consume_or_build_nested_column(int64_t length_upper_bound, MutableColumnPtr* column, + int64_t* values_processed); ParquetColumnReader* shape_source_reader() const; Status advance_child_past_null_parent(ParquetColumnReader* child_reader, int64_t parent_level_idx) const; diff --git a/be/src/format_v2/table_reader.cpp b/be/src/format_v2/table_reader.cpp index 71ab072768f645..aca3278ee5500a 100644 --- a/be/src/format_v2/table_reader.cpp +++ b/be/src/format_v2/table_reader.cpp @@ -765,6 +765,7 @@ std::unique_ptr create_file_description(const TFileRangeDes Status TableReader::prepare_split(const SplitReadOptions& options) { SCOPED_TIMER(_profile.prepare_split_timer); _current_split_pruned = false; + _all_runtime_filters_applied_for_split = options.all_runtime_filters_applied; if (options.conjuncts.has_value()) { _conjuncts = *options.conjuncts; } diff --git a/be/src/format_v2/table_reader.h b/be/src/format_v2/table_reader.h index 9be1e7cee41b03..bcf90653cc3326 100644 --- a/be/src/format_v2/table_reader.h +++ b/be/src/format_v2/table_reader.h @@ -919,6 +919,14 @@ class TableReader { if (agg_type != TPushAggOp::type::COUNT && agg_type != TPushAggOp::type::MINMAX) { return false; } + // Aggregate pushdown returns reduced synthetic rows and may close the physical reader + // before the next scheduler turn. If a runtime filter is still pending, those rows could + // escape before the filter arrives and cannot later be reconstructed from real file rows. + // This is the same irreversibility constraint as table-level metadata COUNT, and applies + // to COUNT and MIN/MAX for Parquet/ORC as well as COUNT for text readers. + if (!_all_runtime_filters_applied_for_split) { + return false; + } // Only support aggregate pushdown when there is no delete or filter, so // the reduced rows consumed by the upper aggregate remain semantically equivalent to a // normal scan. @@ -1559,6 +1567,9 @@ class TableReader { int64_t _condition_cache_hit_count = 0; bool _current_reader_reached_eof = false; int64_t _remaining_table_level_count = -1; + // Snapshot supplied by FileScannerV2 for the active split. It gates every shortcut that emits + // irreversible aggregate rows, not only the table-level row-count shortcut in prepare_split(). + bool _all_runtime_filters_applied_for_split = true; std::optional _global_rowid_context; bool _aggregate_pushdown_tried = false; bool _current_split_pruned = false; diff --git a/be/test/format_v2/parquet/parquet_column_reader_test.cpp b/be/test/format_v2/parquet/parquet_column_reader_test.cpp index 91382203c5cea9..f9bfe137cf74eb 100644 --- a/be/test/format_v2/parquet/parquet_column_reader_test.cpp +++ b/be/test/format_v2/parquet/parquet_column_reader_test.cpp @@ -121,13 +121,8 @@ class NestedSkipReader final : public ParquetColumnReader { Status read(int64_t, MutableColumnPtr&, int64_t*) override { return Status::OK(); } - Status build_nested_column(int64_t length_upper_bound, MutableColumnPtr& column, - int64_t* values_read) override { - auto& values = assert_cast(*column); - for (int64_t row = 0; row < length_upper_bound; ++row) { - values.insert_value(static_cast(row)); - } - *values_read = length_upper_bound; + Status consume_nested_column(int64_t length_upper_bound, int64_t* values_consumed) override { + *values_consumed = length_upper_bound; return Status::OK(); } }; @@ -1929,7 +1924,7 @@ TEST(ParquetColumnReaderBaseTest, SelectionVectorRangesAndValidation) { EXPECT_FALSE(identity.verify(1, -1).ok()); } -TEST(ParquetColumnReaderBaseTest, DefaultSelectUsesSkipReadRangesAndSkipNestedUsesBuild) { +TEST(ParquetColumnReaderBaseTest, DefaultSelectUsesSkipReadRangesAndNestedConsumeIsExplicit) { DefaultSelectReader reader; std::array selected = {1, 3, 4}; SelectionVector selection(selected.data(), selected.size()); @@ -1953,10 +1948,12 @@ TEST(ParquetColumnReaderBaseTest, DefaultSelectUsesSkipReadRangesAndSkipNestedUs EXPECT_FALSE(unsupported_reader.load_nested_batch(1).ok()); int64_t values_read = 0; EXPECT_FALSE(unsupported_reader.build_nested_column(1, mutable_column, &values_read).ok()); + EXPECT_FALSE(unsupported_reader.consume_nested_column(1, &values_read).ok()); NestedSkipReader nested_reader; - auto nested_status = nested_reader.skip_nested_column(3); + auto nested_status = nested_reader.consume_nested_column(3, &values_read); ASSERT_TRUE(nested_status.ok()) << nested_status; + EXPECT_EQ(values_read, 3); } TEST_F(ParquetColumnReaderTest, ScalarReadCoversRequiredNullableAllNullAndMultipleBatches) { diff --git a/be/test/format_v2/parquet/parquet_reader_control_test.cpp b/be/test/format_v2/parquet/parquet_reader_control_test.cpp index ca19f096924e90..a21974bced294d 100644 --- a/be/test/format_v2/parquet/parquet_reader_control_test.cpp +++ b/be/test/format_v2/parquet/parquet_reader_control_test.cpp @@ -17,6 +17,7 @@ #include +#include #include #include #include @@ -218,37 +219,6 @@ class CursorColumnReader final : public ParquetColumnReader { std::vector _read_lengths; }; -class NestedBuildReader final : public ParquetColumnReader { -public: - explicit NestedBuildReader(int64_t values_to_build) - : ParquetColumnReader(int64_schema("nested"), std::make_shared()), - _values_to_build(values_to_build) {} - - Status read(int64_t, MutableColumnPtr&, int64_t*) override { - return Status::NotSupported("unused"); - } - - Status build_nested_column(int64_t length_upper_bound, MutableColumnPtr& column, - int64_t* values_read) override { - if (column.get() == nullptr || values_read == nullptr) { - return Status::InvalidArgument("invalid mock nested build arguments"); - } - _last_length_upper_bound = length_upper_bound; - auto* values = assert_cast(column.get()); - for (int64_t value = 0; value < _values_to_build; ++value) { - values->insert_value(value); - } - *values_read = _values_to_build; - return Status::OK(); - } - - int64_t last_length_upper_bound() const { return _last_length_upper_bound; } - -private: - int64_t _values_to_build = 0; - int64_t _last_length_upper_bound = 0; -}; - class ScriptedNestedReader final : public ParquetColumnReader { public: ScriptedNestedReader(ParquetColumnSchema schema, DataTypePtr type, @@ -269,6 +239,11 @@ class ScriptedNestedReader final : public ParquetColumnReader { return Status::OK(); } + Status load_nested_levels_batch(int64_t rows) override { + _level_load_lengths.push_back(rows); + return Status::OK(); + } + Status build_nested_column(int64_t length_upper_bound, MutableColumnPtr& column, int64_t* values_read) override { _build_lengths.push_back(length_upper_bound); @@ -282,6 +257,15 @@ class ScriptedNestedReader final : public ParquetColumnReader { return Status::OK(); } + Status consume_nested_column(int64_t length_upper_bound, int64_t* values_consumed) override { + DORIS_CHECK(values_consumed != nullptr); + _consume_lengths.push_back(length_upper_bound); + set_nested_build_level_cursor(std::min(nested_build_level_cursor() + length_upper_bound, + static_cast(_def_levels.size()))); + *values_consumed = length_upper_bound; + return Status::OK(); + } + const std::vector& nested_definition_levels() const override { return _def_levels; } const std::vector& nested_repetition_levels() const override { return _rep_levels; } int64_t nested_levels_written() const override { @@ -290,6 +274,8 @@ class ScriptedNestedReader final : public ParquetColumnReader { bool is_or_has_repeated_child() const override { return _has_repeated_child; } const std::vector& build_lengths() const { return _build_lengths; } + const std::vector& consume_lengths() const { return _consume_lengths; } + const std::vector& level_load_lengths() const { return _level_load_lengths; } private: static void insert_value(MutableColumnPtr& column, int64_t value, bool is_null) { @@ -312,7 +298,9 @@ class ScriptedNestedReader final : public ParquetColumnReader { bool _build_nulls = false; int64_t _next_value = 0; std::vector _load_lengths; + std::vector _level_load_lengths; std::vector _build_lengths; + std::vector _consume_lengths; }; class ChunkedNestedLeafReader final : public ParquetColumnReader { @@ -332,6 +320,13 @@ class ChunkedNestedLeafReader final : public ParquetColumnReader { return Status::OK(); } + Status load_nested_levels_batch(int64_t rows) override { + _level_load_lengths.push_back(rows); + _def_levels.assign(static_cast(rows), 1); + _rep_levels.assign(static_cast(rows), 0); + return Status::OK(); + } + Status build_nested_column(int64_t length_upper_bound, MutableColumnPtr& column, int64_t* values_read) override { DORIS_CHECK(column.get() != nullptr); @@ -354,6 +349,13 @@ class ChunkedNestedLeafReader final : public ParquetColumnReader { return Status::OK(); } + Status consume_nested_column(int64_t length_upper_bound, int64_t* values_consumed) override { + DORIS_CHECK(values_consumed != nullptr); + _consume_lengths.push_back(length_upper_bound); + *values_consumed = length_upper_bound; + return Status::OK(); + } + const std::vector& nested_definition_levels() const override { return _def_levels; } const std::vector& nested_repetition_levels() const override { return _rep_levels; } int64_t nested_levels_written() const override { @@ -363,13 +365,17 @@ class ChunkedNestedLeafReader final : public ParquetColumnReader { const std::vector& load_lengths() const { return _load_lengths; } const std::vector& build_lengths() const { return _build_lengths; } + const std::vector& consume_lengths() const { return _consume_lengths; } + const std::vector& level_load_lengths() const { return _level_load_lengths; } const std::vector& initial_column_sizes() const { return _initial_column_sizes; } private: std::vector _def_levels; std::vector _rep_levels; std::vector _load_lengths; + std::vector _level_load_lengths; std::vector _build_lengths; + std::vector _consume_lengths; std::vector _initial_column_sizes; }; @@ -514,26 +520,88 @@ TEST(ParquetColumnReaderControlTest, BaseNestedDefaultsAndSkipNested) { int64_t values_read = 0; EXPECT_FALSE(base_reader.build_nested_column(1, column, &values_read).ok()); - NestedBuildReader ok_reader(3); - ASSERT_TRUE(ok_reader.skip_nested_column(3).ok()); - EXPECT_EQ(ok_reader.last_length_upper_bound(), 3); - - NestedBuildReader short_reader(2); - EXPECT_FALSE(short_reader.skip_nested_column(3).ok()); + int64_t values_consumed = 0; + EXPECT_FALSE(base_reader.consume_nested_column(1, &values_consumed).ok()); } -TEST(ParquetColumnReaderControlTest, NestedSkipMaterializesBoundedBatches) { +TEST(ParquetColumnReaderControlTest, NestedSkipConsumesBoundedBatchesWithoutMaterializing) { auto element_reader = std::make_unique(); auto* element_reader_ptr = element_reader.get(); ListColumnReader reader(bare_repeated_int64_list_schema(), bare_repeated_int64_list_schema().type, std::move(element_reader)); ASSERT_TRUE(reader.skip(8193).ok()); - EXPECT_EQ(element_reader_ptr->load_lengths(), std::vector({4096, 4096, 1})); - EXPECT_EQ(element_reader_ptr->build_lengths(), std::vector({4096, 4096, 1})); - // Each child build sees an empty destination, proving the parent scratch column from the - // previous batch was destroyed instead of retaining all skipped nested values. - EXPECT_EQ(element_reader_ptr->initial_column_sizes(), std::vector({0, 0, 0})); + EXPECT_TRUE(element_reader_ptr->load_lengths().empty()); + EXPECT_EQ(element_reader_ptr->level_load_lengths(), std::vector({4096, 4096, 1})); + EXPECT_EQ(element_reader_ptr->consume_lengths(), std::vector({4096, 4096, 1})); + EXPECT_TRUE(element_reader_ptr->build_lengths().empty()); + EXPECT_TRUE(element_reader_ptr->initial_column_sizes().empty()); +} + +TEST(ParquetColumnReaderControlTest, MapSkipConsumesBothStreamsWithoutMaterializing) { + const std::vector def_levels {3, 3, 1, 3}; + const std::vector rep_levels {0, 1, 0, 0}; + auto key_reader = std::make_unique( + nested_int64_schema("key", 2, 3, 1, 2), + make_nullable(std::make_shared()), def_levels, rep_levels); + auto* key_reader_ptr = key_reader.get(); + auto value_reader = std::make_unique( + nested_int64_schema("value", 2, 3, 1, 2), + make_nullable(std::make_shared()), def_levels, rep_levels); + auto* value_reader_ptr = value_reader.get(); + MapColumnReader reader(nested_map_schema(), nested_map_schema().type, std::move(key_reader), + std::move(value_reader)); + + ASSERT_TRUE(reader.skip(3).ok()); + EXPECT_EQ(key_reader_ptr->level_load_lengths(), std::vector({3})); + EXPECT_EQ(value_reader_ptr->level_load_lengths(), std::vector({3})); + EXPECT_EQ(key_reader_ptr->consume_lengths(), std::vector({3})); + EXPECT_EQ(value_reader_ptr->consume_lengths(), std::vector({3})); + EXPECT_TRUE(key_reader_ptr->build_lengths().empty()); + EXPECT_TRUE(value_reader_ptr->build_lengths().empty()); +} + +TEST(ParquetColumnReaderControlTest, StructSkipConsumesNullSeparatedChildSpans) { + const std::vector def_levels {2, 0, 2}; + const std::vector rep_levels {0, 0, 0}; + auto shape_reader = std::make_unique( + nested_int64_schema("shape", 1, 2), make_nullable(std::make_shared()), + def_levels, rep_levels); + auto* shape_reader_ptr = shape_reader.get(); + auto child_reader = std::make_unique( + nested_int64_schema("child", 1, 2), make_nullable(std::make_shared()), + def_levels, rep_levels); + auto* child_reader_ptr = child_reader.get(); + std::vector> children; + children.push_back(std::move(shape_reader)); + children.push_back(std::move(child_reader)); + StructColumnReader reader(nested_struct_schema(), nested_struct_schema().type, + std::move(children), {-1, 0}); + + ASSERT_TRUE(reader.skip(3).ok()); + EXPECT_EQ(shape_reader_ptr->level_load_lengths(), std::vector({3})); + EXPECT_EQ(child_reader_ptr->level_load_lengths(), std::vector({3})); + EXPECT_EQ(shape_reader_ptr->consume_lengths(), std::vector({1, 1})); + EXPECT_EQ(child_reader_ptr->consume_lengths(), std::vector({1, 1})); + EXPECT_TRUE(shape_reader_ptr->build_lengths().empty()); + EXPECT_TRUE(child_reader_ptr->build_lengths().empty()); +} + +TEST(ParquetColumnReaderControlTest, NestedListSkipConsumesRecursivelyWithoutMaterializing) { + auto leaf_reader = std::make_unique( + nested_int64_schema("leaf", 0, 1, 1, 1), std::make_shared(), + std::vector {1, 1}, std::vector {0, 0}); + auto* leaf_reader_ptr = leaf_reader.get(); + const auto inner_type = std::make_shared(std::make_shared()); + auto inner_reader = std::make_unique(bare_repeated_int64_list_schema(), + inner_type, std::move(leaf_reader)); + const auto outer_type = std::make_shared(inner_type); + ListColumnReader reader(bare_repeated_int64_list_schema(), outer_type, std::move(inner_reader)); + + ASSERT_TRUE(reader.skip(2).ok()); + EXPECT_EQ(leaf_reader_ptr->level_load_lengths(), std::vector({2})); + EXPECT_EQ(leaf_reader_ptr->consume_lengths(), std::vector({2})); + EXPECT_TRUE(leaf_reader_ptr->build_lengths().empty()); } TEST(ParquetColumnReaderControlTest, NestedMaterializerHelpersAppendOffsetsAndParentNulls) { @@ -937,6 +1005,34 @@ TEST(ParquetColumnReaderControlTest, MapRejectsNullKeysAndMisalignedScalarValueR EXPECT_NE(status.to_string().find("value repetition level is not aligned"), std::string::npos); } +TEST(ParquetColumnReaderControlTest, MapConsumePreservesKeyAndValueCorruptionChecks) { + auto null_key_reader = + make_scripted_scalar_reader(nested_int64_schema("key", 2, 3, 1, 2), + scalar_batch({2}, {0}, {-1}, std::vector {})); + auto value_reader = std::make_unique( + nested_int64_schema("value", 2, 3, 1, 2), + make_nullable(std::make_shared()), std::vector {2}, + std::vector {0}); + MapColumnReader null_key_reader_map(nested_map_schema(), nested_map_schema().type, + std::move(null_key_reader), std::move(value_reader)); + int64_t values_consumed = 0; + auto status = null_key_reader_map.consume_nested_column(1, &values_consumed); + EXPECT_FALSE(status.ok()); + EXPECT_NE(status.to_string().find("contains null"), std::string::npos); + + auto key_reader = std::make_unique( + nested_int64_schema("key", 1, 2, 1), make_nullable(std::make_shared()), + std::vector {2, 2}, std::vector {0, 1}); + auto misaligned_value_reader = + make_scripted_scalar_reader(nested_int64_schema("value", 2, 3, 1), + scalar_batch({3, 3}, {0, 0}, {0, 1}, {100, 200})); + MapColumnReader misaligned_reader(nested_map_schema(), nested_map_schema().type, + std::move(key_reader), std::move(misaligned_value_reader)); + status = misaligned_reader.consume_nested_column(1, &values_consumed); + EXPECT_FALSE(status.ok()); + EXPECT_NE(status.to_string().find("value repetition level is not aligned"), std::string::npos); +} + TEST(ParquetColumnReaderControlTest, MapBuildsScalarAndComplexValuePaths) { auto key_reader = std::make_unique( nested_int64_schema("key", 1, 2, 1), make_nullable(std::make_shared()), diff --git a/be/test/format_v2/table_reader_test.cpp b/be/test/format_v2/table_reader_test.cpp index cfd30d85803680..41052ca2c4b97a 100644 --- a/be/test/format_v2/table_reader_test.cpp +++ b/be/test/format_v2/table_reader_test.cpp @@ -1528,6 +1528,7 @@ TEST(TableReaderTest, PendingRuntimeFilterDisablesTableLevelCount) { RuntimeState state {TQueryOptions(), TQueryGlobals()}; auto fake_state = std::make_shared(); + fake_state->aggregate_count = state.batch_size() + 5; FakeTableReader reader(file_schema, fake_state); ASSERT_TRUE(reader.init({ .projected_columns = projected_columns, @@ -1558,6 +1559,61 @@ TEST(TableReaderTest, PendingRuntimeFilterDisablesTableLevelCount) { ASSERT_TRUE(reader.close().ok()); } +TEST(TableReaderTest, PendingRuntimeFilterDisablesMinMaxPushdown) { + const auto test_dir = + std::filesystem::temp_directory_path() / "doris_table_reader_pending_rf_minmax_test"; + std::filesystem::remove_all(test_dir); + std::filesystem::create_directories(test_dir); + const auto file_path = (test_dir / "split.parquet").string(); + write_int_pair_parquet_file(file_path, {3, 1, 5, 2}, {30, 10, 50, 20}, + {"three", "one", "five", "two"}, 2); + + std::vector projected_columns; + projected_columns.push_back(make_table_column(0, "id", std::make_shared())); + projected_columns.push_back(make_table_column(1, "score", std::make_shared())); + set_name_identifiers(&projected_columns); + + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + TableReader reader; + ASSERT_TRUE(reader.init({ + .projected_columns = projected_columns, + .conjuncts = {}, + .format = FileFormat::PARQUET, + .scan_params = nullptr, + .io_ctx = nullptr, + .runtime_state = &state, + .scanner_profile = nullptr, + .push_down_agg_type = TPushAggOp::type::MINMAX, + }) + .ok()); + auto split_options = build_split_options(file_path); + split_options.all_runtime_filters_applied = false; + ASSERT_TRUE(reader.prepare_split(split_options).ok()); + + bool eos = false; + size_t total_rows = 0; + bool checked_first_batch = false; + while (!eos) { + Block block = build_table_block(projected_columns); + ASSERT_TRUE(reader.get_block(&block, &eos).ok()); + total_rows += block.rows(); + if (!checked_first_batch && block.rows() > 0) { + const auto& ids = + assert_cast(expect_not_null_table_column(block, 0)); + ASSERT_EQ(ids.size(), 2); + EXPECT_EQ(ids.get_element(0), 3); + EXPECT_EQ(ids.get_element(1), 1); + checked_first_batch = true; + } + } + // MIN/MAX pushdown would return the two synthetic extrema [1, 5]. Reading the original first + // row group [3, 1] and all four rows proves a pending RF kept the physical reader active. + EXPECT_TRUE(checked_first_batch); + EXPECT_EQ(total_rows, 4); + ASSERT_TRUE(reader.close().ok()); + std::filesystem::remove_all(test_dir); +} + TEST(TableReaderTest, AbortSplitClearsReaderAfterIgnorableNotFound) { std::vector file_schema; file_schema.push_back(make_file_column(0, "id", std::make_shared())); From 888dc357e7ff7fd6935183a17a92f73e8882ad50 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Mon, 13 Jul 2026 09:11:00 +0800 Subject: [PATCH 21/23] [fix](be) Reset binary builders after nested level reads ### What problem does this PR solve? Issue Number: close #65498 Related PR: #65498 Problem Summary: The Parquet levels-only nested path called Arrow RecordReader::Reset() without first releasing ByteArray and FLBA builder chunks. Arrow requires GetBuilderChunks() to reset those builders, so a nested skip followed by a normal read mixed values from both batches and failed with values greater than the current definition/repetition levels. Release and immediately discard binary chunks after copying levels, without copying payloads into Doris Columns. Also derive the scalar consume slot threshold from schema metadata because levels-only batches intentionally omit value-slot metadata. ### Release note Fix nested Parquet queries that skip filtered rows before reading plain-encoded string values. ### Check List (For Author) - Test: Unit Test - 95 targeted Parquet reader, levels-only COUNT, and TableReader tests - Plain-encoded nested MAP skip-then-read regression test - Remote ./build.sh --be - Behavior changed: Yes (levels-only nested reads correctly reset Arrow binary builders) - Does this need documentation: No --- .../format_v2/parquet/reader/column_reader.h | 9 +-- .../parquet/reader/parquet_leaf_reader.cpp | 20 ++++-- .../parquet/reader/parquet_leaf_reader.h | 20 +++--- .../parquet/reader/scalar_column_reader.cpp | 7 +- .../parquet/parquet_column_reader_test.cpp | 65 +++++++++++++++++++ 5 files changed, 103 insertions(+), 18 deletions(-) diff --git a/be/src/format_v2/parquet/reader/column_reader.h b/be/src/format_v2/parquet/reader/column_reader.h index 9416c4eb22739f..51dbd44c11c226 100644 --- a/be/src/format_v2/parquet/reader/column_reader.h +++ b/be/src/format_v2/parquet/reader/column_reader.h @@ -84,8 +84,9 @@ class ParquetColumnReader { // nested_definition_levels(), nested_repetition_levels(), and nested_levels_written() are // available; value indices and payload columns may be absent. Callers may inspect the levels or // call consume_nested_column(), but must not call build_nested_column() afterwards. For example, - // skipping ARRAY uses this method to find ARRAY boundaries without decoding strings. - // Normal scans that need output values use load_nested_batch() instead. + // skipping ARRAY uses this method to find ARRAY boundaries without constructing a + // ColumnString. The underlying Arrow reader may still decode a page because it has no public + // levels-only API. Normal scans that need output values use load_nested_batch() instead. virtual Status load_nested_levels_batch(int64_t rows); virtual Status build_nested_column(int64_t length_upper_bound, MutableColumnPtr& column, @@ -95,7 +96,7 @@ class ParquetColumnReader { // load_nested_levels_batch() without appending them to an output Column. Implementations must // advance exactly the same nested level cursors and perform the same shape/null/alignment // validation as build_nested_column(). The levels-only form is preferred for skip paths because - // it also avoids decoding leaf payloads that will be discarded. + // it avoids transferring leaf payloads into Doris Columns when they will be discarded. // // `length_upper_bound` is expressed at this reader's logical level, not in physical leaf // values. For example, consuming two rows from ARRAY [[1, 2], []] consumes two parent ARRAY @@ -126,7 +127,7 @@ class ParquetColumnReader { ParquetColumnReader() = default; // Load shape levels and consume skipped parent rows in bounded batches. The bound limits level // memory when a parent expands to many children; the levels-only load plus - // consume_nested_column() avoids both payload decoding and output Columns. + // consume_nested_column() avoids payload materialization and output Columns. Status skip_nested_rows(int64_t rows); void update_reader_read_rows(int64_t rows) const; void update_reader_skip_rows(int64_t rows) const; diff --git a/be/src/format_v2/parquet/reader/parquet_leaf_reader.cpp b/be/src/format_v2/parquet/reader/parquet_leaf_reader.cpp index cb59571d14123f..fd261ef5219d27 100644 --- a/be/src/format_v2/parquet/reader/parquet_leaf_reader.cpp +++ b/be/src/format_v2/parquet/reader/parquet_leaf_reader.cpp @@ -340,10 +340,22 @@ Status ParquetLeafReader::collect_levels_batch(::parquet::internal::RecordReader } batch->_read_dense_for_nullable = record_reader.read_dense_for_nullable(); - // Deliberately ignore values_written(), values() and BinaryRecordReader::GetBuilderChunks(). - // COUNT(col) only needs top-level shape. Pulling binary chunks transfers Arrow builder - // ownership into Doris arrays and later into ColumnString, which is exactly the OOM-prone - // materialization path for huge MAP/ARRAY/STRUCT string payloads. + // Arrow's RecordReader::Reset() does not reset ByteArray/FLBA builders. GetBuilderChunks() + // (or DictionaryRecordReader::GetResult()) is the documented reset operation and must be + // called before the next ReadRecords(). Otherwise a levels-only skip followed by a normal read + // observes values from both batches; for example, skipping ARRAY ["a", "b"] and then + // reading ["c"] would report one current level but three values. Release the chunks here and + // let the temporary vector destroy them immediately. We deliberately do not inspect or copy + // their payload into a Doris Column, so the levels-only contract still avoids Doris-side value + // materialization. + if (batch->is_binary_value()) { + std::vector> discarded_chunks; + RETURN_IF_ERROR(get_binary_chunks(_name, record_reader, &discarded_chunks)); + } + + // COUNT(col) and nested skip only need top-level shape. Fixed-width values remain owned by the + // RecordReader and are cleared by Reset(); binary values were released above solely to reset + // the Arrow builder. batch->_values_written = 0; return Status::OK(); } diff --git a/be/src/format_v2/parquet/reader/parquet_leaf_reader.h b/be/src/format_v2/parquet/reader/parquet_leaf_reader.h index 7d97d0ad6985f9..b396b35fd1f32c 100644 --- a/be/src/format_v2/parquet/reader/parquet_leaf_reader.h +++ b/be/src/format_v2/parquet/reader/parquet_leaf_reader.h @@ -119,15 +119,16 @@ class ParquetLeafReader { ParquetNestedScalarBatch* batch, int16_t value_slot_repetition_level = std::numeric_limits::max()) const; - // COUNT(col) shape-only read path. It still calls Arrow RecordReader::ReadRecords() - // to advance the Parquet cursor and obtain def/rep levels, but Doris only copies levels: - // - it does not call BinaryRecordReader::GetBuilderChunks() + // COUNT(col) and nested-skip shape-only read path. It still calls Arrow + // RecordReader::ReadRecords() to advance the Parquet cursor and obtain def/rep levels, but + // Doris only copies levels: // - it does not build value_indices or values_column // - it does not enter DataTypeSerde::read_column_from_decoded_values() - // This lets COUNT(col) on MAP/ARRAY/STRUCT evaluate top-level NULL state while avoiding - // materializing representative leaf STRING/BINARY payloads into Doris Columns. Arrow RecordReader - // does not expose a public levels-only API, so ReadRecords may still perform required page decoding; - // this API guarantees that the V2 reader does not take ownership of or copy value payloads. + // - for Binary/FLBA, it releases and immediately discards Arrow builder chunks because that is + // the RecordReader's required reset operation; it never copies them into a Doris Column + // This lets COUNT(col) on MAP/ARRAY/STRUCT evaluate top-level NULL state and lets skip advance + // nested shape without Doris-side STRING/BINARY materialization. Arrow RecordReader does not + // expose a public levels-only API, so ReadRecords may still perform required page decoding. Status read_nested_levels_batch(int64_t batch_rows, ParquetNestedScalarBatch* batch) const; private: @@ -136,8 +137,9 @@ class ParquetLeafReader { Status collect_batch(::parquet::internal::RecordReader& record_reader, ParquetLeafBatch* batch) const; - // Levels-only variant of collect_batch(). It snapshots only def/rep level state and does not take - // binary chunks or expose fixed-width value buffers. Used by the COUNT(col) aggregation fast path. + // Levels-only variant of collect_batch(). It snapshots only def/rep level state and does not + // expose value buffers. Binary chunks are released only to reset Arrow's builder and are + // immediately discarded. Used by COUNT(col) and nested skip. Status collect_levels_batch(::parquet::internal::RecordReader& record_reader, ParquetLeafBatch* batch) const; diff --git a/be/src/format_v2/parquet/reader/scalar_column_reader.cpp b/be/src/format_v2/parquet/reader/scalar_column_reader.cpp index 1e21c71b211acd..784e4cdc900fb7 100644 --- a/be/src/format_v2/parquet/reader/scalar_column_reader.cpp +++ b/be/src/format_v2/parquet/reader/scalar_column_reader.cpp @@ -487,7 +487,12 @@ Status ScalarColumnReader::consume_nested_column(int64_t length_upper_bound, _name); } DORIS_CHECK(_nested_batch != nullptr); - const int16_t materialized_slot_definition_level = _nested_batch->value_slot_definition_level; + // A levels-only batch intentionally has no value-slot metadata. Reconstruct the same logical + // slot threshold used by load_nested_batch(): a nullable leaf owns a slot at one definition + // level below a non-null value, while a required leaf owns a slot only at its full definition + // level. For example, an empty ARRAY boundary must not be consumed as a STRING value. + const int16_t materialized_slot_definition_level = + static_cast(_definition_level - (_type->is_nullable() ? 1 : 0)); *values_consumed = 0; int64_t level_idx = nested_build_level_cursor(); while (level_idx < _nested_batch->levels_written && *values_consumed < length_upper_bound) { diff --git a/be/test/format_v2/parquet/parquet_column_reader_test.cpp b/be/test/format_v2/parquet/parquet_column_reader_test.cpp index f9bfe137cf74eb..fb4cd129e1b03d 100644 --- a/be/test/format_v2/parquet/parquet_column_reader_test.cpp +++ b/be/test/format_v2/parquet/parquet_column_reader_test.cpp @@ -134,6 +134,7 @@ class ParquetColumnReaderTest : public testing::Test { std::filesystem::remove_all(_test_dir); std::filesystem::create_directories(_test_dir); _file_path = (_test_dir / "reader.parquet").string(); + _plain_file_path = (_test_dir / "plain_reader.parquet").string(); write_parquet_file(); _file_reader = ::parquet::ParquetFileReader::OpenFile(_file_path, false); auto metadata = _file_reader->metadata(); @@ -1819,6 +1820,38 @@ class ParquetColumnReaderTest : public testing::Test { return reader; } + std::unique_ptr create_plain_reader(size_t field_idx) { + // Keep the normal fixture dictionary encoded. This one test writes a plain-encoded copy + // because Arrow BinaryRecordReader has a stricter reset contract than + // DictionaryRecordReader. + auto schema = arrow::schema(_arrow_fields); + auto table = arrow::Table::Make(schema, _arrays); + auto plain_file_result = arrow::io::FileOutputStream::Open(_plain_file_path); + DORIS_CHECK(plain_file_result.ok()); + std::shared_ptr plain_out = *plain_file_result; + ::parquet::WriterProperties::Builder plain_builder; + plain_builder.version(::parquet::ParquetVersion::PARQUET_2_6); + plain_builder.data_page_version(::parquet::ParquetDataPageVersion::V2); + plain_builder.compression(::parquet::Compression::UNCOMPRESSED); + plain_builder.disable_dictionary(); + PARQUET_THROW_NOT_OK(::parquet::arrow::WriteTable( + *table, arrow::default_memory_pool(), plain_out, ROW_COUNT, plain_builder.build())); + DORIS_CHECK(plain_out->Close().ok()); + + _plain_file_reader = ::parquet::ParquetFileReader::OpenFile(_plain_file_path, false); + auto metadata = _plain_file_reader->metadata(); + DORIS_CHECK(metadata != nullptr); + DORIS_CHECK(metadata->num_row_groups() == 1); + _plain_row_group = _plain_file_reader->RowGroup(0); + DORIS_CHECK(_plain_row_group != nullptr); + + ParquetColumnReaderFactory factory(_plain_row_group, metadata->num_columns()); + std::unique_ptr reader; + auto st = factory.create(*_fields[field_idx], &reader); + EXPECT_TRUE(st.ok()) << st; + return reader; + } + std::unique_ptr create_projected_child_reader(size_t field_idx, size_t child_idx) const { const auto& struct_schema = *_fields[field_idx]; @@ -1887,8 +1920,11 @@ class ParquetColumnReaderTest : public testing::Test { std::filesystem::path _test_dir; std::string _file_path; + std::string _plain_file_path; std::unique_ptr<::parquet::ParquetFileReader> _file_reader; + std::unique_ptr<::parquet::ParquetFileReader> _plain_file_reader; std::shared_ptr<::parquet::RowGroupReader> _row_group; + std::shared_ptr<::parquet::RowGroupReader> _plain_row_group; std::vector> _fields; std::vector> _arrow_fields; std::vector> _arrays; @@ -3192,6 +3228,35 @@ TEST_F(ParquetColumnReaderTest, SkipMapWithOverflowThenRead) { EXPECT_EQ(offsets[2], 1); } +TEST_F(ParquetColumnReaderTest, SkipPlainBinaryMapThenReadResetsArrowBuilder) { + const auto field_idx = find_field_idx("nullable_map_int_string_col"); + auto reader = create_plain_reader(field_idx); + + // Row 0 contains two STRING values. The levels-only skip must release (and discard) those + // Arrow BinaryRecordReader builder chunks before the next normal read. If they leak into the + // next batch, ParquetLeafReader observes more values than current definition/repetition levels. + auto st = reader->skip(1); + ASSERT_TRUE(st.ok()) << st; + + MutableColumnPtr column = reader->type()->create_column(); + int64_t rows_read = 0; + st = reader->read(3, column, &rows_read); + ASSERT_TRUE(st.ok()) << st; + ASSERT_EQ(rows_read, 3); + + const auto& nullable_column = assert_cast(*column); + ASSERT_EQ(nullable_column.size(), 3); + EXPECT_TRUE(nullable_column.is_null_at(0)); + const auto& map_column = assert_cast(nullable_column.get_nested_column()); + ASSERT_EQ(map_column.get_offsets().size(), 3); + EXPECT_EQ(map_column.get_offsets()[0], 0); + EXPECT_EQ(map_column.get_offsets()[1], 0); + EXPECT_EQ(map_column.get_offsets()[2], 1); + const auto& values = get_nullable_nested_column(map_column.get_values()); + ASSERT_EQ(values.size(), 1); + EXPECT_EQ(values.get_data_at(0).to_string(), "cc"); +} + TEST_F(ParquetColumnReaderTest, SelectMapWithOverflow) { const auto field_idx = find_field_idx("nullable_map_int_string_col"); auto reader = create_reader(field_idx); From 0c0fb397ded10553f977f816b6f9c112a98e1c4e Mon Sep 17 00:00:00 2001 From: Gabriel Date: Mon, 13 Jul 2026 10:18:08 +0800 Subject: [PATCH 22/23] [fix](be) Preserve direct MINMAX pushdown after projection ### What problem does this PR solve? Issue Number: None Related PR: #65500 Problem Summary: Combining the Parquet reader changes exposed that direct file mappings carry a slot-ref materialization projection. Treating every projection as unsafe disabled valid MIN/MAX statistics pushdown and made existing direct-mapping tests return raw rows. Use mapping triviality as the safety boundary: direct slot-ref mappings remain eligible while casts and other conversions still fall back to row scanning. ### Release note Fix MIN/MAX aggregate pushdown for direct external-file column mappings. ### Check List (For Author) - Test: Unit Test - Remote ASAN BE unit tests: ColumnMapperLocalizeFiltersTest.*, TableReaderTest.*, and ParquetColumnReaderTest.* (121 passed) - Behavior changed: Yes (direct mappings retain MIN/MAX statistics pushdown; converted mappings still fall back) - Does this need documentation: No --- be/src/format_v2/table_reader.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/be/src/format_v2/table_reader.h b/be/src/format_v2/table_reader.h index bcf90653cc3326..534674a7283a0f 100644 --- a/be/src/format_v2/table_reader.h +++ b/be/src/format_v2/table_reader.h @@ -1588,7 +1588,10 @@ class TableReader { static bool _can_push_down_minmax_for_mapping(const ColumnMapping& mapping) { if (mapping.child_mappings.empty()) { - return mapping.is_trivial && mapping.projection == nullptr; + // Direct mappings use a slot-ref projection to materialize the file column. The + // projection does not transform ordering; casts and other conversions are already + // represented by a non-trivial mapping and must fall back to row scanning. + return mapping.is_trivial; } const auto primitive_type = remove_nullable(mapping.file_type)->get_primitive_type(); if (primitive_type != TYPE_STRUCT) { From db14c3e2ef430d234ebb7b375ee19156ebb39132 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Mon, 13 Jul 2026 12:20:22 +0800 Subject: [PATCH 23/23] [fix](be) Address external reader review gaps ### What problem does this PR solve? Issue Number: None Related PR: #65500 Problem Summary: External reader review found four remaining correctness gaps: slotless conjuncts could still permit aggregate pushdown, TIMESTAMPTZ scale-mismatch filters could be localized with different rounding semantics, nested VARBINARY predicates could reach file readers, and decoded ORC statistics did not consistently reject inverted or NaN bounds. Disable aggregate pushdown whenever original conjuncts remain, keep unsafe filter conversions above the reader, and validate every decoded ORC statistics range before using it for metadata pruning or MIN/MAX aggregation. ### Release note Fix external table filtering and metadata aggregate correctness for slotless predicates, TIMESTAMPTZ scale mismatches, nested VARBINARY values, and invalid ORC statistics. ### Check List (For Author) - Test: Unit Test - Remote ASAN BE unit tests: 20 targeted tests passed - Remote clang-format 16 check passed - Remote clang-tidy attempted; blocked by missing stddef.h and pre-existing diagnostics in the validation toolchain - Behavior changed: Yes, unsafe filter and aggregate pushdown paths now fall back to row-level evaluation - Does this need documentation: No --- be/src/format_v2/column_mapper.cpp | 50 ++++++++- be/src/format_v2/orc/orc_reader.cpp | 123 +++++++++++++--------- be/src/format_v2/orc/orc_reader.h | 4 + be/src/format_v2/table_reader.h | 6 ++ be/test/format_v2/column_mapper_test.cpp | 36 ++++++- be/test/format_v2/orc/orc_reader_test.cpp | 19 ++++ be/test/format_v2/table_reader_test.cpp | 44 ++++++++ 7 files changed, 225 insertions(+), 57 deletions(-) diff --git a/be/src/format_v2/column_mapper.cpp b/be/src/format_v2/column_mapper.cpp index baa156c44dfe6b..572e76280a0e37 100644 --- a/be/src/format_v2/column_mapper.cpp +++ b/be/src/format_v2/column_mapper.cpp @@ -1078,18 +1078,61 @@ static bool mapping_can_use_file_column_directly(const ColumnMapping& mapping) { return !needs_complex_rematerialize(mapping); } +static bool type_contains_varbinary(const DataTypePtr& type) { + DORIS_CHECK(type != nullptr); + const auto nested_type = remove_nullable(type); + switch (nested_type->get_primitive_type()) { + case TYPE_VARBINARY: + return true; + case TYPE_ARRAY: + return type_contains_varbinary( + assert_cast(*nested_type).get_nested_type()); + case TYPE_MAP: { + const auto& map_type = assert_cast(*nested_type); + return type_contains_varbinary(map_type.get_key_type()) || + type_contains_varbinary(map_type.get_value_type()); + } + case TYPE_STRUCT: + return std::ranges::any_of( + assert_cast(*nested_type).get_elements(), + [](const DataTypePtr& child_type) { return type_contains_varbinary(child_type); }); + default: + return false; + } +} + static FilterConversionType direct_filter_conversion(const ColumnMapping& mapping) { DORIS_CHECK(mapping.table_type != nullptr); + DORIS_CHECK(mapping.file_type != nullptr); // FileScanOperator deliberately keeps VARBINARY predicates above external readers. Their // physical binary representations are not uniformly supported by reader-side expression and // metadata filtering, so localizing a late runtime filter here can incorrectly reject rows. - if (remove_nullable(mapping.table_type)->get_primitive_type() == TYPE_VARBINARY) { + // Apply the same rule to a complex root because generic array/map/struct expressions rewrite + // the root slot and can otherwise expose a nested VARBINARY child to the reader. + if (type_contains_varbinary(mapping.table_type)) { + return FilterConversionType::FINALIZE_ONLY; + } + const auto table_type = remove_nullable(mapping.table_type); + const auto file_type = remove_nullable(mapping.file_type); + // TIMESTAMPTZ scale mismatch is intentionally materialized as pass-through: a SQL cast rounds + // fractional seconds. A file-local cast would therefore filter different instants from the + // scanner-level predicate evaluated on the pass-through value. + if (table_type->get_primitive_type() == TYPE_TIMESTAMPTZ && + file_type->get_primitive_type() == TYPE_TIMESTAMPTZ && + !mapping.table_type->equals(*mapping.file_type)) { return FilterConversionType::FINALIZE_ONLY; } return mapping.is_trivial ? FilterConversionType::COPY_DIRECTLY : FilterConversionType::CAST_FILTER; } +static FilterConversionType projected_filter_conversion(const ColumnMapping& mapping) { + const auto conversion = direct_filter_conversion(mapping); + return !mapping.is_trivial && conversion != FilterConversionType::FINALIZE_ONLY + ? FilterConversionType::READER_EXPRESSION + : conversion; +} + static const ColumnDefinition* find_file_child_for_mapping(const ColumnDefinition& table_child, const ColumnDefinition& file_parent, TableColumnMappingMode mode, @@ -2054,10 +2097,7 @@ Status TableColumnMapper::_create_direct_mapping(const ColumnDefinition& table_c &mapping->projected_file_children, &mapping->file_type)); DCHECK(mapping->table_type != nullptr); mapping->is_trivial = mapping_can_use_file_column_directly(*mapping); - // TODO: ? READER_EXPRESSION - mapping->filter_conversion = mapping->is_trivial - ? FilterConversionType::COPY_DIRECTLY - : FilterConversionType::READER_EXPRESSION; + mapping->filter_conversion = projected_filter_conversion(*mapping); } } return Status::OK(); diff --git a/be/src/format_v2/orc/orc_reader.cpp b/be/src/format_v2/orc/orc_reader.cpp index fd10a777388995..eb6bb7895dd6e5 100644 --- a/be/src/format_v2/orc/orc_reader.cpp +++ b/be/src/format_v2/orc/orc_reader.cpp @@ -80,6 +80,20 @@ #include "util/timezone_utils.h" namespace doris::format::orc { + +bool detail::valid_statistics_bounds(const Field& min_value, const Field& max_value) { + DORIS_CHECK(min_value.get_type() == max_value.get_type()); + if (min_value.get_type() == TYPE_FLOAT && + (std::isnan(min_value.get()) || std::isnan(max_value.get()))) { + return false; + } + if (min_value.get_type() == TYPE_DOUBLE && + (std::isnan(min_value.get()) || std::isnan(max_value.get()))) { + return false; + } + return !(max_value < min_value); +} + namespace { constexpr uint64_t DEFAULT_ORC_READ_BATCH_SIZE = 4096; @@ -91,6 +105,16 @@ constexpr const char* ORC_MAP_KEY_NAME = "key"; constexpr const char* ORC_MAP_VALUE_NAME = "value"; constexpr const char* ORC_ICEBERG_ID_ATTRIBUTE = "iceberg.id"; +bool set_validated_zone_map(Field min_value, Field max_value, segment_v2::ZoneMap* zone_map) { + DORIS_CHECK(zone_map != nullptr); + if (!detail::valid_statistics_bounds(min_value, max_value)) { + return false; + } + zone_map->min_value = std::move(min_value); + zone_map->max_value = std::move(max_value); + return true; +} + uint64_t orc_metric_value(const std::atomic& metric) { return metric.load(std::memory_order_relaxed); } @@ -397,27 +421,25 @@ bool set_integer_zone_map(const ::orc::Type& type, const ::orc::ColumnStatistics } switch (type.getKind()) { case ::orc::TypeKind::BYTE: - zone_map->min_value = - Field::create_field(cast_set(integer_statistics->getMinimum())); - zone_map->max_value = - Field::create_field(cast_set(integer_statistics->getMaximum())); - return true; + return set_validated_zone_map( + Field::create_field(cast_set(integer_statistics->getMinimum())), + Field::create_field(cast_set(integer_statistics->getMaximum())), + zone_map); case ::orc::TypeKind::SHORT: - zone_map->min_value = Field::create_field( - cast_set(integer_statistics->getMinimum())); - zone_map->max_value = Field::create_field( - cast_set(integer_statistics->getMaximum())); - return true; + return set_validated_zone_map(Field::create_field( + cast_set(integer_statistics->getMinimum())), + Field::create_field( + cast_set(integer_statistics->getMaximum())), + zone_map); case ::orc::TypeKind::INT: - zone_map->min_value = - Field::create_field(cast_set(integer_statistics->getMinimum())); - zone_map->max_value = - Field::create_field(cast_set(integer_statistics->getMaximum())); - return true; + return set_validated_zone_map( + Field::create_field(cast_set(integer_statistics->getMinimum())), + Field::create_field(cast_set(integer_statistics->getMaximum())), + zone_map); case ::orc::TypeKind::LONG: - zone_map->min_value = Field::create_field(integer_statistics->getMinimum()); - zone_map->max_value = Field::create_field(integer_statistics->getMaximum()); - return true; + return set_validated_zone_map( + Field::create_field(integer_statistics->getMinimum()), + Field::create_field(integer_statistics->getMaximum()), zone_map); default: return false; } @@ -435,9 +457,9 @@ bool set_boolean_zone_map(const ::orc::ColumnStatistics& statistics, if (!has_false && !has_true) { return false; } - zone_map->min_value = Field::create_field(static_cast(has_false ? 0 : 1)); - zone_map->max_value = Field::create_field(static_cast(has_true ? 1 : 0)); - return true; + return set_validated_zone_map( + Field::create_field(static_cast(has_false ? 0 : 1)), + Field::create_field(static_cast(has_true ? 1 : 0)), zone_map); } bool set_floating_zone_map(const ::orc::Type& type, const ::orc::ColumnStatistics& statistics, @@ -448,16 +470,16 @@ bool set_floating_zone_map(const ::orc::Type& type, const ::orc::ColumnStatistic return false; } if (type.getKind() == ::orc::TypeKind::FLOAT) { - zone_map->min_value = Field::create_field( - static_cast(double_statistics->getMinimum())); - zone_map->max_value = Field::create_field( - static_cast(double_statistics->getMaximum())); - return true; + return set_validated_zone_map(Field::create_field(static_cast( + double_statistics->getMinimum())), + Field::create_field(static_cast( + double_statistics->getMaximum())), + zone_map); } if (type.getKind() == ::orc::TypeKind::DOUBLE) { - zone_map->min_value = Field::create_field(double_statistics->getMinimum()); - zone_map->max_value = Field::create_field(double_statistics->getMaximum()); - return true; + return set_validated_zone_map( + Field::create_field(double_statistics->getMinimum()), + Field::create_field(double_statistics->getMaximum()), zone_map); } return false; } @@ -476,9 +498,8 @@ bool set_string_zone_map(const ::orc::Type& type, const ::orc::ColumnStatistics& return Field::create_field( std::string(value.data(), trim_right_spaces(value.data(), value.size()))); }; - zone_map->min_value = build_field(string_statistics->getMinimum()); - zone_map->max_value = build_field(string_statistics->getMaximum()); - return true; + return set_validated_zone_map(build_field(string_statistics->getMinimum()), + build_field(string_statistics->getMaximum()), zone_map); } bool set_date_zone_map(const ::orc::ColumnStatistics& statistics, segment_v2::ZoneMap* zone_map) { @@ -488,11 +509,9 @@ bool set_date_zone_map(const ::orc::ColumnStatistics& statistics, segment_v2::Zo return false; } auto& date_dict = date_day_offset_dict::get(); - zone_map->min_value = - Field::create_field(date_dict[date_statistics->getMinimum()]); - zone_map->max_value = - Field::create_field(date_dict[date_statistics->getMaximum()]); - return true; + return set_validated_zone_map( + Field::create_field(date_dict[date_statistics->getMinimum()]), + Field::create_field(date_dict[date_statistics->getMaximum()]), zone_map); } DateV2Value datetime_v2_from_orc_millis(int64_t millis, int32_t nanos_tail, @@ -533,22 +552,27 @@ bool set_timestamp_zone_map(const ::orc::ColumnStatistics& statistics, return false; } if (use_timestamp_tz) { - zone_map->min_value = Field::create_field(timestamp_tz_from_orc_millis( - timestamp_statistics->getMinimum(), timestamp_statistics->getMinimumNanos())); - zone_map->max_value = Field::create_field(timestamp_tz_from_orc_millis( - timestamp_statistics->getMaximum(), timestamp_statistics->getMaximumNanos())); - return true; + return set_validated_zone_map( + Field::create_field( + timestamp_tz_from_orc_millis(timestamp_statistics->getMinimum(), + timestamp_statistics->getMinimumNanos())), + Field::create_field( + timestamp_tz_from_orc_millis(timestamp_statistics->getMaximum(), + timestamp_statistics->getMaximumNanos())), + zone_map); } if (!format::utc_timestamp_range_is_monotonic( format::floor_epoch_seconds(timestamp_statistics->getMinimum(), 1000), format::floor_epoch_seconds(timestamp_statistics->getMaximum(), 1000), timezone)) { return false; } - zone_map->min_value = Field::create_field(datetime_v2_from_orc_millis( - timestamp_statistics->getMinimum(), timestamp_statistics->getMinimumNanos(), timezone)); - zone_map->max_value = Field::create_field(datetime_v2_from_orc_millis( - timestamp_statistics->getMaximum(), timestamp_statistics->getMaximumNanos(), timezone)); - return true; + return set_validated_zone_map(Field::create_field(datetime_v2_from_orc_millis( + timestamp_statistics->getMinimum(), + timestamp_statistics->getMinimumNanos(), timezone)), + Field::create_field(datetime_v2_from_orc_millis( + timestamp_statistics->getMaximum(), + timestamp_statistics->getMaximumNanos(), timezone)), + zone_map); } int32_t decimal_scale_for_orc_type(const ::orc::Type& type) { @@ -596,9 +620,8 @@ bool set_decimal_zone_map(const ::orc::Type& type, const ::orc::ColumnStatistics if (!min_value.has_value() || !max_value.has_value()) { return false; } - zone_map->min_value = Field::create_field(*min_value); - zone_map->max_value = Field::create_field(*max_value); - return true; + return set_validated_zone_map(Field::create_field(*min_value), + Field::create_field(*max_value), zone_map); } bool build_zone_map_from_orc_statistics(const ::orc::Type& type, diff --git a/be/src/format_v2/orc/orc_reader.h b/be/src/format_v2/orc/orc_reader.h index e7de8cf5c1a6fc..2cc06dd9ec7798 100644 --- a/be/src/format_v2/orc/orc_reader.h +++ b/be/src/format_v2/orc/orc_reader.h @@ -37,6 +37,10 @@ namespace doris::format::orc { struct OrcReaderScanState; +namespace detail { +bool valid_statistics_bounds(const Field& min_value, const Field& max_value); +} // namespace detail + // ORC implementation of the format-v2 FileReader contract. The reader consumes // file-local scan requests; table schema mapping is handled before open(). class OrcReader final : public format::FileReader { diff --git a/be/src/format_v2/table_reader.h b/be/src/format_v2/table_reader.h index 534674a7283a0f..c416d0cee2b96c 100644 --- a/be/src/format_v2/table_reader.h +++ b/be/src/format_v2/table_reader.h @@ -927,6 +927,12 @@ class TableReader { if (!_all_runtime_filters_applied_for_split) { return false; } + // Scanner owns the original conjunct list and evaluates it after TableReader finalizes + // rows. Even a slotless conjunct that cannot become a TableFilter must see every source + // row before an aggregate reduces the stream to synthetic COUNT/MINMAX rows. + if (!_conjuncts.empty()) { + return false; + } // Only support aggregate pushdown when there is no delete or filter, so // the reduced rows consumed by the upper aggregate remain semantically equivalent to a // normal scan. diff --git a/be/test/format_v2/column_mapper_test.cpp b/be/test/format_v2/column_mapper_test.cpp index 29b522aee6ccbb..3885deb107f072 100644 --- a/be/test/format_v2/column_mapper_test.cpp +++ b/be/test/format_v2/column_mapper_test.cpp @@ -1189,7 +1189,7 @@ TEST(ColumnMapperCreateMappingTest, ByNameUsesFirstMatchingFileFieldWhenAmbiguou expect_mapping(mapper.mappings()[0], 0, "id", 0, "ID", int_type, int_type); } -TEST(ColumnMapperCreateMappingTest, TimestampTzScaleMismatchDoesNotAddFinalizeCast) { +TEST(ColumnMapperCreateMappingTest, TimestampTzScaleMismatchKeepsFilterAboveReader) { // Scenario: HDFS TVF may expose a table slot as TIMESTAMPTZ(0), while a Parquet logical UTC // timestamp file schema is materialized as TIMESTAMPTZ(6). Finalization must not add a SQL // cast from scale 6 to scale 0, because that cast rounds fractional seconds: @@ -1207,7 +1207,17 @@ TEST(ColumnMapperCreateMappingTest, TimestampTzScaleMismatchDoesNotAddFinalizeCa ASSERT_EQ(mapper.mappings().size(), 1); expect_mapping(mapper.mappings()[0], 0, "ts_tz", 0, "ts_tz", file_type, table_type); EXPECT_TRUE(mapper.mappings()[0].is_trivial); - EXPECT_EQ(mapper.mappings()[0].filter_conversion, FilterConversionType::COPY_DIRECTLY); + EXPECT_EQ(mapper.mappings()[0].filter_conversion, FilterConversionType::FINALIZE_ONLY); + + TableFilter filter { + .conjunct = VExprContext::create_shared(table_slot(0, 0, table_type, "ts_tz")), + .global_indices = {GlobalIndex(0)}}; + FileScanRequest request; + ASSERT_TRUE(mapper.create_scan_request({filter}, table_schema, &request).ok()); + EXPECT_TRUE(request.predicate_columns.empty()); + ASSERT_EQ(request.non_predicate_columns.size(), 1); + EXPECT_EQ(request.non_predicate_columns[0].column_id(), LocalColumnId(0)); + EXPECT_TRUE(request.conjuncts.empty()); } TEST(ColumnMapperCreateMappingTest, ByNameUsesNameMappingForRenamedColumn) { @@ -2187,6 +2197,28 @@ TEST(ColumnMapperLocalizeFiltersTest, VarbinaryFilterStaysAboveFileReader) { EXPECT_TRUE(request.conjuncts.empty()); } +TEST(ColumnMapperLocalizeFiltersTest, NestedVarbinaryFilterStaysAboveFileReader) { + const auto table_column = struct_name_col( + "payload", {name_col("id", i32()), name_col("binary_value", varbinary())}); + const auto file_column = struct_name_col( + "payload", {name_col("id", i32(), 0), name_col("binary_value", varbinary(), 1)}, 7); + + TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_NAME}); + ASSERT_TRUE(mapper.create_mapping({table_column}, {}, {file_column}).ok()); + ASSERT_EQ(mapper.mappings().size(), 1); + EXPECT_EQ(mapper.mappings()[0].filter_conversion, FilterConversionType::FINALIZE_ONLY); + + TableFilter filter { + .conjunct = VExprContext::create_shared(table_slot(0, 0, table_column.type, "payload")), + .global_indices = {GlobalIndex(0)}}; + FileScanRequest request; + ASSERT_TRUE(mapper.create_scan_request({filter}, {table_column}, &request).ok()); + EXPECT_TRUE(request.predicate_columns.empty()); + ASSERT_EQ(request.non_predicate_columns.size(), 1); + EXPECT_EQ(request.non_predicate_columns[0].column_id(), LocalColumnId(7)); + EXPECT_TRUE(request.conjuncts.empty()); +} + TEST(ColumnMapperLocalizeFiltersTest, ConstantFilterBuildsEntryWithoutFileScanColumn) { auto partition_column = name_col("part", i32()); partition_column.is_partition_key = true; diff --git a/be/test/format_v2/orc/orc_reader_test.cpp b/be/test/format_v2/orc/orc_reader_test.cpp index a6f669e227410e..3a70d7421ab734 100644 --- a/be/test/format_v2/orc/orc_reader_test.cpp +++ b/be/test/format_v2/orc/orc_reader_test.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -134,6 +135,24 @@ DateV2Value make_datetime_v2(uint16_t year, uint8_t month, return value; } +TEST(OrcStatisticsBoundsTest, RejectsInvertedAndNanDecodedBounds) { + EXPECT_TRUE(format::orc::detail::valid_statistics_bounds(Field::create_field(1), + Field::create_field(10))); + EXPECT_FALSE(format::orc::detail::valid_statistics_bounds(Field::create_field(10), + Field::create_field(1))); + EXPECT_FALSE(format::orc::detail::valid_statistics_bounds( + Field::create_field(std::numeric_limits::quiet_NaN()), + Field::create_field(10))); + EXPECT_FALSE(format::orc::detail::valid_statistics_bounds( + Field::create_field("z"), Field::create_field("a"))); + EXPECT_FALSE(format::orc::detail::valid_statistics_bounds( + Field::create_field(make_date_v2(2026, 7, 13)), + Field::create_field(make_date_v2(2026, 7, 12)))); + EXPECT_FALSE(format::orc::detail::valid_statistics_bounds( + Field::create_field(Decimal128V3(10)), + Field::create_field(Decimal128V3(1)))); +} + class TableLiteral : public VLiteral { public: template diff --git a/be/test/format_v2/table_reader_test.cpp b/be/test/format_v2/table_reader_test.cpp index 41052ca2c4b97a..fc376e31533e20 100644 --- a/be/test/format_v2/table_reader_test.cpp +++ b/be/test/format_v2/table_reader_test.cpp @@ -1614,6 +1614,50 @@ TEST(TableReaderTest, PendingRuntimeFilterDisablesMinMaxPushdown) { std::filesystem::remove_all(test_dir); } +TEST(TableReaderTest, SlotlessConjunctDisablesAggregatePushdown) { + std::vector file_schema; + file_schema.push_back(make_file_column(0, "id", std::make_shared())); + + std::vector projected_columns; + projected_columns.push_back(make_table_column(0, "id", std::make_shared())); + set_name_identifiers(&projected_columns); + + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + bool predicate_executed = false; + auto fake_state = std::make_shared(); + fake_state->aggregate_count = 3; + FakeTableReader reader(file_schema, fake_state); + ASSERT_TRUE(reader.init({ + .projected_columns = projected_columns, + .conjuncts = {prepared_conjunct( + &state, + std::make_shared( + &predicate_executed))}, + .format = FileFormat::PARQUET, + .scan_params = nullptr, + .io_ctx = nullptr, + .runtime_state = &state, + .scanner_profile = nullptr, + .push_down_agg_type = TPushAggOp::type::COUNT, + }) + .ok()); + + SplitReadOptions split_options; + split_options.current_range.__set_path("fake-table-reader-input"); + ASSERT_TRUE(reader.prepare_split(split_options).ok()); + + Block block = build_table_block(projected_columns); + bool eos = false; + ASSERT_TRUE(reader.get_block(&block, &eos).ok()); + // The slotless predicate cannot become a TableFilter or a file-reader conjunct, but its + // presence still prevents the fake aggregate count (3) from replacing the two physical rows. + ASSERT_NE(fake_state->last_request, nullptr); + EXPECT_TRUE(fake_state->last_request->conjuncts.empty()); + EXPECT_EQ(block.rows(), 2); + EXPECT_TRUE(predicate_executed); + ASSERT_TRUE(reader.close().ok()); +} + TEST(TableReaderTest, AbortSplitClearsReaderAfterIgnorableNotFound) { std::vector file_schema; file_schema.push_back(make_file_column(0, "id", std::make_shared()));