A reordered addIndexes can drop the stored fields and term vectors of the document after an empty input reader

When a merge reorders documents, IndexWriter.addIndexes(CodecReader...) first glues its inputs end to end into one merged view and copies every document through it. To find which input owns document N, the view binary-searches the inputs' start offsets. An input with maxDoc() == 0 starts where the next input starts, and the search can stop on the empty input. The first document of the next input is then read from the empty one: in the new index it has no stored fields and no term vectors, while its terms and doc values are intact, so searches still find it but it comes back empty. No error, CheckIndex is clean, and every release from 9.9.0 to 10.6.0 (10.6.0 by reading) and main is affected. The trigger is narrow: a reordering merge (opt-in) plus an empty input reader built in memory, which no real index produces.

• addIndexes(CodecReader...) copies documents from caller-supplied readers into a new segment of the target index.
• Reordering merge: a merge whose OneMerge#reorder returns a Sorter.DocMap that renumbers the documents, for example BPReorderingMergePolicy (misc module), which treats addIndexes as a forced merge ("addIndexes is considered a forced merge") and so reorders it whenever the merged view has a live document and the reorderer has enough memory.
• Merged view (SlowCompositeCodecReaderWrapper): for a reordering merge, IndexWriter glues the inputs into one CodecReader and copies every document through it (IndexWriter.java:3560-3565). It records where each input starts, docStarts, the running sum of maxDoc().

1. Inputs of 3, 0 and 2 documents give docStarts = [0, 3, 3, 5] (SlowCompositeCodecReaderWrapper.java:70-75): the empty input repeats the start 3.
2. docIdToReaderId (:100-107) calls Arrays.binarySearch(docStarts, doc) and corrects the result only on a miss (-2 - pos). For doc 3 the search probes index 1 first, finds 3, and returns 1, the empty input. The owner is input 2.
3. Only the view's stored-fields reader (prefetch, document, :156-165) and term-vectors reader (prefetch, get, :247-263) use this lookup. They ask the empty input for its local doc 0, which yields nothing for an empty ParallelLeafReader, so doc 3 is written with no stored fields and no term vectors.
4. Postings, doc values, norms and points are merged per input, live docs use ReaderUtil.subIndex (via MultiBits, :97), and vectors use binarySearchStarts (:1093-1106), which already scans to the last equal start. So b0 keeps its terms and doc values and is still found by its id.

Only a document whose id equals a repeated start can be misrouted, and only when the binary search's probes land on the empty input. Executed: [3, empty, 2] and [2, empty, empty, 2] lose b0; [2, empty, 2, empty, empty, 2] loses b0 and c0; the randomized differential lost doc 0 for [empty, 6, 4, 3]. A plain replay of the Java lookup shows [empty, 3, 2], [3, 2, empty] and [2, empty, 2, empty, 2] are not hit. A trailing empty input and an all-deleted input (numDocs 0, maxDoc 1) are not hit (executed). At most one document per run of repeated starts can be lost: only the document whose id equals that start is looked up there (by reading the lookup; an executed probe with two such runs lost exactly b0 and c0). The merge must also really be reordered: IndexWriter calls reorder only without an index sort (and, with blocks, only when a parent field is set), and BP reorders only when the view has a live document and enough memory.

addIndexes([a0, a1, a2], empty, [b0, b1]) with BPReorderingMergePolicy or an identity reorder: 5 documents whose stored ids, in document order, are a0, a1, a2, null, b1 (doc 3 lost); the repro fails with AssertionError: doc 3 lost its stored fields. The same call without reordering keeps all 5.

