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


##########
be/src/olap/rowset/segment_v2/segment_iterator.cpp:
##########
@@ -814,6 +820,17 @@ bool 
SegmentIterator::_can_filter_by_preds_except_leafnode_of_andnode() {
     return true;
 }
 
+bool SegmentIterator::_check_apply_by_inverted_index(ColumnId col_id) {

Review Comment:
   warning: method '_check_apply_by_inverted_index' can be made static 
[readability-convert-member-functions-to-static]
   
   be/src/olap/rowset/segment_v2/segment_iterator.h:286:
   ```diff
   -     bool _check_apply_by_inverted_index(ColumnId col_id);
   +     static bool _check_apply_by_inverted_index(ColumnId col_id);
   ```
   



##########
be/src/vec/exprs/vcompound_pred.h:
##########
@@ -53,6 +53,56 @@
 
     const std::string& expr_name() const override { return _expr_name; }
 
+    bool is_all_ones(const roaring::Roaring& r) {
+        return r.contains(0);
+        for (roaring::RoaringSetBitForwardIterator i = r.begin(); i != 
r.end(); ++i) {
+            if (*i == 0) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    //   1. when meet 'or' conjunct: a or b, if b can apply index, return all 
rows, so b should not be extracted
+    //   2. when meet 'and' conjunct, function with column b can not apply 
inverted index
+    //      eg. a and hash(b)=1, if b can apply index, but hash(b)=1 is not 
for index, so b should not be extracted
+    //          but a and array_contains(b, 1), b can be applied inverted 
index, which b can be extracted
+    Status eval_inverted_index(

Review Comment:
   warning: method 'eval_inverted_index' can be made static 
[readability-convert-member-functions-to-static]
   
   ```suggestion
       static Status eval_inverted_index(
   ```
   
   be/src/vec/exprs/vcompound_pred.h:74:
   ```diff
   -             uint32_t num_rows, roaring::Roaring* bitmap) const override {
   +             uint32_t num_rows, roaring::Roaring* bitmap) override {
   ```
   



##########
be/src/vec/exprs/vcompound_pred.h:
##########
@@ -53,6 +53,56 @@ class VCompoundPred : public VectorizedFnCall {
 
     const std::string& expr_name() const override { return _expr_name; }
 
+    bool is_all_ones(const roaring::Roaring& r) {

Review Comment:
   warning: method 'is_all_ones' can be made static 
[readability-convert-member-functions-to-static]
   
   ```suggestion
       static bool is_all_ones(const roaring::Roaring& r) {
   ```
   



##########
be/src/vec/exprs/vexpr_context.h:
##########
@@ -69,6 +70,22 @@ class VExprContext {
         return _fn_contexts[i].get();
     }
 
+    // execute expr with inverted index which column a, b has inverted indexes
+    //  but some situation although column b has indexes, but apply index is 
not useful, we should
+    //  skip this expr, just do not apply index anymore.
+    /**
+     * @param name_with_types all columns with name and type in all 
_common_expr_ctxs_push_down see in SegmentIterator.h.
+     * @param inverted_indexs_iter columns which extracted from 
_common_expr_ctxs_push_down and has inverted index.
+     * @param num_rows number of rows in one segment.
+     * @param bitmap roaring bitmap to store the result. 0 is present filed by 
index.
+     * @return status not ok means execute failed.
+     */
+    [[nodiscard]] Status eval_inverted_indexs(
+            const std::unordered_map<ColumnId, 
std::pair<vectorized::NameAndTypePair,

Review Comment:
   warning: parameter 1 is const-qualified in the function declaration; 
const-qualification of parameters only has an effect in function definitions 
[readability-avoid-const-params-in-decls]
   
   ```suggestion
               std::unordered_map<ColumnId, 
std::pair<vectorized::NameAndTypePair,
   ```
   



##########
be/src/vec/exprs/vectorized_fn_call.h:
##########
@@ -50,6 +51,12 @@ class VectorizedFnCall : public VExpr {
     Status execute_runtime_fitler(doris::vectorized::VExprContext* context,
                                   doris::vectorized::Block* block, int* 
result_column_id,
                                   std::vector<size_t>& args) override;
+    Status eval_inverted_index(
+            VExprContext* context,
+            const std::unordered_map<ColumnId, 
std::pair<vectorized::NameAndTypePair,

Review Comment:
   warning: parameter 2 is const-qualified in the function declaration; 
const-qualification of parameters only has an effect in function definitions 
[readability-avoid-const-params-in-decls]
   
   ```suggestion
               std::unordered_map<ColumnId, 
std::pair<vectorized::NameAndTypePair,
   ```
   



##########
be/src/vec/functions/function.h:
##########
@@ -395,6 +406,14 @@ class IFunction : public 
std::enable_shared_from_this<IFunction>,
         return Status::OK();
     }
 
+    Status eval_inverted_index(FunctionContext* context,

Review Comment:
   warning: method 'eval_inverted_index' can be made static 
[readability-convert-member-functions-to-static]
   
   ```suggestion
       static Status eval_inverted_index(FunctionContext* context,
   ```
   
   be/src/vec/functions/function.h:411:
   ```diff
   -                                roaring::Roaring* bitmap) const override {
   +                                roaring::Roaring* bitmap) override {
   ```
   



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