github-actions[bot] commented on code in PR #68788:
URL: https://github.com/apache/doris/pull/68788#discussion_r4255497184
##########
be/src/storage/segment/segment_iterator.cpp:
##########
@@ -3159,18 +3200,28 @@ Status SegmentIterator::_next_batch_internal(Block*
block) {
if (_count_emit_shortcut) {
return _emit_count_shortcut_batch(block);
}
+ if (_opts.read_limit > 0 && _rows_returned >= _opts.read_limit) {
Review Comment:
[P1] Keep LIMIT exhaustion from publishing a partial condition cache
This new EOF also reaches `next_batch()`'s unconditional condition-cache
insertion at lines 3098-3107, although the segment has not been fully
evaluated. With `enable_condition_cache=true`, `topn_filter_ratio=0`, and a DUP
table containing an overlapping multi-segment rowset, `WHERE abs(v) > 0 ORDER
BY k LIMIT 1` enables storage TopN. `VMergeIterator::_next_batch()` advances
each consumed child block and requests another batch before the scanner can
stop, so this branch publishes the cache. Only rows retained by LIMIT were
marked after `_apply_read_limit_to_selected_rows()`; other 2048-row granules
remain false. A later query with the same predicate and a larger/no LIMIT
reuses the predicate digest and discards those granules, silently omitting
matching rows. Skip cache publication when EOF is caused by the read budget, or
conservatively mark unvisited/discarded granules as possibly matching.
##########
be/src/exec/operator/olap_scan_operator.cpp:
##########
@@ -436,12 +436,66 @@ Status OlapScanLocalState::_init_profile() {
return Status::OK();
}
+static bool contains_expr_node_type(const VExprSPtr& expr, TExprNodeType::type
node_type) {
+ DORIS_CHECK(expr != nullptr);
+ if (expr->node_type() == node_type) {
+ return true;
+ }
+ if (expr->is_rf_wrapper() && contains_expr_node_type(expr->get_impl(),
node_type)) {
+ return true;
+ }
+ return std::ranges::any_of(expr->children(), [node_type](const auto&
child) {
+ return contains_expr_node_type(child, node_type);
+ });
+}
+
+static Status validate_residual_scan_conjuncts(RuntimeState* state,
+ TPushAggOp::type
push_down_agg_type,
+ const VExprContextSPtrs&
conjuncts) {
+ for (const auto& conjunct : conjuncts) {
+ const auto& root = conjunct->root();
+ if (contains_expr_node_type(root, TExprNodeType::SEARCH_EXPR)) {
+ return Status::InvalidArgument(
+ "SEARCH expression remains as a residual scan predicate. A
valid search() "
+ "must bind at least one indexed field and be evaluated in
SegmentIterator, "
+ "which takes such expressions only when the table is not
merged on read "
+ "(DUPLICATE, or UNIQUE with merge-on-write).
enable_segment_limit_pushdown "
+ "only controls SegmentIterator LIMIT pushdown and cannot
make residual "
+ "SEARCH executable.");
+ }
+ if (!state->query_options().enable_match_without_inverted_index &&
+ contains_expr_node_type(root, TExprNodeType::MATCH_PRED)) {
+ return Status::InvalidArgument(
+ "MATCH expression remains as a residual scan predicate and
would fall back to "
+ "a disabled slow path because
enable_match_without_inverted_index is false. "
+ "SegmentIterator evaluates MATCH only when the table is
not merged on read "
+ "(DUPLICATE, or UNIQUE with merge-on-write).
enable_segment_limit_pushdown "
+ "only controls SegmentIterator LIMIT pushdown and cannot
make residual MATCH "
+ "executable. Set enable_match_without_inverted_index=true
to allow slow MATCH "
+ "execution.");
+ }
+ }
+
+ if (push_down_agg_type == TPushAggOp::COUNT_ON_INDEX &&
!conjuncts.empty()) {
Review Comment:
[P2] Fall back for COUNT queries with slotless volatile predicates
On a DUP/MOW table with an inverted index, `SELECT count(*) FROM t WHERE
random() < 2` reaches this guard with the default
`enable_count_on_index_pushdown=true`:
```text
Aggregate(count(*))
Filter(random() < 2)
Scan(indexed_table)
```
`AggregateStrategies.COUNT_ON_INDEX_WITHOUT_PROJECT` accepts this because
COUNT(*) has no input slots, and `visitPhysicalFilter()` puts the predicate on
the COUNT_ON_INDEX scan. A single `random()` is neither folded nor extracted
into a separate Project. In BE it is nonconstant and has no slot, so
normalization retains it in `_conjuncts` instead of pushing it into
SegmentIterator. The previous COUNT_ON_INDEX path emitted one row per source
record and the scanner safely evaluated this predicate before the upper COUNT;
since random() returns values in [0,1), this example returns the full row
count. The new unconditional rejection turns that valid query into
InvalidArgument. Preserve safe row-shaped residual evaluation or fall back to
ordinary scanning/counting before enabling this pushdown.
--
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]
