This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new 87a7187512a branch-4.1: [refactor](search) Simplify Variant search
iterator binding #66449 (#66487)
87a7187512a is described below
commit 87a7187512a2d960cc541c3ce7b3d3a10eeccc36
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Aug 6 10:40:32 2026 +0800
branch-4.1: [refactor](search) Simplify Variant search iterator binding
#66449 (#66487)
Cherry-picked from #66449
Co-authored-by: lihangyu <[email protected]>
---
be/src/exprs/vexpr_context.h | 26 -----
be/src/exprs/vsearch.cpp | 208 +++++++++++-------------------------
be/test/exprs/vsearch_expr_test.cpp | 61 +++++++++++
3 files changed, 123 insertions(+), 172 deletions(-)
diff --git a/be/src/exprs/vexpr_context.h b/be/src/exprs/vexpr_context.h
index 358ab7cbc5c..b591cbd31f5 100644
--- a/be/src/exprs/vexpr_context.h
+++ b/be/src/exprs/vexpr_context.h
@@ -90,16 +90,6 @@ public:
return _index_iterators[column_id].get();
}
- segment_v2::IndexIterator* get_inverted_index_iterator_by_id(ColumnId
column_id) const {
- if (column_id >= _index_iterators.size()) {
- return nullptr;
- }
- if (!_index_iterators[column_id]) {
- return nullptr;
- }
- return _index_iterators[column_id].get();
- }
-
const IndexFieldNameAndTypePair* get_storage_name_and_type_by_column_id(
int column_index) const {
if (column_index < 0 || column_index >= _col_ids.size()) {
@@ -112,22 +102,6 @@ public:
return &_storage_name_and_type[column_id];
}
- const IndexFieldNameAndTypePair* get_storage_name_and_type_by_id(ColumnId
column_id) const {
- if (column_id >= _storage_name_and_type.size()) {
- return nullptr;
- }
- return &_storage_name_and_type[column_id];
- }
-
- int column_index_by_id(ColumnId column_id) const {
- for (int i = 0; i < _col_ids.size(); ++i) {
- if (_col_ids[i] == column_id) {
- return i;
- }
- }
- return -1;
- }
-
bool get_column_id(int column_index, ColumnId* column_id) const {
if (column_id == nullptr) {
return false;
diff --git a/be/src/exprs/vsearch.cpp b/be/src/exprs/vsearch.cpp
index 3b5da89d290..3de07df56ab 100644
--- a/be/src/exprs/vsearch.cpp
+++ b/be/src/exprs/vsearch.cpp
@@ -32,8 +32,6 @@
#include "glog/logging.h"
#include "runtime/runtime_state.h"
#include "storage/index/inverted/inverted_index_reader.h"
-#include "storage/olap_common.h"
-#include "storage/segment/segment.h"
namespace doris {
using namespace segment_v2;
@@ -44,7 +42,7 @@ struct SearchInputBundle {
std::unordered_map<std::string, IndexIterator*> iterators;
std::unordered_map<std::string, IndexFieldNameAndTypePair> field_types;
std::unordered_map<std::string, int> field_name_to_column_id;
- std::vector<int> column_ids;
+ std::vector<int> column_indexes;
ColumnsWithTypeAndName literal_args;
};
@@ -60,6 +58,58 @@ void add_search_binding_diagnostic(const IndexExecContext*
index_context,
}
}
+Status collect_slot_search_input(const VSearchExpr& expr, const VSlotRef&
slot_ref,
+ const TSearchFieldBinding* binding,
+ IndexExecContext* index_context,
SearchInputBundle* bundle) {
+ DCHECK(index_context != nullptr);
+ DCHECK(bundle != nullptr);
+
+ // VSlotRef::column_id() is the scan-schema position used by
IndexExecContext.
+ const int column_index = slot_ref.column_id();
+ const std::string field_name =
+ binding != nullptr ? binding->field_name : slot_ref.column_name();
+ const bool is_variant_subcolumn = binding != nullptr &&
binding->__isset.is_variant_subcolumn &&
+ binding->is_variant_subcolumn;
+
+ bundle->field_name_to_column_id[field_name] = column_index;
+
+ auto* iterator =
index_context->get_inverted_index_iterator_by_column_id(column_index);
+ if (iterator == nullptr) {
+ // For example, `data.items.message` has its own SlotRef in the scan
schema. The
+ // storage layer may inherit index metadata from `data`, but it still
constructs a
+ // child iterator whose stored field name contains the complete
Variant path.
+ if (is_variant_subcolumn) {
+ add_search_binding_diagnostic(
+ index_context,
+ fmt::format("[VariantSearchBinding] phase=collect_inputs "
+ "result=no_iterator logical_field={}
column_index={} "
+ "reason=slot_iterator_missing",
+ field_name, column_index));
+ }
+ return Status::OK();
+ }
+
+ const auto* storage_name_type =
+
index_context->get_storage_name_and_type_by_column_id(column_index);
+ if (storage_name_type == nullptr) {
+ return Status::InternalError("storage_name_type not found for column
{} in {}",
+ column_index, expr.expr_name());
+ }
+
+ bundle->iterators.emplace(field_name, iterator);
+ bundle->field_types.emplace(field_name, *storage_name_type);
+ bundle->column_indexes.emplace_back(column_index);
+ if (is_variant_subcolumn) {
+ add_search_binding_diagnostic(
+ index_context,
+ fmt::format("[VariantSearchBinding] phase=collect_inputs "
+ "result=direct_iterator logical_field={}
column_index={} "
+ "stored_field={}",
+ field_name, column_index,
storage_name_type->first));
+ }
+ return Status::OK();
+}
+
Status collect_search_inputs(const VSearchExpr& expr, VExprContext* context,
SearchInputBundle* bundle) {
DCHECK(bundle != nullptr);
@@ -70,152 +120,18 @@ Status collect_search_inputs(const VSearchExpr& expr,
VExprContext* context,
return Status::InternalError("No inverted index context available");
}
- // Get field bindings for variant subcolumn support
const auto& search_param = expr.get_search_param();
const auto& field_bindings = search_param.field_bindings;
- std::unordered_map<std::string, ColumnId> parent_to_base_column_id;
- std::unordered_map<std::string, std::string>
parent_to_storage_field_prefix;
-
- // Resolve and cache the base (parent) column id for a variant field
binding.
- // This avoids repeated schema lookups when multiple subcolumns share the
same parent column.
- auto resolve_parent_column_id = [&](const std::string& parent_field,
ColumnId* column_id) {
- // Guard against invalid inputs: variant bindings may miss
parent_field, and callers must
- // provide a valid output pointer to receive the resolved id.
- if (parent_field.empty() || column_id == nullptr) {
- return false;
- }
- auto it = parent_to_base_column_id.find(parent_field);
- if (it != parent_to_base_column_id.end()) {
- *column_id = it->second;
- return true;
- }
- if (index_context == nullptr || index_context->segment() == nullptr) {
- return false;
- }
- const int32_t ordinal =
-
index_context->segment()->tablet_schema()->field_index(parent_field);
- if (ordinal < 0) {
- return false;
- }
- ColumnId resolved_id = static_cast<ColumnId>(ordinal);
- parent_to_base_column_id.emplace(parent_field, resolved_id);
- if (auto* storage_name_type =
index_context->get_storage_name_and_type_by_id(resolved_id);
- storage_name_type != nullptr) {
- parent_to_storage_field_prefix[parent_field] =
storage_name_type->first;
- }
- *column_id = resolved_id;
- return true;
- };
-
- int child_index = 0; // Index for iterating through children
+ size_t child_index = 0;
for (const auto& child : expr.children()) {
if (child->is_slot_ref()) {
auto* column_slot_ref = assert_cast<VSlotRef*>(child.get());
- int column_id = column_slot_ref->column_id();
-
- // Determine the field_name from field_bindings (for variant
subcolumns)
- // field_bindings and children should have the same order
- std::string field_name;
- const TSearchFieldBinding* binding = nullptr;
- if (child_index < field_bindings.size()) {
- // Use field_name from binding (may include "parent.subcolumn"
for variant)
- binding = &field_bindings[child_index];
- field_name = binding->field_name;
- } else {
- // Fallback to column_name if binding not found
- field_name = column_slot_ref->column_name();
- }
-
- bundle->field_name_to_column_id[field_name] = column_id;
-
- auto* iterator =
index_context->get_inverted_index_iterator_by_column_id(column_id);
- const auto* storage_name_type =
-
index_context->get_storage_name_and_type_by_column_id(column_id);
- bool field_added = false;
- // For variant subcolumns, slot_ref might not map to a real
indexed column in the scan schema.
- // Fall back to the parent variant column's iterator and
synthesize lucene field name.
- if (iterator == nullptr && binding != nullptr &&
- binding->__isset.is_variant_subcolumn &&
binding->is_variant_subcolumn &&
- binding->__isset.parent_field_name &&
!binding->parent_field_name.empty()) {
- ColumnId base_column_id = 0;
- if (resolve_parent_column_id(binding->parent_field_name,
&base_column_id)) {
- iterator =
index_context->get_inverted_index_iterator_by_id(base_column_id);
- const auto* base_storage_name_type =
-
index_context->get_storage_name_and_type_by_id(base_column_id);
- if (iterator != nullptr && base_storage_name_type !=
nullptr) {
- std::string prefix = base_storage_name_type->first;
- if (auto pit =
-
parent_to_storage_field_prefix.find(binding->parent_field_name);
- pit != parent_to_storage_field_prefix.end() &&
!pit->second.empty()) {
- prefix = pit->second;
- } else {
-
parent_to_storage_field_prefix[binding->parent_field_name] = prefix;
- }
-
- std::string sub_path;
- if (binding->__isset.subcolumn_path) {
- sub_path = binding->subcolumn_path;
- }
- if (sub_path.empty()) {
- // Fallback: strip "parent." prefix from logical
field name
- std::string pfx = binding->parent_field_name + ".";
- if (field_name.starts_with(pfx)) {
- sub_path = field_name.substr(pfx.size());
- }
- }
- if (!sub_path.empty()) {
- bundle->iterators[field_name] = iterator;
- bundle->field_types[field_name] =
- std::make_pair(prefix + "." + sub_path,
nullptr);
- int base_column_index =
-
index_context->column_index_by_id(base_column_id);
- if (base_column_index >= 0) {
-
bundle->column_ids.emplace_back(base_column_index);
- }
- add_search_binding_diagnostic(
- index_context.get(),
- fmt::format("[VariantSearchBinding]
phase=collect_inputs "
- "result=parent_fallback
logical_field={} "
- "parent_field={} sub_path={}
base_column_id={} "
- "stored_field={}
reason=slot_iterator_missing",
- field_name,
binding->parent_field_name, sub_path,
- base_column_id, prefix + "." +
sub_path));
- field_added = true;
- }
- }
- } else {
- add_search_binding_diagnostic(
- index_context.get(),
- fmt::format("[VariantSearchBinding]
phase=collect_inputs "
- "result=reject logical_field={}
parent_field={} "
- "reason=parent_column_not_found",
- field_name,
binding->parent_field_name));
- }
- }
-
- // Only collect fields that have iterators (materialized columns
with indexes)
- if (!field_added && iterator != nullptr) {
- if (storage_name_type == nullptr) {
- return Status::InternalError("storage_name_type not found
for column {} in {}",
- column_id, expr.expr_name());
- }
-
- bundle->iterators.emplace(field_name, iterator);
- bundle->field_types.emplace(field_name, *storage_name_type);
- bundle->column_ids.emplace_back(column_id);
- if (binding != nullptr &&
binding->__isset.is_variant_subcolumn &&
- binding->is_variant_subcolumn) {
- add_search_binding_diagnostic(
- index_context.get(),
- fmt::format("[VariantSearchBinding]
phase=collect_inputs "
- "result=direct_iterator
logical_field={} column_id={} "
- "stored_field={}",
- field_name, column_id,
storage_name_type->first));
- }
- }
-
- child_index++;
+ const TSearchFieldBinding* binding =
+ child_index < field_bindings.size() ?
&field_bindings[child_index] : nullptr;
+ RETURN_IF_ERROR(collect_slot_search_input(expr, *column_slot_ref,
binding,
+ index_context.get(),
bundle));
+ ++child_index;
} else if (child->is_literal()) {
auto* literal = assert_cast<VLiteral*>(child.get());
bundle->literal_args.emplace_back(literal->get_column_ptr(),
literal->get_data_type(),
@@ -238,7 +154,7 @@ Status collect_search_inputs(const VSearchExpr& expr,
VExprContext* context,
field_bindings[child_index].__isset.subcolumn_path
?
field_bindings[child_index].subcolumn_path
: ""));
- child_index++;
+ ++child_index;
continue;
}
@@ -329,8 +245,8 @@ Status VSearchExpr::evaluate_inverted_index(VExprContext*
context, uint32_t segm
}
index_context->set_index_result_for_expr(this, result_bitmap);
- for (int column_id : bundle.column_ids) {
- index_context->set_true_for_index_status(this, column_id);
+ for (int column_index : bundle.column_indexes) {
+ index_context->set_true_for_index_status(this, column_index);
}
return Status::OK();
diff --git a/be/test/exprs/vsearch_expr_test.cpp
b/be/test/exprs/vsearch_expr_test.cpp
index f8b02ce50d1..74d149e3392 100644
--- a/be/test/exprs/vsearch_expr_test.cpp
+++ b/be/test/exprs/vsearch_expr_test.cpp
@@ -33,6 +33,7 @@
#include "exprs/vsearch.h"
#include "storage/index/index_iterator.h"
#include "storage/segment/variant/nested_group_provider.h"
+#include "storage/tablet/tablet_schema.h"
#if defined(__clang__)
#pragma clang diagnostic push
@@ -40,6 +41,7 @@
#endif
#define private public
#include "exprs/vslot_ref.h"
+#include "storage/segment/segment.h"
#undef private
#if defined(__clang__)
#pragma clang diagnostic pop
@@ -103,6 +105,22 @@ std::shared_ptr<IndexExecContext> make_inverted_context(
nullptr, nullptr,
column_iter_opts);
}
+std::shared_ptr<segment_v2::Segment> make_segment_with_variant_parent() {
+ TabletSchemaPB schema_pb;
+ schema_pb.set_keys_type(KeysType::DUP_KEYS);
+ auto* parent = schema_pb.add_column();
+ parent->set_unique_id(0);
+ parent->set_name("data");
+ parent->set_type("VARIANT");
+ parent->set_is_key(false);
+ parent->set_is_nullable(true);
+
+ auto tablet_schema = std::make_shared<TabletSchema>();
+ tablet_schema->init_from_pb(schema_pb);
+ return std::make_shared<segment_v2::Segment>(0, RowsetId(), tablet_schema,
+ InvertedIndexFileInfo());
+}
+
} // namespace
class VSearchExprTest : public testing::Test {
@@ -1372,6 +1390,49 @@ TEST_F(VSearchExprTest,
EvaluateInvertedIndexHandlesMissingIterators) {
EXPECT_FALSE(status_map[0][expr.get()]);
}
+TEST_F(VSearchExprTest, MissingVariantChildIteratorDoesNotUseParentIterator) {
+ TExprNode variant_node = test_node;
+ variant_node.search_param.original_dsl = "data.items.message:hello";
+ variant_node.search_param.root.field_name = "data.items.message";
+ auto& binding = variant_node.search_param.field_bindings.front();
+ binding.field_name = "data.items.message";
+ binding.__set_is_variant_subcolumn(true);
+ binding.__set_parent_field_name("data");
+ binding.__set_subcolumn_path("items.message");
+
+ auto expr = VSearchExpr::create_shared(variant_node);
+ expr->add_child(create_slot_ref(1, "data.items.message"));
+
+ // Scan column 0 is the Variant parent and has an iterator. Scan column 1
is the requested
+ // child and intentionally has none. SEARCH must not reinterpret the
parent iterator as the
+ // child's index.
+ std::vector<ColumnId> col_ids = {0, 1};
+ std::vector<std::unique_ptr<segment_v2::IndexIterator>> index_iterators;
+ index_iterators.emplace_back(std::make_unique<StubIndexIterator>());
+ index_iterators.emplace_back(nullptr);
+ std::vector<IndexFieldNameAndTypePair> storage_types;
+ storage_types.emplace_back("0.data", std::make_shared<DataTypeString>());
+ storage_types.emplace_back("0.data.items.message",
std::make_shared<DataTypeString>());
+ std::unordered_map<ColumnId, std::unordered_map<const VExpr*, bool>>
status_map;
+ status_map[0][expr.get()] = false;
+ status_map[1][expr.get()] = false;
+
+ // Keep the parent in the segment schema so a parent-rebinding
implementation would find it.
+ segment_v2::ColumnIteratorOptions column_iter_opts;
+ auto segment = make_segment_with_variant_parent();
+ auto inverted_ctx =
+ std::make_shared<IndexExecContext>(col_ids, index_iterators,
storage_types, status_map,
+ nullptr, segment.get(),
column_iter_opts);
+ auto context = std::make_shared<VExprContext>(expr);
+ context->set_index_context(inverted_ctx);
+
+ auto status = expr->evaluate_inverted_index(context.get(), 32);
+ EXPECT_TRUE(status.ok()) << status;
+ EXPECT_TRUE(inverted_ctx->has_index_result_for_expr(expr.get()));
+ EXPECT_FALSE(status_map[0][expr.get()]);
+ EXPECT_FALSE(status_map[1][expr.get()]);
+}
+
TEST_F(VSearchExprTest,
EvaluateInvertedIndexNestedFallbackReturnsNotSupportedInCE) {
TExprNode nested_node = test_node;
nested_node.num_children = 0;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
