github-actions[bot] commented on code in PR #65369:
URL: https://github.com/apache/doris/pull/65369#discussion_r3549122001
##########
regression-test/suites/external_table_p0/hive/test_parquet_lazy_mat_profile.groovy:
##########
@@ -231,6 +231,18 @@ suite("test_parquet_lazy_mat_profile", "p0,external") {
return extractProfileBlockMetrics(profileText, "ParquetReader")
}
+ def q8 = {
Review Comment:
This new case does not actually prove the lazy/profile path changed by the
PR. It projects only `id`, which is also the predicate column, so no
non-predicate column goes through lazy materialization, and unlike the rest of
this profile suite it never retrieves the tokenized `ParquetReader` profile or
checks counters such as lazy-filtered rows, raw rows, or selected-reader rows.
A scan that ignores the new round-by-round/lazy behavior would still return the
same seven ids. Please make this assert the intended profile behavior, or move
this to a result-only test where it is not presented as lazy
materialization/profile coverage.
##########
be/src/format_v2/parquet/parquet_scan.cpp:
##########
@@ -597,42 +741,393 @@ Status
ParquetScanScheduler::skip_current_row_group_rows(int64_t rows) {
return Status::OK();
}
+namespace {
+
+struct PredicateConjunctSchedule {
+ std::map<size_t, VExprContextSPtrs> single_column_conjuncts;
+ VExprContextSPtrs remaining_conjuncts;
+};
+
+PredicateConjunctSchedule build_predicate_conjunct_schedule(
+ const format::FileScanRequest& request) {
+ std::unordered_set<size_t> predicate_block_positions;
+ predicate_block_positions.reserve(request.predicate_columns.size());
+ for (const auto& col : request.predicate_columns) {
+ const auto position_it = request.local_positions.find(col.column_id());
+ DORIS_CHECK(position_it != request.local_positions.end());
+ predicate_block_positions.insert(position_it->second.value());
+ }
+
+ PredicateConjunctSchedule schedule;
+ for (const auto& conjunct : request.conjuncts) {
+ DORIS_CHECK(conjunct != nullptr);
+ std::set<int> referenced_positions;
+ conjunct->root()->collect_slot_column_ids(referenced_positions);
+ if (referenced_positions.size() != 1) {
+ schedule.remaining_conjuncts.push_back(conjunct);
+ continue;
+ }
+ const auto block_position =
static_cast<size_t>(*referenced_positions.begin());
+ if (!predicate_block_positions.contains(block_position)) {
+ schedule.remaining_conjuncts.push_back(conjunct);
+ continue;
+ }
+ schedule.single_column_conjuncts[block_position].push_back(conjunct);
+ }
+ return schedule;
+}
+
+bool can_evaluate_all_with_dictionary(const VExprContextSPtrs& conjuncts) {
+ if (conjuncts.empty()) {
+ return false;
+ }
+ return std::ranges::all_of(conjuncts, [](const auto& conjunct) {
+ return conjunct != nullptr && conjunct->root() != nullptr &&
+ conjunct->root()->can_evaluate_dictionary_filter();
+ });
+}
+
+uint16_t count_selected_rows(const IColumn::Filter& filter) {
+ uint16_t selected_rows = 0;
+ for (const auto value : filter) {
+ selected_rows += value != 0;
+ }
+ return selected_rows;
+}
+
+Status filter_read_predicate_columns(Block* file_block, const
std::vector<uint32_t>& positions,
+ const IColumn::Filter& compact_filter) {
+ if (positions.empty()) {
+ return Status::OK();
+ }
+ RETURN_IF_CATCH_EXCEPTION(Block::filter_block_internal(file_block,
positions, compact_filter));
+ return Status::OK();
+}
+
+IColumn::Filter build_dictionary_entry_filter(size_t block_position,
+ const ParquetColumnSchema&
column_schema,
+ const VExprContextSPtrs&
conjuncts,
+ const ParquetDictionaryWords&
dict_words) {
+ auto fields = dictionary_fields_from_words(dict_words);
+ IColumn::Filter dictionary_filter(fields.size(), 1);
+ DictionaryEvalContext ctx;
+ auto& slot = ctx.slots
+ .emplace(static_cast<int>(block_position),
+ DictionaryEvalContext::SlotDictionary {
+ .data_type = column_schema.type,
.values = {}})
+ .first->second;
+ slot.values.reserve(1);
+
+ for (size_t dict_idx = 0; dict_idx < fields.size(); ++dict_idx) {
+ slot.values.clear();
+ slot.values.push_back(fields[dict_idx]);
+ dictionary_filter[dict_idx] =
VExprContext::evaluate_dictionary_filter(conjuncts, ctx) ==
+
ZoneMapFilterResult::kNoMatch
+ ? 0
+ : 1;
+ }
+ return dictionary_filter;
+}
+
+} // namespace
+
+Status ParquetScanScheduler::prepare_current_dictionary_filters(
+ ParquetFileContext& file_context,
+ const std::vector<std::unique_ptr<ParquetColumnSchema>>& file_schema,
+ const format::FileScanRequest& request, int row_group_idx,
+ const ::parquet::RowGroupMetaData& row_group_metadata) {
+ _current_dictionary_filters.clear();
+ if (request.conjuncts.empty()) {
+ return Status::OK();
+ }
+ const auto schedule = build_predicate_conjunct_schedule(request);
+ if (schedule.single_column_conjuncts.empty()) {
+ return Status::OK();
+ }
+
+ SCOPED_TIMER(_scan_profile.dict_filter_rewrite_time);
+ for (const auto& col : request.predicate_columns) {
+ const auto local_id = col.local_id();
+ if (local_id < 0 || local_id >=
static_cast<int32_t>(file_schema.size())) {
+ continue;
+ }
+ const auto position_it = request.local_positions.find(col.column_id());
+ DORIS_CHECK(position_it != request.local_positions.end());
Review Comment:
`COUNTER_UPDATE` dereferences the counter pointer, but these new dictionary
counters are null whenever the v2 Parquet reader is constructed without a
`RuntimeProfile` (`ParquetProfile::init()` just returns in that case and
`open()` still passes the empty `scan_profile()` to the scheduler). Existing
unit helpers still create profile-less readers, so a dictionary-filtered
request can now crash as soon as `prepare_current_dictionary_filters()` reaches
this update; `RowsFilteredByDictFilter` has the same issue later in
`read_filter_columns()`. Please guard these new counter updates like the
surrounding scan counters, or ensure the scheduler never receives a profile
with null dictionary counters.
--
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]
