diff --git a/internal/core/src/index/BitmapIndex.cpp b/internal/core/src/index/BitmapIndex.cpp index 2b446000c1..076bfbea86 100644 --- a/internal/core/src/index/BitmapIndex.cpp +++ b/internal/core/src/index/BitmapIndex.cpp @@ -231,6 +231,21 @@ BitmapIndex::SerializeIndexData(uint8_t* data_ptr) { } } +template +std::pair, size_t> +BitmapIndex::SerializeValidBitsetData() const { + size_t valid_bitset_size = (total_num_rows_ + 7) / 8; + std::shared_ptr 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 std::pair, size_t> BitmapIndex::SerializeIndexMeta() { @@ -263,6 +278,24 @@ BitmapIndex::SerializeIndexData(uint8_t* data_ptr) { } } +template +void +BitmapIndex::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 BinarySet BitmapIndex::Serialize(const Config& config) { @@ -279,6 +312,11 @@ BitmapIndex::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::ChooseIndexLoadMode(int64_t index_length) { template void BitmapIndex::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::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::BuildOffsetCache() { template <> void -BitmapIndex::DeserializeIndexData(const uint8_t* data_ptr, - size_t index_length) { +BitmapIndex::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::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::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::MMapIndexData(const std::string& file_name, roaring::Roaring value; value = roaring::Roaring::read(reinterpret_cast(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::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::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::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::LoadEntries(storage::IndexEntryReader& reader, // V3 format: meta is in __meta__ entry auto index_length = reader.GetMeta(BITMAP_INDEX_LENGTH); total_num_rows_ = reader.GetMeta(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::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()) { diff --git a/internal/core/src/index/BitmapIndex.h b/internal/core/src/index/BitmapIndex.h index e435d4d374..4f744d9953 100644 --- a/internal/core/src/index/BitmapIndex.h +++ b/internal/core/src/index/BitmapIndex.h @@ -298,17 +298,36 @@ class BitmapIndex : public ScalarIndex { void SerializeIndexData(uint8_t* index_data_ptr); + std::pair, size_t> + SerializeValidBitsetData() const; + std::pair, size_t> SerializeIndexMeta(); std::pair 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 { 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(); diff --git a/internal/core/src/index/BitmapIndexArrayTest.cpp b/internal/core/src/index/BitmapIndexArrayTest.cpp index 70aa9ffa0c..e8f6acbbb8 100644 --- a/internal/core/src/index/BitmapIndexArrayTest.cpp +++ b/internal/core/src/index/BitmapIndexArrayTest.cpp @@ -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 { + 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 + 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 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 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*>(loaded_index); + ASSERT_NE(index_ptr, nullptr); + AssertNullSemanticsImpl(index_ptr); + break; + } + case proto::schema::DataType::String: { + auto index_ptr = dynamic_cast*>( + loaded_index); + ASSERT_NE(index_ptr, nullptr); + AssertNullSemanticsImpl(index_ptr); + break; + } + default: + FAIL() << "unsupported element type " + << proto::schema::DataType_Name(element_type); + } + } + + template + void + AssertNullSemanticsImpl(index::ScalarIndex* 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(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{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>(ctx); + index->BuildWithFieldData( + std::vector{field_data}); + auto binary_set = index->Serialize({}); + + auto loaded_index = + std::make_unique>(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>(ctx); + index->BuildWithFieldData( + std::vector{field_data}); + auto binary_set = index->Serialize({}); + + auto loaded_index = + std::make_unique>(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& 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"); + }); diff --git a/internal/core/src/index/Meta.h b/internal/core/src/index/Meta.h index fc6847ee84..c5a5ef507e 100644 --- a/internal/core/src/index/Meta.h +++ b/internal/core/src/index/Meta.h @@ -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";