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


##########
be/src/exprs/function/like.cpp:
##########
@@ -1107,6 +1114,68 @@ Status FunctionRegexpLike::open(FunctionContext* context,
     return Status::OK();
 }
 
+// R8 (unity build): file-scope helpers use a namespace private to this file.
+namespace like_gram_index_detail {
+
+// A declined index leaves the rows to the predicate; the scan decides every 
other index status.
+Status dispatch_query(bool is_like, const std::string& pattern, 
segment_v2::IndexIterator* iter,
+                      const IndexFieldNameAndTypePair& data_type_with_name, 
uint32_t num_rows,
+                      segment_v2::InvertedIndexResultBitmap* bitmap_result) {
+    segment_v2::InvertedIndexParam param;
+    param.column_name = data_type_with_name.first;
+    param.column_type = data_type_with_name.second;
+    param.query_value = Field::create_field<TYPE_STRING>(pattern);
+    param.query_type = is_like ? 
segment_v2::InvertedIndexQueryType::LIKE_GRAM_QUERY
+                               : 
segment_v2::InvertedIndexQueryType::REGEXP_GRAM_QUERY;
+    param.num_rows = num_rows;
+    param.roaring = std::make_shared<roaring::Roaring>();
+
+    Status query_status = iter->read_from_index(&param);
+    if (!query_status.ok()) {
+        if (query_status.is<ErrorCode::INVERTED_INDEX_EVALUATE_SKIPPED>() ||
+            query_status.is<ErrorCode::INVERTED_INDEX_NOT_SUPPORTED>()) {
+            return Status::OK();
+        }
+        return query_status;
+    }
+
+    segment_v2::InvertedIndexResultBitmap result(param.roaring, nullptr);
+    result.set_approximate(true);
+    *bitmap_result = result;
+    return Status::OK();
+}
+
+} // namespace like_gram_index_detail
+
+Status FunctionLikeBase::evaluate_gram_index(
+        GramCompileKind kind, const ColumnsWithTypeAndName& arguments,
+        const std::vector<IndexFieldNameAndTypePair>& data_type_with_names,
+        std::vector<segment_v2::IndexIterator*> iterators, uint32_t num_rows,
+        segment_v2::InvertedIndexResultBitmap& bitmap_result) const {
+    const bool is_like = (kind == GramCompileKind::LIKE);
+    if (!config::enable_gram_index_regexp) {
+        return Status::OK();
+    }
+    // VExpr binds whatever children it finds -- one entry per indexed column, 
one per literal
+    // -- so the shape reaching here is not guaranteed. Answer only the shape 
this compiler
+    // understands: a single indexed column and a single constant pattern. 
`LIKE ... ESCAPE`
+    // arrives with a second literal and would otherwise be compiled with the 
default escaping,
+    // which is not what the query asked for.
+    if (iterators.size() != 1 || data_type_with_names.size() != 1 || 
arguments.size() != 1 ||

Review Comment:
   [P1] Preserve child roles before using this bitmap. `VExpr` separates 
indexed children into `iterators` and literals into `arguments`, so this shape 
also accepts a reversed predicate such as `'abc' LIKE pattern_col` (and 
REGEXP/RLIKE). Scan normalization leaves that form as a common expression, then 
this code treats the constant LHS as the pattern and queries the RHS column. A 
row with `pattern_col = 'a%'` satisfies the scalar predicate, but the gram 
query for `abc` can exclude it before recheck. The same role loss can compile 
an ESCAPE literal when child 1 is a dynamic unindexed pattern. Please only push 
when the indexed slot is child 0 and the constant pattern is child 1 (or carry 
child positions through the API), and add parity coverage for both shapes.



##########
be/src/storage/segment/segment_iterator.cpp:
##########
@@ -1365,6 +1374,27 @@ Status SegmentIterator::_apply_index_expr() {
     return Status::OK();
 }
 
+void SegmentIterator::_apply_approx_index_result(VExprContext* expr_ctx) {
+    // An approximate result is a superset of candidates: rows outside the 
bitmap certainly do
+    // not match, while rows inside it may not match. So this only narrows 
_row_bitmap down to
+    // the candidate set and never records the expression in consumed_by_index 
-- it must stay in
+    // _common_expr_ctxs_push_down so that _execute_common_expr re-verifies 
each candidate row.
+    const auto* approx =
+            
expr_ctx->get_index_context()->get_approx_index_result_for_expr(expr_ctx->root().get());
+    if (approx == nullptr || approx->get_data_bitmap() == nullptr) {
+        // InvertedIndexResultBitmap allows a null data bitmap (the 
default-constructed "no
+        // result" shape). In theory the approximate map only ever holds 
results with
+        // is_empty() == false, but dereferencing a null pointer here would be 
a segfault, which
+        // costs far more than one extra null check.
+        return;
+    }
+    const uint64_t before = _row_bitmap.cardinality();
+    _row_bitmap &= *approx->get_data_bitmap();
+    const uint64_t after = _row_bitmap.cardinality();
+    _opts.stats->gram_index_candidate_rows += static_cast<int64_t>(after);

Review Comment:
   [P2] Count final candidate rows once per segment instead of summing every 
intermediate intersection. With two approximate conjuncts that narrow 100 rows 
to 80 and then 10, only 10 rows reach scalar re-verification, but this adds 80 
+ 10 and reports 90; a later exact or ANN intersection can overstate it 
further, even when no row is rechecked. That contradicts the counter's contract 
as rows still needing expression re-verification and can exceed scan input. 
Please record the final index-stage bitmap cardinality once when any gram 
result was applied, and add a multi-conjunct/interleaving counter test.



-- 
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