• Who is exposed. Applications that reorder addIndexes(CodecReader...) merges (BPReorderingMergePolicy from the misc module, or a custom @lucene.experimental OneMerge#reorder) and pass an input reader with maxDoc() == 0 at a position the binary search lands on. The merge must really be reordered (no index sort; for BP a live document in the view and enough memory). Default merge policies do not reorder.
• How such an input arises. It must be built in memory: IndexWriter never writes a 0-document segment (SegmentMerger.java:101-103, PendingDeletes.java:88), so readers opened on real indexes cannot provide one. Examples: an empty ParallelLeafReader (upstream TestParallelReaderEmptyIndex passes exactly this to addIndexes), or a custom CodecReader that reports no documents. Segment merges cannot produce such an input unless a custom OneMerge#wrapForMerge returns a 0-document reader (not tested). No downstream caller of this shape was found in limited code searches (not exhaustive). Likelihood: rare.
• Silent and lasting. No exception with assertions on or off, and CheckIndex reports nothing (the test framework runs it on close; the diagram probe runs it explicitly). The committed target keeps the empty document, and by reasoning later merges copy it as it is. The document is still found by queries and keeps its doc values, but fetching, highlighting or re-indexing from stored fields gets nothing for it.
• Bounded. At most one document per run of repeated starts (by reading the lookup). In the executed probe no other document's data is substituted, deleted documents stay deleted, and the tested term queries and ACL filter return the same hits as without reordering. By reasoning: addIndexes only reads its inputs, so a fresh target can be rebuilt from them while they exist (with the fix, or without the empty input); repairing an existing target means replacing the affected documents, since a second addIndexes into it adds another copy.
• Caveat. The silent form was shown with an empty ParallelLeafReader. Another empty reader whose stored-fields or term-vectors reader rejects local doc 0 could make the merge throw instead (not tested).

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => java.lang.AssertionError: documents with a stored id, of maxDoc 2 expected:<2> but was:<1>
       org.junit.Assert.fail(Assert.java:89)
       org.junit.Assert.failNotEquals(Assert.java:835)
       org.junit.Assert.assertEquals(Assert.java:647)
       org.apache.lucene.misc.index.TestReorderedAddIndexesEmptyReader.testDocAfterEmptyReaderKeepsStoredFields(TestReorderedAddIndexesEmptyReader.java:59)

• Class javadoc: "A merged CodecReader view of multiple CodecReader" (SlowCompositeCodecReaderWrapper.java:44-45). A view whose document 3 is not input 3's first document is not a merged view of the inputs.
• OneMerge#reorder: "Extend this method if you wish to renumber doc IDs. This method will be called when index sorting is disabled on a merged view of the OneMerge" (MergePolicy.java:312-314). Renumbering must not lose content.
• addIndexes(CodecReader...): "NOTE: empty segments are dropped by this method and not added to this index" (IndexWriter.java:3262). No code rejects a maxDoc() == 0 input (validateMergeReader checks versions and the index sort only, :3228-3252), and TestParallelReaderEmptyIndex adds SlowCodecReaderWrapper.wrap(new ParallelLeafReader()) (:50). LUCENE-1918 (2009) already fixed exceptions when an empty ParallelReader was passed to addIndexes.
• Consistency. The vector lookup in the same class scans to the last repeated start: "Arrays.binarySearch can return any of a sequence of repeated value but we always want the last one" (:1099-1103, added by f21ec8414c1). ReaderUtil.subIndex, used for the same job by MultiBits, scans to the last match too. It was added in 2009 (7fff5a7ea7c, LUCENE-1771) with the loop that 933bac2435e (2002, "Patched to fix a bug when empty indexes or segments exist") had added to the multi-reader lookups it descends from.
• History. The introducing PR #12622 never discusses empty readers. No javadoc, test, CHANGES or MIGRATE entry restricts reordered inputs to maxDoc() >= 1.
• Why tests missed it. TestParallelReaderEmptyIndex passes the empty reader alone, so no view is built (SlowCompositeCodecReaderWrapper.wrap returns a single reader as is, SlowCompositeCodecReaderWrapper.java:49-57); TestAddIndexes.testAddEmpty passes no readers; testFakeAllDeleted uses maxDoc 1; MockRandomMergePolicy overrides reorder but not findMerges(CodecReader...), so the randomized coverage that relies on it never reorders an addIndexes merge. The dedicated nightly test TestBPReorderingMergePolicy.testReorderOnAddIndexes does reorder one, but has no maxDoc() == 0 input.

The strongest case for "works as designed" (made by an intended-behaviour checker, and separately by a reviewer from another model family, who rated it weak): the view is package-private merge infrastructure ("primarily targeted at merging, not searching"), OneMerge#reorder is @lucene.experimental, and IndexWriter never writes 0-document segments (LUCENE-5173), so only synthetic empty inputs reach the lookup; on real inputs the starts are strictly increasing and the plain binary search is correct.

Why it fails: that limits how often it happens, not whether it is wrong. The loss happens during the merge, the view's own purpose, and experimental marks API instability, not permission to drop data. Empty inputs are accepted and tested upstream, the class's other doc-to-input lookups (live docs through ReaderUtil.subIndex, vectors through binarySearchStarts) already take the last equal start, and the fix is the one the vector path received. Verdict: not intended, a defect.

One fix was validated in a scratch worktree at a9779ef3c193; the alternatives are by reasoning.

Replace the raw Arrays.binarySearch hit with the existing helper that scans to the last equal start. ReaderUtil.subIndex exists back to 9.9.0, so the patch backports unchanged.

   private int docIdToReaderId(int doc) {
     Objects.checkIndex(doc, docStarts[docStarts.length - 1]);
-    int readerId = Arrays.binarySearch(docStarts, doc);
-    if (readerId < 0) {
-      readerId = -2 - readerId;
-    }
-    return readerId;
+    // docStarts has repeated values when a reader is empty: pick the last reader that starts at or
+    // before doc, which is the only one that can contain it
+    return ReaderUtil.subIndex(doc, docStarts);
   }

Trade-offs

• Same O(log n) search, plus a forward scan over runs of equal starts, which only exist when inputs are empty. No benchmark was run.
• A Z3 model of the lookup shows the scan-to-last rule correct for 2 to 6 inputs (query Q3 unsat).

What the checks validated

• Gradle, seed DEADBEEF, -ea and -da: 15 classes, 106 tests, 0 failures (3 skipped, as on stock): the 14 repro tests (the finder's, an independent reproducer's core and misc tests, the scope test) and the upstream suites misc TestBPReorderingMergePolicy, TestBPIndexReorderer, TestBpVectorReorderer and core TestGetVectorCountCodecWrappers, TestAddIndexes, TestParallelReaderEmptyIndex, TestSortingCodecReader, TestIndexWriterMerging, TestIndexWriterForceMerge, TestMultiDocValues, TestStressIndexing2. On stock main the same set, run with assertions on, has 8 failures, all in repro classes.
• A separate nightly-enabled run: the repro and the three BP suites, 22 tests including the @Nightly testReorderOnAddIndexes, 0 failures, 0 skipped (the log lists the tests, not the command line).
• :lucene:core:check :lucene:misc:check -x test with the fix and the repro test: build successful.
• On the 9.9.0 and 10.5.2 release jars, the release's own class with only this method changed passes the repro; recompiled unchanged it still fails.

Keep Arrays.binarySearch and, on an exact hit, advance while the next start equals doc, as binarySearchStarts does.

Trade-offs

Same behaviour as the recommended fix with a few more lines, duplicating a helper that already exists. Not run separately.

Build docStarts and the per-input arrays only from inputs with maxDoc() > 0, keeping all inputs for the merged FieldInfos.

Trade-offs

Removes repeated starts at the source, for this lookup and any later one, but touches the constructor and every per-input array. A design choice for the maintainers; the recommended fix is the smallest change for this bug. Not run.

Throw for an empty input reader.

Trade-offs

Changes accepted behaviour that upstream TestParallelReaderEmptyIndex relies on. Not run.

Drop inputs with maxDoc() == 0 before calling addIndexes.

Trade-offs

By reasoning, not executed. They contribute no documents, though a custom empty reader could carry field definitions.

Method: SMT solving (Z3, bit-vector logic) of one correctness obligation (the study's "formal" tier). The hunt unit on CodecReader wrappers modelled docIdToReaderId with exact Java int arithmetic and the JDK's Arrays.binarySearch loop, and asked for 2 to 6 inputs: can the returned input fail to contain the document?

• Q1, every input has maxDoc >= 1: unsat for n = 2 to 6 (the lookup is correct).
• Q2, inputs may be empty: sat from n = 3; in every witness the document sits at a start shared by an empty input and the next input, and the chosen input has maxDoc 0 (for n = 3: inputs [a, 0, b], document a). An exhaustive plain replay over maxDoc <= 4, n <= 5 found 1,320 of 37,100 lookups wrong, every one on an empty input.
• Q3, the scan-to-last fix: unsat (the fix is complete for the modelled sizes).

The obligation was posed separately with and without empty inputs, so empty inputs were in the modeller's mind; the failing document and placement came from the solver. By file times the solver log predates the unit's randomized differential harness, which then flagged the same stored-field mismatches independently (through the view and through addIndexes with an identity reorder, only with empty inputs), and the public-API JUnit test.