Gabriel39 commented on code in PR #69646:
URL: https://github.com/apache/doris/pull/69646#discussion_r4384276772
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/TopnFilterPushDownVisitor.java:
##########
@@ -92,6 +92,11 @@ public Boolean visit(Plan plan, PushDownContext ctx) {
@Override
public Boolean visitPhysicalProject(
PhysicalProject<? extends Plan> project, PushDownContext ctx) {
+ // TopN can skip or reorder input before this project. Crossing
assert_true(k < 3), for
+ // example, could suppress errors in pruned row groups instead of
preserving evaluation.
+ if
(project.getProjects().stream().anyMatch(Expression::containsVolatileOrNoneMovableExpression))
{
Review Comment:
Fixed in 495b081d65. TopN pushdown now reuses the existing date-arithmetic
safety classification, including nested add/subtract and date rounding, while
retaining literal-zero identities. The reported secondary-key projection test
failed before the fix and now passes; the safe identity control still creates a
TopN filter.
Added `topn_evaluation_safety` SQL coverage with an overflowing high-key row
followed by multiple batches of low-key rows, checking both the absent TopN
target and the overflow error. Full SQL execution is pending CI; all 33 tests
across the four related FE suites passed, and the Groovy regression compiled
successfully.
##########
be/src/format_v2/parquet/parquet_scan.cpp:
##########
@@ -1409,21 +1414,147 @@ std::vector<format::LocalColumnIndex>
ParquetScanScheduler::adaptive_predicate_p
return result;
}
+namespace {
+
+struct TopNOrderingColumn {
+ const VTopNPred* predicate;
+ const ParquetColumnSchema* schema;
+};
+
+std::optional<TopNOrderingColumn> find_topn_ordering_column(
+ const std::vector<std::unique_ptr<ParquetColumnSchema>>& file_schema,
+ const format::FileScanRequest& request) {
+ const size_t safe_count =
+ std::min(request.metadata_pruning_safe_conjunct_count,
request.conjuncts.size());
+ const VTopNPred* topn = nullptr;
+ for (size_t idx = 0; idx < safe_count; ++idx) {
+ const auto& root = request.conjuncts[idx]->root();
+ if (root->is_topn_filter() && root->can_evaluate_zonemap_filter()) {
+ topn = assert_cast<const VTopNPred*>(root.get());
+ break;
+ }
+ }
+ if (topn == nullptr) {
+ return std::nullopt;
+ }
+ const auto* slot = assert_cast<const VSlotRef*>(topn->get_child(0).get());
+ const auto position = std::ranges::find_if(request.local_positions,
[&](const auto& entry) {
+ return entry.second.value() == slot->column_id();
+ });
+ if (position == request.local_positions.end()) {
+ return std::nullopt;
+ }
+ const auto column = std::ranges::find_if(file_schema, [&](const auto&
schema) {
+ return schema->local_id == position->first.value();
+ });
+ if (column == file_schema.end() || (*column)->kind !=
ParquetColumnSchemaKind::PRIMITIVE ||
+ (*column)->max_repetition_level != 0 ||
+ !expr_zonemap::data_types_compatible((*column)->type,
slot->data_type())) {
+ return std::nullopt;
+ }
+ return TopNOrderingColumn {.predicate = topn, .schema = column->get()};
+}
+
+} // namespace
+
+void ParquetScanScheduler::prepare_topn_row_group_order(
+ const ParquetFileContext& file_context,
+ const std::vector<std::unique_ptr<ParquetColumnSchema>>& file_schema,
+ const format::FileScanRequest& request) {
+ _row_group_order_prepared = true;
+ const auto binding = find_topn_ordering_column(file_schema, request);
+ if (!binding.has_value()) {
+ return;
+ }
+ const auto* topn = binding->predicate;
+ const auto& schema = *binding->schema;
+ const size_t safe_count =
+ std::min(request.metadata_pruning_safe_conjunct_count,
request.conjuncts.size());
+ const auto& metadata = file_context.native_metadata->to_thrift();
+ _topn_footer_recheck = std::ranges::any_of(_scan_plan->row_groups,
[&](const auto& plan) {
+ const auto& group = metadata.row_groups[plan.row_group_id];
+ DORIS_CHECK(schema.leaf_column_id >= 0 && schema.leaf_column_id <
group.columns.size());
+ const auto& chunk = group.columns[schema.leaf_column_id];
+ return chunk.__isset.meta_data && chunk.meta_data.__isset.statistics;
+ });
+ // Files without statistics cannot benefit from repeated footer checks.
Row-ID fetches promise
+ // increasing positions, and an unsafe table expression must continue
observing file order.
+ if (!_topn_footer_recheck || request.row_ids.has_value() ||
!request.allow_row_reordering ||
Review Comment:
Fixed in 495b081d65. Both hash-join and nested-loop-join visitors now check
the join's complete expression list (hash, other, and mark conjuncts) before
traversing either child. This fences ON predicates that scan-level request
flags cannot observe.
The ON-assertion hash-join and nested-loop-join planner regressions both
failed before the fix and now pass; safe-join controls retain TopN filtering.
Added SQL error/EXPLAIN coverage with a high-key failing row followed by
low-key batches. All 33 related FE tests passed; the full SQL regression is
pending CI.
--
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]
