fix: persist bitmap index valid bitset for nullable array fields (#49008)

issue: https://github.com/milvus-io/milvus/issues/48901 --------- Signed-off-by: SpadeA <tangchenjie1210@gmail.com>
2026-07-21 02:05:41 +00:00 · 2026-04-16 14:21:41 +08:00
parent 90c56084fb
commit ddd14094ff
4 changed files with 408 additions and 18 deletions
@@ -231,6 +231,21 @@ BitmapIndex<T>::SerializeIndexData(uint8_t* data_ptr) {
    }
 }

+template <typename T>
+std::pair<std::shared_ptr<uint8_t[]>, size_t>
+BitmapIndex<T>::SerializeValidBitsetData() const {
+    size_t valid_bitset_size = (total_num_rows_ + 7) / 8;
+    std::shared_ptr<uint8_t[]> valid_bitset_data(
+        new uint8_t[valid_bitset_size]);
+    memset(valid_bitset_data.get(), 0, valid_bitset_size);
+    for (size_t i = 0; i < total_num_rows_; ++i) {
+        if (valid_bitset_[i]) {
+            valid_bitset_data[i / 8] |= (1 << (i % 8));
+        }
+    }
+    return std::make_pair(valid_bitset_data, valid_bitset_size);
+}
+
 template <typename T>
 std::pair<std::shared_ptr<uint8_t[]>, size_t>
 BitmapIndex<T>::SerializeIndexMeta() {
@@ -263,6 +278,24 @@ BitmapIndex<std::string>::SerializeIndexData(uint8_t* data_ptr) {
    }
 }

