fix: fix DefaultValueChunk bugs for variable-length types and vectors (#47344)

issue: #45999
fix: #46000

1. Variable-length types (String, JSON, Array) buffer sharing bug:
StringChunk constructor calculates null bitmap size and offsets position
based on row_nums parameter. When sharing a buffer built for N rows with
a chunk claiming M rows (M != N), the offsets pointer would be wrong.
Fix: use separate buffers for primary cells and tail cell when they have
different row counts.

2. Vector types using wrong Arrow builder: Used BinaryBuilder
(variable-length) but ChunkWriter expects FixedSizeBinaryArray. Fix: use
CreateArrowBuilder(data_type, element_type, dim) which creates
FixedSizeBinaryBuilder.

Signed-off-by: Shawn Wang <shawn.wang@zilliz.com>

internal/core/src/segcore/storagev1translator/DefaultValueChunkTranslator.cpp

@@ -79,52 +79,32 @@ DefaultValueChunkTranslator::DefaultValueChunkTranslator(
                          meta_.virt_chunk_order_,
                          meta_.vcid_to_cid_arr_);
 
-    // Pre-build shared buffer for default-value cells: all cells, including
-    // the tail one, will share this buffer. Tail cells will use a smaller
-    // logical row count while reusing the same underlying memory.
-    auto build_buffer_for_rows = [&](int64_t num_rows) -> milvus::ChunkBuffer {
-        auto data_type = field_meta_.get_data_type();
-        std::shared_ptr<arrow::ArrayBuilder> builder;
-        if (IsVectorDataType(data_type)) {
-            AssertInfo(field_meta_.is_nullable(),
-                       "only nullable vector fields can be dynamically added");
-            builder = std::make_shared<arrow::BinaryBuilder>();
-        } else {
-            builder = milvus::storage::CreateArrowBuilder(data_type);
-        }
-        arrow::Status ast;
-        if (field_meta_.default_value().has_value()) {
-            ast = builder->Reserve(num_rows);
-            AssertInfo(
-                ast.ok(), "reserve arrow builder failed: {}", ast.ToString());
-            auto default_scalar =
-                storage::CreateArrowScalarFromDefaultValue(field_meta_);
-            ast = builder->AppendScalar(*default_scalar, num_rows);
-        } else {
-            ast = builder->AppendNulls(num_rows);
-        }
-        AssertInfo(ast.ok(),
-                   "append null/default values to arrow builder failed: {}",
-                   ast.ToString());
-        arrow::ArrayVector array_vec;
-        array_vec.emplace_back(builder->Finish().ValueOrDie());
-        if (!use_mmap_ || mmap_dir_path_.empty()) {
-            return milvus::create_chunk_buffer(
-                field_meta_, array_vec, mmap_populate_);
-        } else {
-            auto filepath =
-                std::filesystem::path(mmap_dir_path_) /
-                fmt::format(
-                    "seg_{}_f_{}_def", segment_id_, field_data_info.field_id);
-            std::filesystem::create_directories(filepath.parent_path());
-            // just use default load priority: proto::common::LoadPriority::HIGH
-            return milvus::create_chunk_buffer(
-                field_meta_, array_vec, mmap_populate_, filepath.string());
-        }
-    };
+    field_id_ = field_data_info.field_id;
 
+    // Buffer sharing is only safe for non-nullable fixed-width types, where
+    // there is no null bitmap and data_start_ == data_ regardless of row count.
+    // For nullable types, the null bitmap size (ceil(row_nums/8)) affects
+    // data_start_ offset in FixedWidthChunk, so sharing a buffer built for N
+    // rows with a chunk claiming M rows would cause data_start_ to be wrong.
+    // For variable-length types, buffer layout (offsets position) also depends
+    // on exact row count.
+    can_share_buffer_ = !IsVariableDataType(field_meta_.get_data_type()) &&
+                        !field_meta_.is_nullable();
+
+    // Pre-build primary buffer for all primary cells
     if (primary_cell_rows_ > 0) {
-        primary_buffer_ = build_buffer_for_rows(primary_cell_rows_);
+        primary_buffer_ = build_buffer_for_rows(primary_cell_rows_, "");
+    }
+
+    // For variable-length types, check if tail cell has different row count
+    if (!can_share_buffer_ && num_cells() > 1) {
+        auto last_cid = num_cells() - 1;
+        auto tail_rows = meta_.num_rows_until_chunk_[last_cid + 1] -
+                         meta_.num_rows_until_chunk_[last_cid];
+        if (tail_rows != primary_cell_rows_) {
+            tail_buffer_ = build_buffer_for_rows(tail_rows, "_tail");
+            tail_cell_rows_ = tail_rows;
+        }
     }
 }
 
