airborne12 commented on code in PR #38908:
URL: https://github.com/apache/doris/pull/38908#discussion_r1719278316
##########
be/src/vec/exprs/vcompound_pred.h:
##########
@@ -53,76 +54,94 @@ 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::IndexFieldNameAndTypePair,
-
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;
+ Status evaluate_inverted_index(VExprContext* context,
+ uint32_t segment_num_rows) const override {
+ segment_v2::InvertedIndexResultBitmap res;
+ bool all_pass = true;
+
+ switch (_op) {
+ case TExprOpcode::COMPOUND_OR: {
+ for (const auto& child : _children) {
+ if (Status st = child->evaluate_inverted_index(context,
segment_num_rows);
+ !st.ok()) {
+ LOG(ERROR) << "expr:" << child->expr_name()
+ << " evaluate_inverted_index error:" <<
st.to_string();
+ all_pass = false;
+ continue;
}
- if (child_roaring->cardinality() == 0) {
- // 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
- bitmap->addRange(0, num_rows);
- return Status::OK();
+ if (context->has_inverted_index_result_for_expr(child.get())) {
+ auto index_result =
context->get_inverted_index_result_for_expr(child.get());
+ if (res.is_empty()) {
+ res = std::move(index_result);
+ } else {
+ res |= index_result;
+ }
+ } else {
+ all_pass = false;
}
- *res |= *child_roaring;
}
- *bitmap = *res;
- } else if (_op == TExprOpcode::COMPOUND_AND) {
- for (int i = 0; i < _children.size(); ++i) {
- std::shared_ptr<roaring::Roaring> child_roaring =
- std::make_shared<roaring::Roaring>();
- Status st = _children[0]->eval_inverted_index(context,
colid_to_inverted_index_iter,
- num_rows,
child_roaring.get());
- if (!st.ok()) {
+ break;
+ }
+ case TExprOpcode::COMPOUND_AND: {
+ for (const auto& child : _children) {
+ if (Status st = child->evaluate_inverted_index(context,
segment_num_rows);
+ !st.ok()) {
+ LOG(ERROR) << "expr:" << child->expr_name()
+ << " evaluate_inverted_index error:" <<
st.to_string();
+ all_pass = false;
continue;
}
- if (i == 0) {
- *res = *child_roaring;
+ if (context->has_inverted_index_result_for_expr(child.get())) {
+ auto index_result =
context->get_inverted_index_result_for_expr(child.get());
+ if (res.is_empty()) {
+ res = std::move(index_result);
+ } else {
+ res &= index_result;
+ }
+
+ if (res.get_data_bitmap()->isEmpty()) {
+ break; // Early exit if result is empty
+ }
} else {
- *res &= *child_roaring;
- }
- if (res->isEmpty()) {
- // the left expr no need to be extracted by inverted
index, just return 0 rows
- // res bitmap will be zero
- return Status::OK();
+ all_pass = false;
}
}
- *bitmap = *res;
- } else if (_op == TExprOpcode::COMPOUND_NOT) {
- Status st = _children[0]->eval_inverted_index(context,
colid_to_inverted_index_iter,
- num_rows, res.get());
+ break;
+ }
+ case TExprOpcode::COMPOUND_NOT: {
+ const auto& child = _children[0];
+ Status st = child->evaluate_inverted_index(context,
segment_num_rows);
if (!st.ok()) {
+ LOG(ERROR) << "expr:" << child->expr_name()
+ << " evaluate_inverted_index error:" <<
st.to_string();
return st;
}
- std::shared_ptr<roaring::Roaring> all_rows =
std::make_shared<roaring::Roaring>();
- all_rows->addRange(0, num_rows);
- *bitmap = *all_rows - *res;
- } else {
+
+ if (context->has_inverted_index_result_for_expr(child.get())) {
+ auto index_result =
context->get_inverted_index_result_for_expr(child.get());
+ roaring::Roaring full_result;
+ full_result.addRange(0, segment_num_rows);
+ res = std::move(index_result.op_not(&full_result));
+ } else {
+ all_pass = false;
+ }
+ break;
+ }
+ default:
return Status::NotSupported(
- "Compound operator must be AND or OR or Not can execute
with inverted index.");
+ "Compound operator must be AND, OR, or NOT to execute with
inverted index.");
+ }
+
+ if (all_pass && !res.is_empty()) {
Review Comment:
`res.is_empty` here means that the bitmap in `res` is `nullptr`. In other
words, the inverted index evaluation in the compound predicate may not have
been processed.
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