+template <typename T>
+void
+BitmapIndex<T>::DeserializeValidBitsetData(const uint8_t* data_ptr,
+                                           size_t data_size) {
+    auto expected_size = (total_num_rows_ + 7) / 8;
+    AssertInfo(data_size == expected_size,
+               "bitmap valid_bitset size mismatch, expect {}, got {}",
+               expected_size,
+               data_size);
+    valid_bitset_ = TargetBitmap(total_num_rows_, false);
+    for (size_t i = 0; i < total_num_rows_; ++i) {
+        uint8_t byte = data_ptr[i / 8];
+        if (byte & (1 << (i % 8))) {
+            valid_bitset_.set(i);
+        }
+    }
+}
+
 template <typename T>
 BinarySet
 BitmapIndex<T>::Serialize(const Config& config) {
@@ -279,6 +312,11 @@ BitmapIndex<T>::Serialize(const Config& config) {
    BinarySet ret_set;
    ret_set.Append(BITMAP_INDEX_DATA, index_data, index_data_size);
    ret_set.Append(BITMAP_INDEX_META, index_meta.first, index_meta.second);
+    if (schema_.nullable()) {
+        auto valid_bitset = SerializeValidBitsetData();
+        ret_set.Append(
+            BITMAP_INDEX_VALID_BITSET, valid_bitset.first, valid_bitset.second);
+    }

    LOG_INFO("build bitmap index with cardinality = {}, num_rows = {}",
             data_.size(),
@@ -354,7 +392,8 @@ BitmapIndex<T>::ChooseIndexLoadMode(int64_t index_length) {
 template <typename T>
 void
 BitmapIndex<T>::DeserializeIndexData(const uint8_t* data_ptr,
-                                     size_t index_length) {
+                                     size_t index_length,
+                                     bool rebuild_validity_from_postings) {
    ChooseIndexLoadMode(index_length);
    for (size_t i = 0; i < index_length; ++i) {
        T key;
@@ -370,8 +409,10 @@ BitmapIndex<T>::DeserializeIndexData(const uint8_t* data_ptr,
        } else {
            data_[key] = value;
        }
-        for (const auto& v : value) {
-            valid_bitset_.set(v);
+        if (rebuild_validity_from_postings) {
+            for (const auto& v : value) {
+                valid_bitset_.set(v);
+            }
        }
    }
 }
@@ -413,8 +454,10 @@ BitmapIndex<T>::BuildOffsetCache() {

 template <>
 void
-BitmapIndex<std::string>::DeserializeIndexData(const uint8_t* data_ptr,
-                                               size_t index_length) {
+BitmapIndex<std::string>::DeserializeIndexData(
+    const uint8_t* data_ptr,
+    size_t index_length,
+    bool rebuild_validity_from_postings) {
    ChooseIndexLoadMode(index_length);
    for (size_t i = 0; i < index_length; ++i) {
        size_t key_size;
@@ -433,8 +476,10 @@ BitmapIndex<std::string>::DeserializeIndexData(const uint8_t* data_ptr,
        } else {
            data_[key] = value;
        }
-        for (const auto& v : value) {
-            valid_bitset_.set(v);
+        if (rebuild_validity_from_postings) {
+            for (const auto& v : value) {
+                valid_bitset_.set(v);
+            }
        }
    }
 }
@@ -468,7 +513,8 @@ BitmapIndex<T>::MMapIndexData(const std::string& file_name,
                              const uint8_t* data_ptr,
                              size_t data_size,
                              size_t index_length,
-                              milvus::proto::common::LoadPriority priority) {
+                              milvus::proto::common::LoadPriority priority,
+                              bool rebuild_validity_from_postings) {
    std::filesystem::create_directories(
        std::filesystem::path(file_name).parent_path());

@@ -483,8 +529,10 @@ BitmapIndex<T>::MMapIndexData(const std::string& file_name,
            roaring::Roaring value;
            value =
                roaring::Roaring::read(reinterpret_cast<const char*>(data_ptr));
-            for (const auto& v : value) {
-                valid_bitset_.set(v);
+            if (rebuild_validity_from_postings) {
+                for (const auto& v : value) {
+                    valid_bitset_.set(v);
+                }
            }

            // convert roaring vaule to frozen mode
@@ -537,7 +585,15 @@ BitmapIndex<T>::LoadWithoutAssemble(const BinarySet& binary_set,
                                           index_meta_buffer->size);
    auto index_length = index_meta.first;
    total_num_rows_ = index_meta.second;
-    valid_bitset_ = TargetBitmap(total_num_rows_, false);
+    valid_bitset_ = TargetBitmap(total_num_rows_, !schema_.nullable());
+    bool rebuild_validity_from_postings = schema_.nullable();
+
+    auto valid_bitset_buffer = binary_set.GetByName(BITMAP_INDEX_VALID_BITSET);
+    if (valid_bitset_buffer != nullptr) {
+        DeserializeValidBitsetData(valid_bitset_buffer->data.get(),
+                                   valid_bitset_buffer->size);
+        rebuild_validity_from_postings = false;
+    }

    auto index_data_buffer = binary_set.GetByName(BITMAP_INDEX_DATA);

@@ -558,9 +614,12 @@ BitmapIndex<T>::LoadWithoutAssemble(const BinarySet& binary_set,
                      index_data_buffer->data.get(),
                      index_data_buffer->size,
                      index_length,
-                      priority);
+                      priority,
+                      rebuild_validity_from_postings);
    } else {
-        DeserializeIndexData(index_data_buffer->data.get(), index_length);
+        DeserializeIndexData(index_data_buffer->data.get(),
+                             index_length,
+                             rebuild_validity_from_postings);
    }

    if (enable_offset_cache.has_value() && enable_offset_cache.value()) {
@@ -1313,6 +1372,12 @@ BitmapIndex<T>::WriteEntries(storage::IndexEntryWriter* writer) {
    uint8_t* data_ptr = index_data.get();
    SerializeIndexData(data_ptr);
    writer->WriteEntry(BITMAP_INDEX_DATA, index_data.get(), index_data_size);
+    if (schema_.nullable()) {
+        auto valid_bitset = SerializeValidBitsetData();
+        writer->WriteEntry(BITMAP_INDEX_VALID_BITSET,
+                           valid_bitset.first.get(),
+                           valid_bitset.second);
+    }

    LOG_INFO("write bitmap index entries with cardinality = {}, num_rows = {}",
             data_.size(),
@@ -1329,7 +1394,18 @@ BitmapIndex<T>::LoadEntries(storage::IndexEntryReader& reader,
    // V3 format: meta is in __meta__ entry
    auto index_length = reader.GetMeta<size_t>(BITMAP_INDEX_LENGTH);
    total_num_rows_ = reader.GetMeta<size_t>(BITMAP_INDEX_NUM_ROWS);
-    valid_bitset_ = TargetBitmap(total_num_rows_, false);
+    valid_bitset_ = TargetBitmap(total_num_rows_, !schema_.nullable());
+    bool rebuild_validity_from_postings = schema_.nullable();
+
+    auto entry_names = reader.GetEntryNames();
+    if (std::find(entry_names.begin(),
+                  entry_names.end(),
+                  BITMAP_INDEX_VALID_BITSET) != entry_names.end()) {
+        auto valid_bitset_entry = reader.ReadEntry(BITMAP_INDEX_VALID_BITSET);
+        DeserializeValidBitsetData(valid_bitset_entry.data.data(),
+                                   valid_bitset_entry.data.size());
+        rebuild_validity_from_postings = false;
+    }

    auto data_entry = reader.ReadEntry(BITMAP_INDEX_DATA);

@@ -1349,9 +1425,12 @@ BitmapIndex<T>::LoadEntries(storage::IndexEntryReader& reader,
                      data_entry.data.data(),
                      data_entry.data.size(),
                      index_length,
-                      priority);
+                      priority,
+                      rebuild_validity_from_postings);
    } else {
-        DeserializeIndexData(data_entry.data.data(), index_length);
+        DeserializeIndexData(data_entry.data.data(),
+                             index_length,
+                             rebuild_validity_from_postings);
    }

    if (enable_offset_cache.has_value() && enable_offset_cache.value()) {
@@ -298,17 +298,36 @@ class BitmapIndex : public ScalarIndex<T> {
    void
    SerializeIndexData(uint8_t* index_data_ptr);

+    std::pair<std::shared_ptr<uint8_t[]>, size_t>
+    SerializeValidBitsetData() const;
+
    std::pair<std::shared_ptr<uint8_t[]>, size_t>
    SerializeIndexMeta();

    std::pair<size_t, size_t>
    DeserializeIndexMeta(const uint8_t* data_ptr, size_t data_size);

+    void
+    DeserializeValidBitsetData(const uint8_t* data_ptr, size_t data_size);
+
    T
    ParseKey(const uint8_t** ptr);

+    // Deserialize posting data.
+    //
+    // New bitmap index formats persist valid_bitset_, which is the
+    // authoritative source of row validity. Legacy formats do not, so we may
+    // rebuild validity from postings as a backward-compatibility fallback for
+    // nullable fields. Non-nullable fields do not persist valid_bitset_ and
+    // are treated as all-valid on load.
+    //
+    // Rebuilding validity from postings is lossy for ARRAY fields: empty
+    // arrays have no element postings, so they cannot be distinguished from
+    // null arrays during reconstruction.
    void
-    DeserializeIndexData(const uint8_t* data_ptr, size_t index_length);
+    DeserializeIndexData(const uint8_t* data_ptr,
+                         size_t index_length,
+                         bool rebuild_validity_from_postings);

    void
    BuildOffsetCache();
@@ -352,12 +371,24 @@ class BitmapIndex : public ScalarIndex<T> {
                 const T& upper_bound_value,
                 bool ub_inclusive);

+    // Build mmap-backed posting storage from serialized bitmap index data.
+    //
+    // New bitmap index formats persist valid_bitset_, which is the
+    // authoritative source of row validity. Legacy formats do not, so we may
+    // rebuild validity from postings as a backward-compatibility fallback for
+    // nullable fields. Non-nullable fields do not persist valid_bitset_ and
+    // are treated as all-valid on load.
+    //
+    // Rebuilding validity from postings is lossy for ARRAY fields: empty
+    // arrays have no element postings, so they cannot be distinguished from
+    // null arrays during reconstruction.
    void
    MMapIndexData(const std::string& filepath,
                  const uint8_t* data,
                  size_t data_size,
                  size_t index_length,
-                  milvus::proto::common::LoadPriority priority);
+                  milvus::proto::common::LoadPriority priority,
+                  bool rebuild_validity_from_postings);

    void
    UnmapIndexData();
@@ -32,6 +32,7 @@
 #include "common/Types.h"
 #include "common/protobuf_utils.h"
 #include "gtest/gtest.h"
+#include "index/BitmapIndex.h"
 #include "index/Index.h"
 #include "index/IndexFactory.h"
 #include "index/IndexInfo.h"
@@ -484,3 +485,281 @@ INSTANTIATE_TYPED_TEST_SUITE_P(ArrayBitmapE2ECheckV1,
 INSTANTIATE_TYPED_TEST_SUITE_P(ArrayBitmapE2ECheckV1,
                               ArrayBitmapIndexTestV2,
                               BitmapTypeV1);
+
+struct BitmapIndexArrayRegressionParam {
+    proto::schema::DataType element_type;
+    bool use_v3;
+    bool use_mmap;
+};
+
+class BitmapIndexArrayRegressionTest
+    : public testing::TestWithParam<BitmapIndexArrayRegressionParam> {
+ protected:
+    static constexpr int64_t collection_id_ = 1;
+    static constexpr int64_t partition_id_ = 2;
+    static constexpr int64_t segment_id_ = 3;
+    static constexpr int64_t field_id_ = 101;
+    static constexpr int64_t num_rows_ = 4;
+
+    std::string
+    ElementTypeName(proto::schema::DataType element_type) const {
+        switch (element_type) {
+            case proto::schema::DataType::Int32:
+                return "int32";
+            case proto::schema::DataType::String:
+                return "string";
+            default:
+                return "unknown";
+        }
+    }
+
+    std::vector<milvus::Array>
+    BuildArrayData(proto::schema::DataType element_type, bool use_mmap) const {
+        auto posting_count =
+            use_mmap ? DEFAULT_BITMAP_INDEX_BUILD_MODE_BOUND + 50 : 2;
+
+        std::vector<ScalarFieldProto> scalar_arrays(num_rows_);
+        switch (element_type) {
+            case proto::schema::DataType::Int32: {
+                for (int i = 0; i < posting_count; ++i) {
+                    scalar_arrays[1].mutable_int_data()->add_data(i);
+                }
+                scalar_arrays[2].mutable_int_data()->add_data(-1);
+                break;
+            }
+            case proto::schema::DataType::String: {
+                for (int i = 0; i < posting_count; ++i) {
+                    scalar_arrays[1].mutable_string_data()->add_data(
+                        fmt::format("s{}", i));
+                }
+                scalar_arrays[2].mutable_string_data()->add_data("ignored");
+                break;
+            }
+            default:
+                ThrowInfo(ErrorCode::DataTypeInvalid,
+                          "unsupported element type {} in regression test",
+                          proto::schema::DataType_Name(element_type));
+        }
+
+        std::vector<milvus::Array> array_data;
+        array_data.reserve(num_rows_);
+        for (const auto& scalar_array : scalar_arrays) {
+            array_data.emplace_back(scalar_array);
+        }
+        return array_data;
+    }
+
+    FieldDataPtr
+    BuildFieldData(proto::schema::DataType element_type, bool use_mmap) const {
+        auto array_data = BuildArrayData(element_type, use_mmap);
+        auto field_data =
+            storage::CreateFieldData(DataType::ARRAY, DataType::NONE, true);
+        uint8_t valid_data = 0x0B;  // rows 0,1,3 valid; row 2 null.
+        field_data->FillFieldData(
+            array_data.data(), &valid_data, array_data.size(), 0);
+        return field_data;
+    }
+
+    void
+    AssertNullSemantics(index::IndexBase* loaded_index,
+                        proto::schema::DataType element_type) const {
+        switch (element_type) {
+            case proto::schema::DataType::Int32: {
+                auto index_ptr =
+                    dynamic_cast<index::ScalarIndex<int32_t>*>(loaded_index);
+                ASSERT_NE(index_ptr, nullptr);
+                AssertNullSemanticsImpl(index_ptr);
+                break;
+            }
+            case proto::schema::DataType::String: {
+                auto index_ptr = dynamic_cast<index::ScalarIndex<std::string>*>(
+                    loaded_index);
+                ASSERT_NE(index_ptr, nullptr);
+                AssertNullSemanticsImpl(index_ptr);
+                break;
+            }
+            default:
+                FAIL() << "unsupported element type "
+                       << proto::schema::DataType_Name(element_type);
+        }
+    }
+
+    template <typename T>
+    void
+    AssertNullSemanticsImpl(index::ScalarIndex<T>* index_ptr) const {
+        auto is_null = index_ptr->IsNull();
+        auto is_not_null = index_ptr->IsNotNull();
+
+        ASSERT_EQ(is_null.size(), num_rows_);
+        ASSERT_EQ(is_not_null.size(), num_rows_);
+
+        EXPECT_FALSE(is_null[0]) << "empty array row should stay non-null";
+        EXPECT_TRUE(is_not_null[0]) << "empty array row should stay non-null";
+
+        EXPECT_FALSE(is_null[1]) << "non-empty array row should stay non-null";
+        EXPECT_TRUE(is_not_null[1])
+            << "non-empty array row should stay non-null";
+
+        EXPECT_TRUE(is_null[2]) << "null array row should stay null";
+        EXPECT_FALSE(is_not_null[2]) << "null array row should stay null";
+
+        EXPECT_FALSE(is_null[3]) << "empty array row should stay non-null";
+        EXPECT_TRUE(is_not_null[3]) << "empty array row should stay non-null";
+    }
+};
+
+TEST_P(BitmapIndexArrayRegressionTest,
+       NullableEmptyArrayPreservesValidityAfterReload) {
+    auto param = GetParam();
+    auto index_build_id = param.use_v3 ? 3001 : 3002;
+    auto index_version = index_build_id;
+
+    proto::schema::FieldSchema field_schema;
+    field_schema.set_data_type(proto::schema::DataType::Array);
+    field_schema.set_element_type(param.element_type);
+    field_schema.set_nullable(true);
+
+    auto field_meta = storage::FieldDataMeta{
+        collection_id_, partition_id_, segment_id_, field_id_, field_schema};
+    auto index_meta = storage::IndexMeta{
+        segment_id_, field_id_, index_build_id, index_version};
+
+    auto field_data = BuildFieldData(param.element_type, param.use_mmap);
+
+    std::string root_path =
+        fmt::format("{}/bitmap_empty_array_regression_{}_{}_{}",
+                    TestLocalPath,
+                    ElementTypeName(param.element_type),
+                    param.use_v3 ? "v3" : "binaryset",
+                    param.use_mmap ? "mmap" : "memory");
+    boost::filesystem::remove_all(root_path);
+
+    storage::StorageConfig storage_config;
+    storage_config.storage_type = "local";
+    storage_config.root_path = root_path;
+    auto chunk_manager = storage::CreateChunkManager(storage_config);
+    auto fs = storage::InitArrowFileSystem(storage_config);
+    storage::FileManagerContext ctx(field_meta, index_meta, chunk_manager, fs);
+
+    auto mmap_path = fmt::format("{}/bitmap_index.mmap", root_path);
+
+    if (param.use_v3) {
+        auto payload_reader =
+            std::make_shared<milvus::storage::PayloadReader>(field_data);
+        storage::InsertData insert_data(payload_reader);
+        insert_data.SetFieldDataMeta(field_meta);
+        insert_data.SetTimestamps(0, 100);
+
+        auto serialized_bytes = insert_data.Serialize(storage::Remote);
+        auto log_path = fmt::format("/{}/{}/{}/{}/{}/{}",
+                                    TestLocalPath,
+                                    collection_id_,
+                                    partition_id_,
+                                    segment_id_,
+                                    field_id_,
+                                    0);
+        chunk_manager->Write(
+            log_path, serialized_bytes.data(), serialized_bytes.size());
+
+        Config config;
+        config["index_type"] = milvus::index::BITMAP_INDEX_TYPE;
+        config[INSERT_FILES_KEY] = std::vector<std::string>{log_path};
+        config[INDEX_NUM_ROWS_KEY] = num_rows_;
+        config[milvus::index::SCALAR_INDEX_ENGINE_VERSION] = 3;
+
+        auto build_index =
+            indexbuilder::IndexFactory::GetInstance().CreateIndex(
+                DataType::ARRAY, config, ctx);
+        build_index->Build();
+
+        auto create_index_result = build_index->Upload();
+        ASSERT_GT(create_index_result->GetMemSize(), 0);
+        ASSERT_GT(create_index_result->GetSerializedSize(), 0);
+
+        index::CreateIndexInfo index_info{};
+        index_info.index_type = milvus::index::BITMAP_INDEX_TYPE;
+        index_info.field_type = DataType::ARRAY;
+
+        config["index_files"] = create_index_result->GetIndexFiles();
+        config[milvus::LOAD_PRIORITY] =
+            milvus::proto::common::LoadPriority::HIGH;
+        if (param.use_mmap) {
+            config[milvus::index::MMAP_FILE_PATH] = mmap_path;
+        }
+        ctx.set_for_loading_index(true);
+
+        auto loaded_index =
+            index::IndexFactory::GetInstance().CreateIndex(index_info, ctx);
+        loaded_index->LoadV3(config);
+        AssertNullSemantics(loaded_index.get(), param.element_type);
+    } else {
+        Config load_config;
+        if (param.use_mmap) {
+            load_config[milvus::index::MMAP_FILE_PATH] = mmap_path;
+        }
+
+        switch (param.element_type) {
+            case proto::schema::DataType::Int32: {
+                auto index = std::make_unique<index::BitmapIndex<int32_t>>(ctx);
+                index->BuildWithFieldData(
+                    std::vector<FieldDataPtr>{field_data});
+                auto binary_set = index->Serialize({});
+
+                auto loaded_index =
+                    std::make_unique<index::BitmapIndex<int32_t>>(ctx);
+                loaded_index->Load(binary_set, load_config);
+                AssertNullSemantics(loaded_index.get(), param.element_type);
+                break;
+            }
+            case proto::schema::DataType::String: {
+                auto index =
+                    std::make_unique<index::BitmapIndex<std::string>>(ctx);
+                index->BuildWithFieldData(
+                    std::vector<FieldDataPtr>{field_data});
+                auto binary_set = index->Serialize({});
+
+                auto loaded_index =
+                    std::make_unique<index::BitmapIndex<std::string>>(ctx);
+                loaded_index->Load(binary_set, load_config);
+                AssertNullSemantics(loaded_index.get(), param.element_type);
+                break;
+            }
+            default:
+                FAIL() << "unsupported element type "
+                       << proto::schema::DataType_Name(param.element_type);
+        }
+    }
+
+    boost::filesystem::remove_all(root_path);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+    BitmapIndexArrayRegression,
+    BitmapIndexArrayRegressionTest,
+    testing::Values(
+        BitmapIndexArrayRegressionParam{
+            proto::schema::DataType::Int32, true, false},
+        BitmapIndexArrayRegressionParam{
+            proto::schema::DataType::Int32, true, true},
+        BitmapIndexArrayRegressionParam{
+            proto::schema::DataType::Int32, false, false},
+        BitmapIndexArrayRegressionParam{
+            proto::schema::DataType::Int32, false, true},
+        BitmapIndexArrayRegressionParam{
+            proto::schema::DataType::String, true, false},
+        BitmapIndexArrayRegressionParam{
+            proto::schema::DataType::String, true, true},
+        BitmapIndexArrayRegressionParam{
+            proto::schema::DataType::String, false, false},
+        BitmapIndexArrayRegressionParam{
+            proto::schema::DataType::String, false, true}),
+    [](const testing::TestParamInfo<BitmapIndexArrayRegressionParam>& info) {
+        auto element_type =
+            info.param.element_type == proto::schema::DataType::Int32
+                ? "Int32"
+                : "String";
+        return fmt::format("{}_{}_{}",
+                           element_type,
+                           info.param.use_v3 ? "V3" : "BinarySet",
+                           info.param.use_mmap ? "MMap" : "Memory");
+    });
@@ -35,6 +35,7 @@ constexpr const char* BITMAP_INDEX_DATA = "bitmap_index_data";
 constexpr const char* BITMAP_INDEX_META = "bitmap_index_meta";
 constexpr const char* BITMAP_INDEX_LENGTH = "bitmap_index_length";
 constexpr const char* BITMAP_INDEX_NUM_ROWS = "bitmap_index_num_rows";
+constexpr const char* BITMAP_INDEX_VALID_BITSET = "valid_bitset";

 constexpr const char* INDEX_TYPE = "index_type";
 constexpr const char* METRIC_TYPE = "metric_type";