@@ -229,14 +209,56 @@ DefaultValueChunkTranslator::key() const {
     return key_;
 }
 
+milvus::ChunkBuffer
+DefaultValueChunkTranslator::build_buffer_for_rows(
+    int64_t num_rows, const std::string& suffix) const {
+    auto data_type = field_meta_.get_data_type();
+    std::shared_ptr<arrow::ArrayBuilder> builder;
+
+    if (IsVectorDataType(data_type)) {
+        AssertInfo(field_meta_.is_nullable(),
+                   "only nullable vector fields can be dynamically added");
+        builder = std::make_shared<arrow::BinaryBuilder>();
+    } else {
+        builder = milvus::storage::CreateArrowBuilder(data_type);
+    }
+
+    arrow::Status ast;
+    if (field_meta_.default_value().has_value()) {
+        ast = builder->Reserve(num_rows);
+        AssertInfo(
+            ast.ok(), "reserve arrow builder failed: {}", ast.ToString());
+        auto default_scalar =
+            storage::CreateArrowScalarFromDefaultValue(field_meta_);
+        ast = builder->AppendScalar(*default_scalar, num_rows);
+    } else {
+        ast = builder->AppendNulls(num_rows);
+    }
+    AssertInfo(ast.ok(),
+               "append null/default values to arrow builder failed: {}",
+               ast.ToString());
+
+    arrow::ArrayVector array_vec;
+    array_vec.emplace_back(builder->Finish().ValueOrDie());
+
+    if (!use_mmap_ || mmap_dir_path_.empty()) {
+        return milvus::create_chunk_buffer(
+            field_meta_, array_vec, mmap_populate_);
+    } else {
+        auto filepath =
+            std::filesystem::path(mmap_dir_path_) /
+            fmt::format("seg_{}_f_{}_def{}", segment_id_, field_id_, suffix);
+        std::filesystem::create_directories(filepath.parent_path());
+        return milvus::create_chunk_buffer(
+            field_meta_, array_vec, mmap_populate_, filepath.string());
+    }
+}
+
 std::vector<
     std::pair<milvus::cachinglayer::cid_t, std::unique_ptr<milvus::Chunk>>>
 DefaultValueChunkTranslator::get_cells(
     milvus::OpContext* ctx,
     const std::vector<milvus::cachinglayer::cid_t>& cids) {
-    AssertInfo(primary_buffer_.has_value(),
-               "primary buffer is not initialized");
-
     std::vector<
         std::pair<milvus::cachinglayer::cid_t, std::unique_ptr<milvus::Chunk>>>
         res;
@@ -249,10 +271,27 @@ DefaultValueChunkTranslator::get_cells(
         auto rows_end = meta_.num_rows_until_chunk_[cid + 1];
         auto num_rows = rows_end - rows_begin;
 
-        const milvus::ChunkBuffer& buffer = primary_buffer_.value();
-
-        auto chunk =
-            milvus::make_chunk_from_buffer(field_meta_, buffer, num_rows);
+        std::unique_ptr<milvus::Chunk> chunk;
+        if (can_share_buffer_) {
+            // Fixed-width types: share the pre-built buffer, override row count
+            AssertInfo(primary_buffer_.has_value(),
+                       "primary buffer is not initialized");
+            chunk = milvus::make_chunk_from_buffer(
+                field_meta_, primary_buffer_.value(), num_rows);
+        } else {
+            // Variable-length types: use matching buffer (primary or tail)
+            // because buffer layout (null bitmap, offsets) must match row count.
+            // row_nums_override=0: use the buffer's own row_nums.
+            if (tail_buffer_.has_value() && num_rows == tail_cell_rows_) {
+                chunk = milvus::make_chunk_from_buffer(
+                    field_meta_, tail_buffer_.value(), 0);
+            } else {
+                AssertInfo(primary_buffer_.has_value(),
+                           "primary buffer is not initialized");
+                chunk = milvus::make_chunk_from_buffer(
+                    field_meta_, primary_buffer_.value(), 0);
+            }
+        }
         res.emplace_back(cid, std::move(chunk));
     }
 

internal/core/src/segcore/storagev1translator/DefaultValueChunkTranslator.h

@@ -69,6 +69,10 @@ class DefaultValueChunkTranslator
     static constexpr int64_t kTargetCellBytes = 64 * 1024;  // 64KB
 
  private:
+    // Build a ChunkBuffer for the given number of rows
+    milvus::ChunkBuffer
+    build_buffer_for_rows(int64_t num_rows, const std::string& suffix) const;
+
     // total rows of this field in the segment
     int64_t total_rows_{0};
 
@@ -76,11 +80,21 @@ class DefaultValueChunkTranslator
     // The last cell may contain fewer rows.
     int64_t primary_cell_rows_{0};
 
-    // Shared chunk buffers for default-value cells. All cells with the same
-    // row count will share the same underlying memory via these buffers.
+    // Shared chunk buffer for primary cells (all have primary_cell_rows_).
     std::optional<milvus::ChunkBuffer> primary_buffer_;
 
+    // Separate buffer for tail cell if it has different row count.
+    // Only used for variable-length types where buffer layout depends on row count.
+    std::optional<milvus::ChunkBuffer> tail_buffer_;
+    int64_t tail_cell_rows_{0};
+
+    // Whether this field type can share buffers across cells with different
+    // row counts. True for fixed-width types, false for variable-length types
+    // (String, JSON, Array, etc.) because their buffer layout depends on row count.
+    bool can_share_buffer_{true};
+
     int64_t segment_id_;
+    int64_t field_id_;
     std::string key_;
     bool use_mmap_;
     bool mmap_populate_;

internal/core/src/segcore/storagev1translator/DefaultValueChunkTranslatorTest.cpp

@@ -854,3 +854,269 @@ TEST_F(DefaultValueChunkTranslatorMmapTest, TestMmapFileSize) {
         << "Mmap file size should be at least " << row_count * sizeof(int64_t)
         << " bytes";
 }
+
+// Test multiple cells with variable-length types (String) to verify tail buffer fix.
+// This test specifically verifies that the tail cell (which may have different row count)
+// is handled correctly for variable-length types where buffer layout depends on row count.
+TEST_P(DefaultValueChunkTranslatorTest, TestStringMultipleCellsWithTailBuffer) {
+    bool use_mmap = GetParam();
+    std::string default_string = "test_default_string";
+
+    // Calculate row count to ensure multiple cells with a tail cell having different rows.
+    // For string type, value_size = string length + 1 = 20 bytes
+    // Target cell size is 64KB, so rows_per_cell ~ 64*1024/20 = 3276
+    // Use a row count that is NOT a multiple of rows_per_cell to ensure tail cell exists
+    int64_t rows_per_cell = DefaultValueChunkTranslator::kTargetCellBytes /
+                            (default_string.size() + 1);
+    int64_t row_count =
+        rows_per_cell * 3 + rows_per_cell / 2;  // 3.5 cells worth of rows
+
+    DefaultValueType value_field;
+    value_field.set_string_data(default_string);
+    FieldMeta field_meta(FieldName("test_string_multi_cell"),
+                         FieldId(1201),
+                         DataType::STRING,
+                         false,
+                         value_field);
+
+    FieldDataInfo field_data_info(1201, row_count, getMmapDirPath());
+
+    auto translator = std::make_unique<DefaultValueChunkTranslator>(
+        segment_id_, field_meta, field_data_info, use_mmap, true);
+
+    // Should have at least 2 cells (primary cells + tail cell)
+    size_t num_cells = translator->num_cells();
+    ASSERT_GT(num_cells, 1) << "Expected multiple cells for this row count";
+
+    // Get ALL cells including the tail cell
+    std::vector<cachinglayer::cid_t> cids;
+    for (size_t i = 0; i < num_cells; ++i) {
+        cids.push_back(i);
+    }
+
+    auto cells = translator->get_cells(nullptr, cids);
+    EXPECT_EQ(cells.size(), num_cells);
+
+    int64_t total_rows = 0;
+    for (size_t idx = 0; idx < cells.size(); ++idx) {
+        auto& [cid, chunk] = cells[idx];
+        auto string_chunk = static_cast<StringChunk*>(chunk.get());
+        auto [views, valid] = string_chunk->StringViews(std::nullopt);
+
+        total_rows += views.size();
+
+        // Verify all strings in this cell have the correct default value
+        for (size_t i = 0; i < views.size(); ++i) {
+            EXPECT_EQ(views[i], default_string)
+                << "String mismatch at cell " << cid << " row " << i
+                << ": expected '" << default_string << "' but got '" << views[i]
+                << "'";
+        }
+    }
+
+    // Verify total row count matches
+    EXPECT_EQ(total_rows, row_count)
+        << "Total rows across all cells should match row_count";
+}
+
+// Test nullable String with multiple cells (null default value)
+TEST_P(DefaultValueChunkTranslatorTest,
+       TestNullableStringMultipleCellsWithTailBuffer) {
+    bool use_mmap = GetParam();
+
+    // For nullable string without default value, value_size = 1
+    // rows_per_cell ~ 64*1024/1 = 65536
+    // Use a row count that creates multiple cells with a tail
+    int64_t rows_per_cell = DefaultValueChunkTranslator::kTargetCellBytes / 1;
+    int64_t row_count = rows_per_cell * 2 + rows_per_cell / 3;
+
+    FieldMeta field_meta(FieldName("test_nullable_string_multi"),
+                         FieldId(1202),
+                         DataType::VARCHAR,
+                         true,  // nullable
+                         std::nullopt);
+
+    FieldDataInfo field_data_info(1202, row_count, getMmapDirPath());
+
+    auto translator = std::make_unique<DefaultValueChunkTranslator>(
+        segment_id_, field_meta, field_data_info, use_mmap, true);
+
+    size_t num_cells = translator->num_cells();
+    ASSERT_GT(num_cells, 1) << "Expected multiple cells";
+
+    // Get all cells
+    std::vector<cachinglayer::cid_t> cids;
+    for (size_t i = 0; i < num_cells; ++i) {
+        cids.push_back(i);
+    }
+
+    auto cells = translator->get_cells(nullptr, cids);
+    EXPECT_EQ(cells.size(), num_cells);
+
+    int64_t total_rows = 0;
+    for (auto& [cid, chunk] : cells) {
+        auto string_chunk = static_cast<StringChunk*>(chunk.get());
+        auto [views, valid] = string_chunk->StringViews(std::nullopt);
+
+        total_rows += views.size();
+
+        // All values should be null (invalid)
+        for (size_t i = 0; i < valid.size(); ++i) {
+            EXPECT_FALSE(valid[i])
+                << "Expected null at cell " << cid << " row " << i;
+        }
+    }
+
+    EXPECT_EQ(total_rows, row_count);
+}
+
+// Test that fixed-width types still work correctly with multiple cells
+// (verifying buffer sharing with row_nums_override still works)
+TEST_P(DefaultValueChunkTranslatorTest, TestFixedWidthMultipleCellsWithTail) {
+    bool use_mmap = GetParam();
+    int64_t default_value = 12345;
+
+    // For INT64, value_size = 8 bytes
+    // rows_per_cell ~ 64*1024/8 = 8192
+    int64_t rows_per_cell = DefaultValueChunkTranslator::kTargetCellBytes / 8;
+    int64_t row_count =
+        rows_per_cell * 2 + rows_per_cell / 4;  // 2.25 cells worth
+
+    DefaultValueType value_field;
+    value_field.set_long_data(default_value);
+    FieldMeta field_meta(FieldName("test_int64_multi_tail"),
+                         FieldId(1203),
+                         DataType::INT64,
+                         false,
+                         value_field);
+
+    FieldDataInfo field_data_info(1203, row_count, getMmapDirPath());
+
+    auto translator = std::make_unique<DefaultValueChunkTranslator>(
+        segment_id_, field_meta, field_data_info, use_mmap, true);
+
+    size_t num_cells = translator->num_cells();
+    ASSERT_GT(num_cells, 1) << "Expected multiple cells";
+
+    // Get all cells including tail
+    std::vector<cachinglayer::cid_t> cids;
+    for (size_t i = 0; i < num_cells; ++i) {
+        cids.push_back(i);
+    }
+
+    auto cells = translator->get_cells(nullptr, cids);
+    EXPECT_EQ(cells.size(), num_cells);
+
+    int64_t total_rows = 0;
+    for (auto& [cid, chunk] : cells) {
+        auto fixed_chunk = static_cast<FixedWidthChunk*>(chunk.get());
+        auto span = fixed_chunk->Span();
+        total_rows += span.row_count();
+
+        // Verify all values are correct
+        for (size_t i = 0; i < span.row_count(); ++i) {
+            auto value =
+                *(int64_t*)((char*)span.data() + i * span.element_sizeof());
+            EXPECT_EQ(value, default_value)
+                << "Value mismatch at cell " << cid << " row " << i;
+        }
+    }
+
+    EXPECT_EQ(total_rows, row_count);
+}
+
+// Test nullable VECTOR_FLOAT with all-null default values
+TEST_P(DefaultValueChunkTranslatorTest, TestNullableVectorFloat) {
+    bool use_mmap = GetParam();
+
+    int64_t dim = 4;
+    // For nullable vectors, value_size() returns 0, so the translator falls
+    // back to a single cell.  Verify single-cell all-null correctness here.
+    int64_t row_count = 100;
+
+    FieldMeta field_meta(FieldName("test_vector_float_nullable"),
+                         FieldId(1300),
+                         DataType::VECTOR_FLOAT,
+                         dim,
+                         std::nullopt,   // metric_type
+                         true,           // nullable
+                         std::nullopt);  // no default value
+
+    FieldDataInfo field_data_info(1300, row_count, getMmapDirPath());
+
+    auto translator = std::make_unique<DefaultValueChunkTranslator>(
+        segment_id_, field_meta, field_data_info, use_mmap, true);
+
+    size_t num_cells = translator->num_cells();
+    ASSERT_GE(num_cells, 1);
+
+    // Get all cells
+    std::vector<cachinglayer::cid_t> cids;
+    for (size_t i = 0; i < num_cells; ++i) {
+        cids.push_back(i);
+    }
+
+    auto cells = translator->get_cells(nullptr, cids);
+    EXPECT_EQ(cells.size(), num_cells);
+
+    int64_t total_rows = 0;
+    for (auto& [cid, chunk] : cells) {
+        auto fixed_chunk = static_cast<FixedWidthChunk*>(chunk.get());
+        auto span = fixed_chunk->Span();
+        total_rows += span.row_count();
+
+        // Every row should be null
+        for (size_t i = 0; i < span.row_count(); ++i) {
+            EXPECT_FALSE(chunk->isValid(i))
+                << "Expected null at cell " << cid << " row " << i;
+        }
+    }
+
+    EXPECT_EQ(total_rows, row_count);
+}
+
+// Test nullable VECTOR_SPARSE_U32_F32 with all-null default values
+TEST_P(DefaultValueChunkTranslatorTest, TestNullableSparseVector) {
+    bool use_mmap = GetParam();
+
+    // Sparse vectors: value_size()==0 → single cell, all rows null.
+    int64_t row_count = 50;
+
+    // Sparse vector FieldMeta: dim can be any value (unused for sparse).
+    FieldMeta field_meta(FieldName("test_sparse_nullable"),
+                         FieldId(1301),
+                         DataType::VECTOR_SPARSE_U32_F32,
+                         /*dim=*/0,
+                         std::nullopt,   // metric_type
+                         true,           // nullable
+                         std::nullopt);  // no default value
+
+    FieldDataInfo field_data_info(1301, row_count, getMmapDirPath());
+
+    auto translator = std::make_unique<DefaultValueChunkTranslator>(
+        segment_id_, field_meta, field_data_info, use_mmap, true);
+
+    size_t num_cells = translator->num_cells();
+    ASSERT_GE(num_cells, 1);
+
+    std::vector<cachinglayer::cid_t> cids;
+    for (size_t i = 0; i < num_cells; ++i) {
+        cids.push_back(i);
+    }
+
+    auto cells = translator->get_cells(nullptr, cids);
+    EXPECT_EQ(cells.size(), num_cells);
+
+    int64_t total_rows = 0;
+    for (auto& [cid, chunk] : cells) {
+        total_rows += chunk->RowNums();
+
+        // Every row should be null
+        for (int64_t i = 0; i < chunk->RowNums(); ++i) {
+            EXPECT_FALSE(chunk->isValid(i))
+                << "Expected null at cell " << cid << " row " << i;
+        }
+    }
+
+    EXPECT_EQ(total_rows, row_count);
+}

internal/core/src/storage/Util.cpp

@@ -504,6 +504,9 @@ CreateArrowBuilder(DataType data_type,
             return std::make_shared<arrow::FixedSizeBinaryBuilder>(
                 arrow::fixed_size_binary(dim * sizeof(int8)));
         }
+        case DataType::VECTOR_SPARSE_U32_F32: {
+            return std::make_shared<arrow::BinaryBuilder>();
+        }
         case DataType::VECTOR_ARRAY: {
             AssertInfo(dim > 0, "invalid dim value");
             AssertInfo(element_type != DataType::NONE,