github-actions[bot] commented on code in PR #66412:
URL: https://github.com/apache/doris/pull/66412#discussion_r3710207447


##########
be/src/format_v2/parquet/parquet_scan.cpp:
##########
@@ -2421,6 +3207,230 @@ Status 
ParquetScanScheduler::read_filter_columns(int64_t batch_rows,
         return status;
     };
 
+    auto execute_raw_dnf = [&](const detail::PredicateConjunctStage& stage,
+                               bool* applied) -> Status {
+        DORIS_CHECK(applied != nullptr);
+        *applied = false;
+        if (stage.raw_dnf_columns.empty() || *selected_rows == 0) {
+            return Status::OK();
+        }
+        auto readers_it = 
_current_raw_dnf_readers.find(stage.expression.get());
+        if (readers_it == _current_raw_dnf_readers.end()) {
+            return Status::OK();
+        }
+        size_t dictionary_columns = 0;
+        size_t raw_value_columns = 0;
+        size_t fixed_width_columns = 0;
+        const uint16_t selected_rows_before = *selected_rows;
+        const size_t branch_count = 
stage.raw_dnf_columns.front().branch_conjuncts.size();
+        _raw_disjunction_filter_scratch.resize(selected_rows_before);
+        // The scratch buffer is retained across batches; every DNF 
intersection must start from
+        // the full branch mask rather than a previous batch's survivors.
+        std::ranges::fill(_raw_disjunction_filter_scratch,
+                          static_cast<uint8_t>((1U << branch_count) - 1));
+        for (auto& column : readers_it->second) {
+            bool used_filter = false;
+            if (column.dictionary_filter.has_value()) {
+                uint16_t survivors = 0;
+                RETURN_IF_ERROR(column.reader->select_with_dictionary_filter(
+                        *selection, selected_rows_before, batch_rows, 
*column.dictionary_filter,
+                        nullptr, &_raw_disjunction_branch_filter_scratch, 
&survivors, &used_filter,
+                        true));
+                if (used_filter) {
+                    ++dictionary_columns;
+                }
+            } else {
+                DirectPredicateExecutionKind execution_kind = 
DirectPredicateExecutionKind::NONE;
+                const VExprSPtrs conjuncts {column.expression};
+                RETURN_IF_ERROR(column.reader->select_with_fixed_width_filter(
+                        *selection, selected_rows_before, batch_rows, 
conjuncts,
+                        cast_set<int>(column.position), nullptr,
+                        &_raw_disjunction_branch_filter_scratch, &used_filter, 
&execution_kind));
+                if (used_filter &&
+                    (execution_kind == DirectPredicateExecutionKind::RAW_FIXED 
||
+                     execution_kind == 
DirectPredicateExecutionKind::RAW_BINARY ||
+                     execution_kind == 
DirectPredicateExecutionKind::CONVERTED_FIXED)) {
+                    ++raw_value_columns;
+                }
+                if (used_filter &&
+                    (execution_kind == DirectPredicateExecutionKind::RAW_FIXED 
||
+                     execution_kind == 
DirectPredicateExecutionKind::CONVERTED_FIXED)) {
+                    ++fixed_width_columns;
+                }
+            }
+            if (!used_filter) {

Review Comment:
   [P1] Fall back for mixed encodings in the DNF executor
   
   This is a second, distinct mixed-encoding failure from the simple-OR branch 
below. Rectangular-DNF preflight classifies every non-fully-dictionary chunk 
from only the expression/type and NULL-density gates, so a valid 
dictionary-to-PLAIN chunk at 50%-<90% NULL installs these auxiliary readers. 
`ScalarColumnReader::read_fixed_width_filter()` then checks the complete 
advertised encoding set, sees `RLE_DICTIONARY` beside PLAIN, and safely returns 
`used_filter=false` before consuming—but this path converts that refusal into 
`InternalError`. The ordinary residual reader supports this transition, and all 
DNF readers are auxiliary, so please include the complete per-Column-Chunk 
encoding capability in DNF preflight and disable the whole mask stage while 
ordinary cursors are untouched. Fixing only the `branch.reader == nullptr` 
logic in the existing simple-OR thread will not cover this executor.



##########
be/src/format_v2/parquet/parquet_profile.cpp:
##########
@@ -211,6 +211,12 @@ void ParquetProfile::init(RuntimeProfile* profile) {
             profile, "DictionaryPredicateDirectRows", TUnit::UNIT, 
parquet_profile, 1);
     dictionary_predicate_projected_rows = ADD_CHILD_COUNTER_WITH_LEVEL(
             profile, "DictionaryPredicateProjectedRows", TUnit::UNIT, 
parquet_profile, 1);
+    multi_column_or_raw_filter_batches = ADD_CHILD_COUNTER_WITH_LEVEL(
+            profile, "MultiColumnOrRawFilterBatches", TUnit::UNIT, 
parquet_profile, 1);
+    multi_column_or_raw_filter_fallback_batches = ADD_CHILD_COUNTER_WITH_LEVEL(

Review Comment:
   [P2] Count policy-gated raw-filter fallbacks
   
   This counter only advances when a reader was already created and then 
returns `used_filter=false`. The normal path for the new policy is different: 
file/Row-Group NULL-density, projection, encoding, and dictionary-survival 
gates insert the stage into `_disabled_raw_disjunctions` and run the residual 
expression without incrementing this or any attempted/disabled-by-reason 
counter. A profile with raw batches = 0 and fallback batches = 0 therefore 
cannot distinguish an ineligible request from a recognized raw OR/DNF that the 
policy rejected, including the cases exercised by the new 
`KeepsLegacy*OutsideProfitableRange` tests. Please publish the decision where 
those gates fire (ideally attempted/enabled and disabled-by-reason counters), 
or count residual batches for disabled recognized stages and cover that meaning 
in the tests.



##########
be/src/format_v2/parquet/parquet_scan.cpp:
##########
@@ -1834,6 +2293,297 @@ Status 
ParquetScanScheduler::prepare_current_dictionary_filters(
         _current_predicate_columns.emplace(local_id, std::move(column_reader));
         update_counter_if_not_null(_scan_profile.dict_filter_columns, 1);
     }
+
+    std::unordered_map<size_t, size_t> remaining_position_occurrences;
+    for (const auto& remaining_stage : schedule.remaining_stages) {
+        for (const size_t position : remaining_stage.required_positions) {
+            ++remaining_position_occurrences[position];
+        }
+    }
+    std::unordered_set<size_t> delete_positions;
+    for (const auto& conjunct : request.delete_conjuncts) {
+        std::set<int> positions;
+        conjunct->root()->collect_slot_column_ids(positions);
+        for (const int position : positions) {
+            if (position >= 0) {
+                delete_positions.insert(cast_set<size_t>(position));
+            }
+        }
+    }
+
+    for (const auto& stage : schedule.remaining_stages) {
+        if (stage.raw_disjunction_branches.empty() ||
+            _disabled_raw_disjunctions.contains(stage.expression.get())) {
+            continue;
+        }
+        std::vector<RawDisjunctionBranchReader> branch_readers;
+        branch_readers.reserve(stage.raw_disjunction_branches.size());
+        bool usable = true;
+        for (const auto& branch : stage.raw_disjunction_branches) {
+            const auto predicate_index_it =
+                    
_predicate_indices_by_position_scratch.find(branch.position);
+            if (predicate_index_it == 
_predicate_indices_by_position_scratch.end()) {
+                usable = false;
+                break;
+            }
+            const auto& column = 
request.predicate_columns[predicate_index_it->second];
+            const auto local_id = column.column_id();
+            if (!local_id.is_valid() ||
+                local_id.value() >= static_cast<int32_t>(file_schema.size())) {
+                usable = false;
+                break;
+            }
+            const auto& column_schema = file_schema[local_id.value()];
+            DORIS_CHECK(column_schema != nullptr);
+            if (column_schema->leaf_column_id < 0 ||
+                column_schema->leaf_column_id >=
+                        static_cast<int>(row_group_metadata.columns.size())) {
+                usable = false;
+                break;
+            }
+            const auto& column_chunk = 
row_group_metadata.columns[column_schema->leaf_column_id];
+            if (!column_chunk.__isset.meta_data) {
+                usable = false;
+                break;
+            }
+            const int expression_column_id = cast_set<int>(branch.position);
+            const bool raw_eligible = 
branch.expression->can_execute_on_raw_fixed_values(
+                                              column_schema->type, 
expression_column_id) ||
+                                      
branch.expression->can_execute_on_raw_binary_values(
+                                              column_schema->type, 
expression_column_id) ||
+                                      
branch.expression->can_execute_on_null_map(
+                                              column_schema->type, 
expression_column_id);
+            if (!raw_eligible) {
+                usable = false;
+                break;
+            }
+
+            bool dictionary_eligible = false;
+            if (!branch.expression->raw_predicate_result_for_null()) {
+                dictionary_eligible = supports_row_level_dictionary_filter(
+                                              *column_schema, 
column_chunk.meta_data) &&
+                                      
(branch.expression->can_execute_on_raw_fixed_values(
+                                               column_schema->type, 
expression_column_id) ||
+                                       
branch.expression->can_execute_on_raw_binary_values(
+                                               column_schema->type, 
expression_column_id));
+            }
+            const bool projected = !request.is_predicate_only(local_id);
+            const int minimum_null_denominator = dictionary_eligible && 
!projected ? 10 : 2;
+            if ((!dictionary_eligible && projected) ||
+                !has_null_fraction_at_least(column_chunk.meta_data, 1, 
minimum_null_denominator)) {
+                // Branch readers pay off only after avoiding enough nullable 
materialization. The
+                // paired matrix establishes conservative 10% dictionary and 
50% PLAIN thresholds;
+                // projected PLAIN branches retain the residual path because 
they decode twice.
+                usable = false;
+                break;
+            }
+            const bool use_predicate_reader =
+                    request.is_predicate_only(local_id) &&
+                    remaining_position_occurrences[branch.position] == 1 &&
+                    !delete_positions.contains(branch.position) &&
+                    
!schedule.single_column_conjuncts.contains(branch.position);
+            if (dictionary_eligible) {
+                
update_counter_if_not_null(_scan_profile.dict_filter_candidate_columns, 1);
+            }
+
+            std::unique_ptr<ParquetColumnReader> column_reader;
+            if (!use_predicate_reader || dictionary_eligible) {
+                RETURN_IF_ERROR(NativeColumnReader::create(
+                        *column_schema, &column, file_context.native_file,

Review Comment:
   [P1] Build raw execution readers on the Row-Group I/O wrapper
   
   This object is used later to scan data pages, but it permanently captures 
`file_context.native_file` while `prepare_current_dictionary_filters()` still 
runs before `set_native_random_access_ranges()`; the DNF construction below has 
the same ordering. For a projected-only OR request 
(`non_predicate_positions.empty()`), the scheduler can then enable MergeRange 
and build the ordinary survivor readers from `native_data_file()`, while the 
actual raw branch scans continue through these base-file auxiliaries. DNF 
similarly scans through its auxiliaries and then skips duplicate ordinary 
readers. Because active MergeRange also suppresses the early FileCache 
predicate prefetch, the new hot path gets neither the shared ranges nor that 
warm-up and restores one independent remote stream per mask/branch before 
rereading or skipping the ordinary readers. Please make duplicate raw-reader 
layouts part of the Row-Group I/O decision: preferably eliminate the duplicate 
cursor/consume the ordinary rea
 der once, or otherwise disable MergeRange and retain explicit prefetch. Two 
independent same-leaf readers should not simply be rebound to the wrapper's 
forward-consumed range cache.



##########
be/src/format_v2/parquet/parquet_scan.cpp:
##########
@@ -2421,6 +3207,230 @@ Status 
ParquetScanScheduler::read_filter_columns(int64_t batch_rows,
         return status;
     };
 
+    auto execute_raw_dnf = [&](const detail::PredicateConjunctStage& stage,
+                               bool* applied) -> Status {
+        DORIS_CHECK(applied != nullptr);
+        *applied = false;
+        if (stage.raw_dnf_columns.empty() || *selected_rows == 0) {
+            return Status::OK();
+        }
+        auto readers_it = 
_current_raw_dnf_readers.find(stage.expression.get());
+        if (readers_it == _current_raw_dnf_readers.end()) {
+            return Status::OK();
+        }
+        size_t dictionary_columns = 0;
+        size_t raw_value_columns = 0;
+        size_t fixed_width_columns = 0;
+        const uint16_t selected_rows_before = *selected_rows;
+        const size_t branch_count = 
stage.raw_dnf_columns.front().branch_conjuncts.size();
+        _raw_disjunction_filter_scratch.resize(selected_rows_before);
+        // The scratch buffer is retained across batches; every DNF 
intersection must start from
+        // the full branch mask rather than a previous batch's survivors.
+        std::ranges::fill(_raw_disjunction_filter_scratch,
+                          static_cast<uint8_t>((1U << branch_count) - 1));
+        for (auto& column : readers_it->second) {
+            bool used_filter = false;
+            if (column.dictionary_filter.has_value()) {
+                uint16_t survivors = 0;
+                RETURN_IF_ERROR(column.reader->select_with_dictionary_filter(
+                        *selection, selected_rows_before, batch_rows, 
*column.dictionary_filter,
+                        nullptr, &_raw_disjunction_branch_filter_scratch, 
&survivors, &used_filter,
+                        true));
+                if (used_filter) {
+                    ++dictionary_columns;
+                }
+            } else {
+                DirectPredicateExecutionKind execution_kind = 
DirectPredicateExecutionKind::NONE;
+                const VExprSPtrs conjuncts {column.expression};
+                RETURN_IF_ERROR(column.reader->select_with_fixed_width_filter(
+                        *selection, selected_rows_before, batch_rows, 
conjuncts,
+                        cast_set<int>(column.position), nullptr,
+                        &_raw_disjunction_branch_filter_scratch, &used_filter, 
&execution_kind));
+                if (used_filter &&
+                    (execution_kind == DirectPredicateExecutionKind::RAW_FIXED 
||
+                     execution_kind == 
DirectPredicateExecutionKind::RAW_BINARY ||
+                     execution_kind == 
DirectPredicateExecutionKind::CONVERTED_FIXED)) {
+                    ++raw_value_columns;
+                }
+                if (used_filter &&
+                    (execution_kind == DirectPredicateExecutionKind::RAW_FIXED 
||
+                     execution_kind == 
DirectPredicateExecutionKind::CONVERTED_FIXED)) {
+                    ++fixed_width_columns;
+                }
+            }
+            if (!used_filter) {
+                return Status::InternalError(
+                        "Validated multi-column DNF column {} could not 
execute directly",
+                        column.position);
+            }
+            DORIS_CHECK_EQ(_raw_disjunction_branch_filter_scratch.size(), 
selected_rows_before);
+            for (size_t row = 0; row < selected_rows_before; ++row) {
+                _raw_disjunction_filter_scratch[row] &= 
_raw_disjunction_branch_filter_scratch[row];
+            }
+        }
+        const uint16_t new_selected_rows = 
count_selected_rows(_raw_disjunction_filter_scratch);
+        if (conjunct_filtered_rows != nullptr) {
+            *conjunct_filtered_rows += 
static_cast<int64_t>(selected_rows_before) -
+                                       static_cast<int64_t>(new_selected_rows);
+        }
+        if (new_selected_rows != selected_rows_before) {
+            predicate_columns_need_alignment = true;
+            *selected_rows = new_selected_rows == 0 ? 0
+                                                    : 
apply_compact_filter_to_selection(
+                                                              
_raw_disjunction_filter_scratch,
+                                                              selection, 
selected_rows_before);
+        }
+        for (const auto& column : readers_it->second) {
+            if (!materialized_positions.contains(column.position)) {
+                const auto predicate_reader_it = 
_current_predicate_columns.find(column.local_id);
+                DORIS_CHECK(predicate_reader_it != 
_current_predicate_columns.end());
+                RETURN_IF_ERROR(predicate_reader_it->second->skip(batch_rows));
+                materialized_positions.insert(column.position);
+                
read_column_positions.push_back(cast_set<uint32_t>(column.position));
+            }
+            auto placeholder = 
file_block->get_by_position(column.position).column->clone_empty();
+            placeholder->insert_many_defaults(*selected_rows);
+            file_block->replace_by_position(column.position, 
std::move(placeholder));
+            remember_column_selection(cast_set<uint32_t>(column.position));
+        }
+        
update_counter_if_not_null(_scan_profile.multi_column_or_raw_filter_batches, 1);
+        
update_counter_if_not_null(_scan_profile.multi_column_or_raw_filter_branches,
+                                   cast_set<int64_t>(branch_count));
+        
update_counter_if_not_null(_scan_profile.dictionary_predicate_direct_batches,
+                                   cast_set<int64_t>(dictionary_columns));
+        
update_counter_if_not_null(_scan_profile.dictionary_predicate_direct_rows,

Review Comment:
   [P2] Keep DNF direct-filter accounting in selected-row units
   
   Each DNF reader receives the current `selection` and returns one mask byte 
per `selected_rows_before`, so rows rejected by earlier predicate rounds are 
skipped rather than evaluated by these mask expressions. These updates instead 
charge `batch_rows` for every DNF column: an 8K batch narrowed to one row makes 
two mask columns report 16K direct rows instead of two, while the existing 
single-column and sibling raw-OR paths use `selected_rows_before`. This block 
also never adds `selected_rows_before - new_selected_rows` to 
`RowsFilteredByDictFilter` when every DNF column used a dictionary bitmap, 
although those same two paths publish that benefit. Please keep the direct rows 
in selected-input units, add the all-dictionary rejected-row attribution, and 
cover both with a DNF stage after a selective conjunct.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to