From a9008b0fd27d8a4608cb25e2be1ef7272be22dfd Mon Sep 17 00:00:00 2001 From: feihongxu0824 Date: Mon, 18 May 2026 20:25:06 +0800 Subject: [PATCH] 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 --- src/db/sqlengine/planner/doc_filter.cc | 5 ++- tests/db/collection_test.cc | 55 ++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/db/sqlengine/planner/doc_filter.cc b/src/db/sqlengine/planner/doc_filter.cc index f91535a..756a1b9 100644 --- a/src/db/sqlengine/planner/doc_filter.cc +++ b/src/db/sqlengine/planner/doc_filter.cc @@ -98,7 +98,7 @@ std::optional 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(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 DocFilter::is_matched_by_forward_filter(uint64_t id) const { } arrow::Datum datum = maybe_result.MoveValueUnsafe(); if (datum.is_scalar()) { - return datum.scalar_as().value; + const auto &scalar = datum.scalar_as(); + return scalar.is_valid && scalar.value; } LOG_ERROR("Datum is not scalar, id[%zu] type[%s]", (size_t)id, datum.type()->ToString().c_str()); diff --git a/tests/db/collection_test.cc b/tests/db/collection_test.cc index 6053ad0..8738895 100644 --- a/tests/db/collection_test.cc +++ b/tests/db/collection_test.cc @@ -4340,4 +4340,59 @@ TEST_F(CollectionTest, CornerCase_CreateIndex) { std::make_shared(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(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>("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{"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"); + 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); } \ No newline at end of file