This is an automated email from the ASF dual-hosted git repository.
eldenmoon pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 30a699a18a7 [refactor](search) Simplify Variant search iterator
binding (#66449)
30a699a18a7 is described below
commit 30a699a18a7d9219a2188745da9d27ae84673b53
Author: lihangyu <[email protected]>
AuthorDate: Wed Aug 5 17:47:32 2026 +0800
[refactor](search) Simplify Variant search iterator binding (#66449)
Related PR: #60847, #63660
Problem Summary:
OlapScanner materializes a requested Variant path as a scan-schema
column. For example, data.items.message gets its own SlotRef and scan
column position. VariantColumnReader then resolves either direct
subcolumn index metadata or metadata inherited from data, and
SegmentIterator constructs the runtime iterator for that child column
with the complete stored Variant path.
VSearch duplicated that storage responsibility. When the child iterator
was absent, it resolved the tablet ColumnId for data, borrowed the
parent iterator, and synthesized a stored field name. This mixed scan
column positions with tablet ColumnIds and could bypass the path and
physical index selection already performed by the storage layer,
including NestedGroup selection.
This PR removes the expression-level parent rebinding. VSearch now
consumes only the iterator attached to the SlotRef scan column. If that
iterator is absent, the field remains an empty index input. It also
removes the three fallback-only IndexExecContext APIs and separates
SlotRef collection into a small helper that names the scan column index
explicitly.
A focused unit test constructs the exact boundary case: the data parent
iterator exists, while the data.items.message child iterator does not.
The test verifies that SEARCH does not mark or execute the parent
iterator.
### Release note
Variant SEARCH no longer borrows a parent Variant iterator when the
materialized child iterator is absent.
---
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 5bfd77f923e..3f5e33510cd 100644
--- a/be/src/exprs/vexpr_context.h
+++ b/be/src/exprs/vexpr_context.h
@@ -91,16 +91,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()) {
@@ -113,22 +103,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 fdfc36e2098..ecaef392db8 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;
}
@@ -330,8 +246,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 cc3fa5820fc..9703a465093 100644
--- a/be/test/exprs/vsearch_expr_test.cpp
+++ b/be/test/exprs/vsearch_expr_test.cpp
@@ -35,6 +35,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
@@ -42,6 +43,7 @@
#endif
#define private public
#include "exprs/vslot_ref.h"
+#include "storage/segment/segment.h"
#undef private
#if defined(__clang__)
#pragma clang diagnostic pop
@@ -105,6 +107,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 {
@@ -1359,6 +1377,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]
