xiaokang commented on code in PR #32620:
URL: https://github.com/apache/doris/pull/32620#discussion_r1569755108
##########
be/src/vec/exprs/vcompound_pred.h:
##########
@@ -53,6 +53,70 @@ class VCompoundPred : public VectorizedFnCall {
const std::string& expr_name() const override { return _expr_name; }
+ // 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(
+ VExprContext* context,
+ const std::unordered_map<ColumnId,
std::pair<vectorized::NameAndTypePair,
+
segment_v2::InvertedIndexIterator*>>&
+ colid_to_inverted_index_iter,
+ uint32_t num_rows, roaring::Roaring* bitmap) const override {
+ std::shared_ptr<roaring::Roaring> res =
std::make_shared<roaring::Roaring>();
+ if (_op == TExprOpcode::COMPOUND_OR) {
+ for (auto child : _children) {
+ std::shared_ptr<roaring::Roaring> child_roaring =
+ std::make_shared<roaring::Roaring>();
+ Status st = child->eval_inverted_index(context,
colid_to_inverted_index_iter,
+ num_rows,
child_roaring.get());
+ if (!st.ok()) {
+ bitmap->addRange(0, num_rows);
+ return st;
+ }
+ *res |= *child_roaring;
+ if (res->cardinality() == num_rows) {
+ // means inverted index filter do not reduce any rows
+ // the left expr no need to be extracted by inverted index,
+ // and cur roaring is all rows which means this inverted
index is not useful,
+ // do not need to calculate with res bitmap
+ return Status::OK();
+ }
+ }
+ *bitmap = *res;
+ } else if (_op == TExprOpcode::COMPOUND_AND) {
+ for (auto child : _children) {
+ std::shared_ptr<roaring::Roaring> child_roaring =
+ std::make_shared<roaring::Roaring>();
+ Status st = child->eval_inverted_index(context,
colid_to_inverted_index_iter,
+ num_rows,
child_roaring.get());
+ if (!st.ok()) {
+ continue;
+ }
+ *res &= *child_roaring;
Review Comment:
res is all zeros when inited, so *res &= *child_roaring will always be zeros.
##########
be/src/vec/exprs/vexpr_context.cpp:
##########
@@ -119,6 +119,13 @@ int VExprContext::register_function_context(RuntimeState*
state, const TypeDescr
_fn_contexts.back()->set_check_overflow_for_decimal(state->check_overflow_for_decimal());
return _fn_contexts.size() - 1;
}
+Status VExprContext::eval_inverted_indexs(
+ const std::unordered_map<ColumnId,
std::pair<vectorized::NameAndTypePair,
+
segment_v2::InvertedIndexIterator*>>&
+ colid_to_inverted_index_iter,
+ uint32_t num_rows, roaring::Roaring* bitmap) {
+ return _root->eval_inverted_index(this, colid_to_inverted_index_iter,
num_rows, bitmap);
Review Comment:
Is there any special purpose for name s? If no, you can just use
eval_inverted_index.
##########
be/src/vec/exprs/vcompound_pred.h:
##########
@@ -53,6 +53,70 @@ class VCompoundPred : public VectorizedFnCall {
const std::string& expr_name() const override { return _expr_name; }
+ // 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(
+ VExprContext* context,
+ const std::unordered_map<ColumnId,
std::pair<vectorized::NameAndTypePair,
+
segment_v2::InvertedIndexIterator*>>&
+ colid_to_inverted_index_iter,
+ uint32_t num_rows, roaring::Roaring* bitmap) const override {
Review Comment:
bitmap is over written by new values and its original value passed in is
totally ignored. So what's the sematics of this argument?
##########
be/src/olap/rowset/segment_v2/segment_iterator.cpp:
##########
@@ -1210,6 +1227,34 @@ Status SegmentIterator::_apply_inverted_index() {
}
}
+ // support expr to evaluate inverted index
+ std::unordered_map<ColumnId, std::pair<vectorized::NameAndTypePair,
InvertedIndexIterator*>>
+ iter_map;
+
+ for (auto col_id : _common_expr_columns) {
+ if (_check_apply_by_inverted_index(col_id)) {
+ iter_map[col_id] = std::make_pair(_storage_name_and_type[col_id],
+
_inverted_index_iterators[col_id].get());
+ }
+ }
+ for (auto expr_ctx : _common_expr_ctxs_push_down) {
+ // _inverted_index_iterators has all column ids which has inverted
index
+ // _common_expr_columns has all column ids from
_common_expr_ctxs_push_down
+ // if current bitmap is already empty just return
+ if (_row_bitmap.isEmpty()) {
+ break;
+ }
+ std::shared_ptr<roaring::Roaring> result_bitmap =
std::make_shared<roaring::Roaring>();
+ if (Status st = expr_ctx->eval_inverted_indexs(iter_map, num_rows(),
result_bitmap.get());
Review Comment:
strange code style. Suggest that just call eval_inverted_indexs, and then
check ok status in if branch, and then check not ok status in else branch.
##########
be/src/vec/exprs/vcompound_pred.h:
##########
@@ -53,6 +53,70 @@ class VCompoundPred : public VectorizedFnCall {
const std::string& expr_name() const override { return _expr_name; }
+ // 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(
+ VExprContext* context,
+ const std::unordered_map<ColumnId,
std::pair<vectorized::NameAndTypePair,
+
segment_v2::InvertedIndexIterator*>>&
+ colid_to_inverted_index_iter,
+ uint32_t num_rows, roaring::Roaring* bitmap) const override {
+ std::shared_ptr<roaring::Roaring> res =
std::make_shared<roaring::Roaring>();
+ if (_op == TExprOpcode::COMPOUND_OR) {
+ for (auto child : _children) {
+ std::shared_ptr<roaring::Roaring> child_roaring =
+ std::make_shared<roaring::Roaring>();
+ Status st = child->eval_inverted_index(context,
colid_to_inverted_index_iter,
+ num_rows,
child_roaring.get());
+ if (!st.ok()) {
+ bitmap->addRange(0, num_rows);
Review Comment:
why addRange if status is not ok?
--
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]
