[Bug]: EmbListOffset(lims, rows) drops trailing empty lists, producing a short offset meta

[foxspy]

Summary

EmbListOffset(const size_t* lims, size_t rows) infers the end of lims by scanning for the first element equal to rows. Empty lists do not advance the cumulative offset, so a run of empty lists at the tail of a segment appears in lims as a run of repeated values equal to rows — and the loop swallows all of them, pushing back only one.

The resulting EmbListOffset is short by exactly the number of trailing empty lists, which makes num_el() (and therefore ExternalCount(), added in #1793) smaller than the segment's real row count.

https://github.com/zilliztech/knowhere/blob/branch_v305/include/knowhere/emb_list_utils.h#L30-L41

EmbListOffset(const size_t* lims, size_t rows) {
    size_t idx = 0;
    assert(lims[idx] == 0);
    assert(rows > 0);
    while (lims[idx] < rows) {        // <-- infers array length from a sentinel value
        assert(idx == 0 || lims[idx] >= lims[idx - 1]);
        offset.push_back(lims[idx]);
        idx++;
    }
    assert(lims[idx] == rows);        // always true: the loop stops at the first match
    offset.push_back(lims[idx]);      // <-- only one entry is pushed back
}

The build path reaches it here:

• include/knowhere/index/index_node.h:424 — BuildEmbList(dataset, cfg, lims, dataset->GetRows(), ...); the 4th argument is the flattened vector count, used as the sentinel.
• src/index/index_node.cc:402 — EmbListOffset doc_offset(lims, num_rows);

Worked example

8 rows; rows 0–2 hold 2/3/2 vectors, rows 3–7 are empty lists. Total flattened vectors rows = 7.

lims (written by the caller, n+1 = 9 entries):
  [0, 2, 5, 7, 7, 7, 7, 7, 7]
        ^-- correct num_el = 8

loop: 0<7 push, 2<7 push, 5<7 push, 7<7 false -> stop at idx 3
      then push lims[3] = 7

offset = [0, 2, 5, 7]   ->  num_el() = 3     (expected 8, lost 5)

Formally: for n rows with t trailing empty lists, lims[n-t] … lims[n] all equal the total. The loop stops at index n-t, so offset ends up with n-t+1 entries and num_el() == n-t. The loss is exactly t. When the last row is non-empty, t == 0 and nothing is lost — which is why this stays dormant in the usual ColBERT-style workload where every row has vectors.

Impact

Once the offset meta is short, the index is short. Two distinct symptoms:

1. Filtered search fails hard. The caller passes a row-domain bitset sized to the segment's row count; Index<T>::Search compares it against ExternalCount() and returns Status::invalid_args:

   failed to search: invalid args: bitset size should be <= external count,
   but we get bitset size: 24808, external count: 24803
   

2. Unfiltered search silently under-recalls. When the filter matches everything, the caller passes an empty BitsetView, so the if (!bitset_.empty()) guard skips the check entirely and the search runs against an index that is missing those rows. No error, no warning.

The second one is the more dangerous of the two, since nothing surfaces it.

Also worth noting: both asserts in that constructor are plain assert, compiled out under NDEBUG — and even with them enabled neither would fire, because the loop stops precisely where lims[idx] == rows holds. The truncation is completely silent.

Reproduction

No compaction or large dataset needed. The only requirement is that the last rows of the segment have an empty list for the emb-list field.

1. Create a collection with an ARRAY<STRUCT{vector FLOAT_VECTOR(d), ...}> field, not nullable.
2. Insert N rows where the first rows carry vectors and the last k rows carry an empty array.
3. Flush, build the index with a MAX_SIM metric, load.
4. Search with an embedding-list query plus any scalar filter that matches a proper subset of rows.

Expected: bitset size: N, external count: N-k. An unfiltered search on the same collection succeeds but never returns the last k rows.

Note on nullable

This is not a nullable-handling bug — it is triggered specifically because the field is not nullable. When nullable is on, the caller skips invalid rows entirely, so they never enter lims and no repeated tail values are produced. With nullable off, an empty array is a legitimate zero-length list and must stay in lims, which is what collides with the sentinel scan.

Fix

A correct constructor already exists on main, introduced by #1673 (7cc9d4be), which takes the element count explicitly instead of scanning for a sentinel:

// include/knowhere/emb_list_utils.h (main)
EmbListOffset(const size_t* lims, size_t rows, size_t num_el) {
    offset.assign(lims, lims + num_el + 1);
    assert(offset.back() == rows);
    ...
}

// src/index/index_node.cc:464 (main)
EmbListOffset doc_offset(lims, num_rows, num_el);

But it is not on branch_v305:

$ git merge-base --is-ancestor 7cc9d4be 7d74caa0 ; echo $?
1                                   # not an ancestor

$ git show 7d74caa0:include/knowhere/emb_list_utils.h | grep -c 'size_t num_el) {'
0

Suggested actions:

1. Cherry-pick enhance: support nullable external id mapping #1673 to branch_v305, or at minimum thread num_el through BuildEmbList and switch to the 3-argument constructor.
2. Consider removing the 2-argument constructor outright — it cannot know the array length and is unsound whenever an empty list can appear.
3. Replace the two asserts with checks that survive release builds.

Please do not relax the bitset_.size() > ExternalCount() check as a workaround — that check is correct and is what caught this; loosening it would convert a hard failure into a silent wrong-result path.

Observed in production

• Milvus b0d70ae093, knowhere pin 7d74caa09 (i.e. carrying fix: validate embedding-list filter bitsets against external ID count #1793).
• Sealed compaction-output segment, 24808 rows, deltalog count = 0.
• All scalar indexes on the segment load with num_rows = 24808; the emb-list vector index reports ExternalCount() = 24803.
• Field is a non-nullable ARRAY<STRUCT{vector FLOAT_VECTOR(1152), path, model}>; 999 of the rows hold vectors and the rest hold empty arrays.
• Ruled out: NULLs (is null returns 0, is not null returns all rows), empty-list skipping at build (that would give 999, not 24803), and nullable-row compaction (the field carries no nullable flag).

#1793 is what made this diagnosable — switching the comparison from Count() (flattened vectors) to ExternalCount() (lists) changed the reported number from a meaningless data count: 5412 to external count: 24803, i.e. off by exactly the trailing empty run. It reported the problem accurately; it did not fix the artifact.