mirror of
https://github.com/milvus-io/milvus.git
synced 2026-07-21 10:15:43 +00:00
fix: avoid null expr index path recursion (#50983)
## Summary issue: #50980 Fix ARRAY `IS NULL` hang when a scalar BITMAP index is present by avoiding recursive `std::call_once` re-entry in `PhyNullExpr::DetermineExecPath()`. This PR also adds a sealed ARRAY + BITMAP null-expression regression test. ## Verification - `clang-format-12 -i internal/core/src/exec/expression/NullExpr.cpp internal/core/src/exec/expression/ExprArrayTest.cpp` - `git diff --check` - built `all_tests` - focused C++ tests passed: `Expr.TestArrayNullExprWithBitmapIndex:Expr.TestArrayNullExpr:Expr.TestStructArrayParentNullExprUsesRepresentativeSubField:Expr.TestVectorArrayNullExpr` Signed-off-by: Buqian Zheng <zhengbuqian@gmail.com>
This commit is contained in:
@@ -2226,11 +2226,7 @@ class SegmentExpr : public Expr {
|
||||
bool
|
||||
CanUseNestedIndex() const override {
|
||||
EnsureExecPathDetermined();
|
||||
if (exec_path_ != ExprExecPath::ScalarIndex || pinned_index_.empty()) {
|
||||
return false;
|
||||
}
|
||||
auto* index_ptr = pinned_index_[0].get();
|
||||
return index_ptr != nullptr && index_ptr->IsNestedIndex();
|
||||
return PinnedIndexIsNested();
|
||||
}
|
||||
|
||||
template <typename T>
|
||||
@@ -2456,6 +2452,18 @@ class SegmentExpr : public Expr {
|
||||
}
|
||||
|
||||
protected:
|
||||
// Non-reentrant nested-index check for callers already inside
|
||||
// DetermineExecPath(). Do not call CanUseNestedIndex() from there because
|
||||
// it re-enters EnsureExecPathDetermined()'s std::call_once.
|
||||
bool
|
||||
PinnedIndexIsNested() const {
|
||||
if (exec_path_ != ExprExecPath::ScalarIndex || pinned_index_.empty()) {
|
||||
return false;
|
||||
}
|
||||
auto* index_ptr = pinned_index_[0].get();
|
||||
return index_ptr != nullptr && index_ptr->IsNestedIndex();
|
||||
}
|
||||
|
||||
const segcore::SegmentInternalInterface* segment_;
|
||||
const FieldId field_id_;
|
||||
bool is_pk_field_{false};
|
||||
|
||||
@@ -41,6 +41,7 @@
|
||||
#include "expr/ITypeExpr.h"
|
||||
#include "filemanager/InputStream.h"
|
||||
#include "gtest/gtest.h"
|
||||
#include "index/BitmapIndex.h"
|
||||
#include "knowhere/comp/index_param.h"
|
||||
#include "pb/plan.pb.h"
|
||||
#include "plan/PlanNode.h"
|
||||
@@ -551,6 +552,98 @@ TEST(Expr, TestArrayNullExpr) {
|
||||
}
|
||||
}
|
||||
|
||||
TEST(Expr, TestArrayNullExprWithBitmapIndex) {
|
||||
auto schema = std::make_shared<Schema>();
|
||||
schema->AddDebugField(
|
||||
"fakevec", DataType::VECTOR_FLOAT, 16, knowhere::metric::L2);
|
||||
auto i64_fid = schema->AddDebugField("id", DataType::INT64);
|
||||
auto long_array_fid = schema->AddDebugField(
|
||||
"long_array", DataType::ARRAY, DataType::INT64, true);
|
||||
schema->set_primary_field_id(i64_fid);
|
||||
|
||||
constexpr int N = 128;
|
||||
auto raw_data = DataGen(schema, N, 44, 0, 1, 3);
|
||||
auto valid_data = raw_data.get_col_valid(long_array_fid);
|
||||
auto long_array_col = raw_data.get_col<ScalarFieldProto>(long_array_fid);
|
||||
|
||||
auto segment = CreateSealedWithFieldDataLoaded(schema, raw_data);
|
||||
|
||||
FixedVector<Array> arrays;
|
||||
arrays.reserve(N);
|
||||
std::vector<uint8_t> valid_bitmap((N + 7) / 8, 0);
|
||||
for (int i = 0; i < N; ++i) {
|
||||
arrays.emplace_back(long_array_col[i]);
|
||||
if (valid_data[i]) {
|
||||
valid_bitmap[i >> 3] |= 1 << (i & 0x07);
|
||||
}
|
||||
}
|
||||
|
||||
auto field_data =
|
||||
storage::CreateFieldData(DataType::ARRAY, DataType::INT64, true);
|
||||
field_data->FillFieldData(arrays.data(), valid_bitmap.data(), N, 0);
|
||||
|
||||
proto::schema::FieldSchema field_schema;
|
||||
field_schema.set_name("long_array");
|
||||
field_schema.set_fieldid(long_array_fid.get());
|
||||
field_schema.set_data_type(proto::schema::DataType::Array);
|
||||
field_schema.set_element_type(proto::schema::DataType::Int64);
|
||||
field_schema.set_nullable(true);
|
||||
storage::FileManagerContext ctx;
|
||||
ctx.fieldDataMeta = storage::FieldDataMeta{
|
||||
kCollectionID,
|
||||
kPartitionID,
|
||||
kSegmentID,
|
||||
long_array_fid.get(),
|
||||
field_schema,
|
||||
};
|
||||
ctx.indexMeta =
|
||||
storage::IndexMeta{kSegmentID, long_array_fid.get(), 4000, 4000};
|
||||
|
||||
auto bitmap_index =
|
||||
std::make_unique<index::BitmapIndex<int64_t>>(ctx, false);
|
||||
bitmap_index->BuildWithFieldData({field_data});
|
||||
ASSERT_FALSE(bitmap_index->IsNestedIndex());
|
||||
ASSERT_EQ(bitmap_index->Count(), N);
|
||||
|
||||
LoadIndexInfo load_index_info;
|
||||
load_index_info.field_id = long_array_fid.get();
|
||||
load_index_info.field_type = DataType::ARRAY;
|
||||
load_index_info.element_type = DataType::INT64;
|
||||
load_index_info.index_params = GenIndexParams(bitmap_index.get());
|
||||
load_index_info.cache_index =
|
||||
CreateTestCacheIndex("array_bitmap", std::move(bitmap_index));
|
||||
segment->LoadIndex(load_index_info);
|
||||
|
||||
auto null_expr = std::make_shared<expr::NullExpr>(
|
||||
expr::ColumnInfo(
|
||||
long_array_fid, DataType::ARRAY, DataType::INT64, {}, true),
|
||||
proto::plan::NullExpr_NullOp_IsNull);
|
||||
auto plan =
|
||||
std::make_shared<plan::FilterBitsNode>(DEFAULT_PLANNODE_ID, null_expr);
|
||||
|
||||
auto final = ExecuteQueryExpr(plan, segment.get(), N, MAX_TIMESTAMP);
|
||||
ASSERT_EQ(final.size(), N);
|
||||
for (int i = 0; i < N; ++i) {
|
||||
ASSERT_EQ(final[i], !valid_data[i]) << "row " << i;
|
||||
}
|
||||
|
||||
milvus::exec::OffsetVector offsets;
|
||||
offsets.reserve(N / 2);
|
||||
for (int i = 0; i < N; ++i) {
|
||||
if (i % 2 == 0) {
|
||||
offsets.emplace_back(i);
|
||||
}
|
||||
}
|
||||
auto col_vec = milvus::test::gen_filter_res(
|
||||
plan.get(), segment.get(), N, MAX_TIMESTAMP, &offsets);
|
||||
BitsetTypeView view(col_vec->GetRawData(), col_vec->size());
|
||||
ASSERT_EQ(view.size(), offsets.size());
|
||||
for (int i = 0; i < offsets.size(); ++i) {
|
||||
ASSERT_EQ(view[i], !valid_data[offsets[i]])
|
||||
<< "offset row " << offsets[i];
|
||||
}
|
||||
}
|
||||
|
||||
TEST(Expr, TestStructArrayParentNullExprUsesRepresentativeSubField) {
|
||||
auto schema = std::make_shared<Schema>();
|
||||
auto fakevec_fid = schema->AddDebugField(
|
||||
|
||||
@@ -130,7 +130,7 @@ PhyNullExpr::DetermineExecPath() {
|
||||
}
|
||||
|
||||
SegmentExpr::DetermineExecPath();
|
||||
if (exec_path_ == ExprExecPath::ScalarIndex && CanUseNestedIndex()) {
|
||||
if (PinnedIndexIsNested()) {
|
||||
exec_path_ = ExprExecPath::RawData;
|
||||
pinned_index_.clear();
|
||||
num_index_chunk_ = 0;
|
||||
|
||||
Reference in New Issue
Block a user