xiaoxue_admin
/
zvec
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: nullable scalar field filter leaks null documents without inverted index (#410) 

When a nullable scalar field has no inverted index, the forward filter path
fails to handle null values from Arrow's filter evaluation:

1. get_forward_bit(): BooleanArray::operator[] returns nullopt for null entries,
   which is_filtered() treats as "no filter" (not filtered), letting null docs
   through. Fix: use value_or(false) to treat null as "not matched".

2. is_matched_by_forward_filter(): reads BooleanScalar.value without checking
   is_valid, which is UB for null scalars. Fix: check is_valid first.

Closes #409

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
feihongxu0824 2026-05-18 20:25:06 +08:00 committed by GitHub
parent 3e02031eb8
commit a9008b0fd2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 58 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/db/sqlengine/planner/doc_filter.cc
View File

@ -98,7 +98,7 @@ std::optional<bool> DocFilter::get_forward_bit(uint64_t id) const {
const auto &arr = forward_bitmap_->chunk(c);
if (id < rows_seen + arr->length()) {
auto *bool_array = static_cast<arrow::BooleanArray *>(arr.get());
return (*bool_array)[id - rows_seen];
return (*bool_array)[id - rows_seen].value_or(false);
}
rows_seen += arr->length();
}
@ -155,7 +155,8 @@ std::optional<bool> DocFilter::is_matched_by_forward_filter(uint64_t id) const {
}
arrow::Datum datum = maybe_result.MoveValueUnsafe();
if (datum.is_scalar()) {
return datum.scalar_as<arrow::BooleanScalar>().value;
const auto &scalar = datum.scalar_as<arrow::BooleanScalar>();
return scalar.is_valid && scalar.value;
}
LOG_ERROR("Datum is not scalar, id[%zu] type[%s]", (size_t)id,
datum.type()->ToString().c_str());

55
tests/db/collection_test.cc
View File

@ -4340,4 +4340,59 @@ TEST_F(CollectionTest, CornerCase_CreateIndex) {
std::make_shared<IVFIndexParams>(MetricType::IP));
ASSERT_FALSE(s.ok());
ASSERT_EQ(s.code(), StatusCode::INVALID_ARGUMENT);
}
TEST_F(CollectionTest, Feature_Query_NullableFilter_WithoutIndex) {
auto run_test = [&](bool with_scalar_index) {
FileHelper::RemoveDirectory(col_path);
IndexParams::Ptr scalar_idx =
with_scalar_index ? std::make_shared<InvertIndexParams>(false) : nullptr;
auto schema =
TestHelper::CreateNormalSchema(/*nullable=*/true, "demo", scalar_idx);
CollectionOptions options{false, true, 100 * 1024 * 1024};
auto result = Collection::CreateAndOpen(col_path, *schema, options);
ASSERT_TRUE(result.has_value());
auto collection = result.value();
int non_null_count = 50;
int null_count = 50;
int total = non_null_count + null_count;
auto s = TestHelper::CollectionInsertDoc(collection, 0, non_null_count,
/*nullable=*/false);
ASSERT_TRUE(s.ok());
s = TestHelper::CollectionInsertDoc(collection, non_null_count, total,
/*nullable=*/true);
ASSERT_TRUE(s.ok());
collection->Flush();
auto stats = collection->Stats().value();
ASSERT_EQ(stats.doc_count, total);
auto query_doc = TestHelper::CreateDoc(1, *schema);
VectorQuery query;
query.topk_ = total;
query.field_name_ = "dense_fp32";
auto vec = query_doc.get<std::vector<float>>("dense_fp32");
ASSERT_TRUE(vec.has_value());
query.query_vector_.assign((char *)vec.value().data(),
vec.value().size() * sizeof(float));
query.filter_ = "int32 > 0";
query.output_fields_ = std::vector<std::string>{"int32"};
auto query_result = collection->Query(query);
ASSERT_TRUE(query_result.has_value());
for (auto &doc : query_result.value()) {
auto int32_val = doc->get<int32_t>("int32");
ASSERT_TRUE(int32_val.has_value())
<< "Null doc leaked through filter: " << doc->pk()
<< " (with_scalar_index=" << with_scalar_index << ")";
ASSERT_GT(int32_val.value(), 0);
}
ASSERT_EQ(query_result.value().size(), non_null_count - 1)
<< "with_scalar_index=" << with_scalar_index;
};
run_test(false);
run_test(true);
}