SegmentOrder.reorder() throws NullPointerException when a leaf has no core cache helper

SegmentOrder (new in 10.4.0, #15591) re-orders a reader's segments so that the segments with the best values of a numeric sort are searched first. With an INT, LONG, FLOAT or DOUBLE primary sort, reorder() sorts the leaves with a comparator that memoises each leaf's min/max value under reader.getCoreCacheHelper().getKey() (SegmentOrder.java:137) without checking for null. LeafReader#getCoreCacheHelper documents null as a legal answer, and ParallelLeafReader over more than one reader, SortingCodecReader and MemoryIndex return it, as content-altering FilterLeafReaders typically do. So reorder() on a reader with two or more leaves, one of them non-cacheable, throws NullPointerException inside the BaseCompositeReader constructor before any view exists. Every other consumer of a leaf's core cache helper in the main sources checks for null; LRUQueryCache and QueryBitSetProducer then compute without caching. No wrong results; 10.4.0 to 10.6.0 and main are affected, and it is not fixed upstream.

1. One comparator per SegmentOrder. fromSort maps an INT, LONG, FLOAT or DOUBLE primary sort (also through SortedNumericSortField) to numericSorter (SegmentOrder.java:42-77); any other type returns the reader unchanged (:79-85). numericSorter creates one NumericFieldReaderContextComparator (:91-92) whose HashMap memo is never evicted (:115).
2. reorder() sorts in a constructor. It copies the leaves into a new array and hands it, with the comparator, to a BaseCompositeReader (SegmentOrder.java:95-98), whose constructor first runs Arrays.sort on it (BaseCompositeReader.java:74-77).
3. The unguarded key. compare calls getSortValue for both leaves (SegmentOrder.java:130-134), which starts with IndexReader.CacheKey key = reader.getCoreCacheHelper().getKey(); (:137). A leaf whose helper is null throws there, before loadSortValue (:144-172) reads any field. Every recorded stack (all from descending sorts; an ascending sort goes through :133): getSortValue:137 <- compare:132 <- Arrays.sort <- BaseCompositeReader.<init>:76 <- reorder:97.
4. null is a documented answer. LeafReader#getCoreCacheHelper: "A return value of null indicates that this reader is not suited for caching, which is typically the case for short-lived wrappers that alter the content of the wrapped leaf reader" (LeafReader.java:72-74). It is returned by ParallelLeafReader unless it has exactly one parallel reader that is also its only stored-fields reader (ParallelLeafReader.java:300-311), so by every leaf of a ParallelCompositeReader over two or more readers; by SortingCodecReader ("no caching on sorted views", SortingCodecReader.java:848-852, and :460-463); by MemoryIndex's reader (MemoryIndex.java:2296-2299); and by FilterLeafReaders that choose to (FilterLeafReader.java:40-42 only suggests delegating the helpers when a wrapper does not change the content).

• It needs a numeric primary sort, at least two leaves (Arrays.sort never calls the comparator for one element) and at least one compared leaf without a core cache helper.
• A single leaf, a non-numeric sort or a plain DirectoryReader never fails; the control re-orders a 2-segment index to leaf maxima [109, 9].
• The sort field and the data play no role: 4 numeric types x {an indexed field, a field absent from the index} x both directions gave the same NPE in all 16 cases.
• Deterministic: 20 of 20 iterations fail.

• Who is exposed. Callers of SegmentOrder.fromSort(<INT|LONG|FLOAT|DOUBLE sort>).reorder(reader) whose reader has two or more leaves and at least one leaf without a core cache helper. Per-request wrappers are the typical case: LUCENE-7410 (#8463), which made the helper nullable, describes applications that "wrap it per request depending on the logged user and throw away the wrapper once the request is completed".
• Default vs opt-in. Opt-in: nothing in Lucene calls SegmentOrder except its own test; plain DirectoryReaders are never affected.
• Effect. The call throws before any view exists. The caller gets no reader; the input reader and its reference count are unchanged (executed probe). Such an application cannot use numeric segment reordering on those readers (a non-numeric sort or a single leaf never reaches the comparator); it has to order the leaves itself. A working re-ordered view would hold the same documents with different composite document ids, since the ids follow the new leaf order (BaseCompositeReader.java:81-87); that is by design.
• Scale. Two leaves with one value each are enough.
• Reach. A read-only code search found no use in Elasticsearch, OpenSearch or Solr (not exhaustive). The failure is immediate whenever this path executes; how many applications reach it is unmeasured. Likelihood (registry): rare.

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => java.lang.AssertionError: expected the leaves ordered by f descending (2, 1), got java.lang.NullPointerException: Cannot invoke "org.apache.lucene.index.IndexReader$CacheHelper.getKey()" because the return value of "org.apache.lucene.index.LeafReader.getCoreCacheHelper()" is null
       org.apache.lucene.index.TestSegmentOrderNullCoreCacheHelper.testReorderLeavesWithoutCoreCacheHelper(TestSegmentOrderNullCoreCacheHelper.java:41)
     Caused by: java.lang.NullPointerException: Cannot invoke "org.apache.lucene.index.IndexReader$CacheHelper.getKey()" because the return value of "org.apache.lucene.index.LeafReader.getCoreCacheHelper()" is null
       org.apache.lucene.index.SegmentOrder$NumericFieldReaderContextComparator.getSortValue(SegmentOrder.java:137)
       org.apache.lucene.index.SegmentOrder$NumericFieldReaderContextComparator.compare(SegmentOrder.java:132)
       org.apache.lucene.index.SegmentOrder$NumericFieldReaderContextComparator.compare(SegmentOrder.java:113)
       java.base/java.util.TimSort.countRunAndMakeAscending(TimSort.java:355)
       java.base/java.util.TimSort.sort(TimSort.java:220)
       [...]

• Contract. LeafReader#getCoreCacheHelper: "Optional method ... A return value of null indicates that this reader is not suited for caching" (LeafReader.java:65-74). SegmentOrder#reorder: "Produces a new view over an IndexReader by re-ordering the reader's segments" (SegmentOrder.java:33), no restriction on the reader; it declares only throws IOException (:34) and documents no unchecked exception; fromSort says only that a non-numeric primary sort "will have no effect" (:39-40).
• What null is meant to do. In LUCENE-7410 (#8463), Adrien Grand (@jpountz) wrote: "the query cache and BitSetProducer for joins skip caching when LeafReader.getCoreCacheHelper returns null. Other APIs like segment-based replication or FieldCache fail with an exception since not being able to cache is a problem/performance trap in those cases." LRUQueryCache skips caching (LRUQueryCache.java:827-831, :914-918); QueryBitSetProducer computes without caching (guarded lookup QueryBitSetProducer.java:63-68, guarded cache.put :84-86).
• The only unguarded consumer. Outside wrappers that just delegate, the main sources use a leaf's core cache helper in six places: LRUQueryCache (twice), QueryBitSetProducer and two test-framework classes (PerThreadPKLookup, AssertingLeafReader) all check for null; SegmentOrder.java:137 calls .getKey() on it directly.
• History. #15591 (@romseygeek, merged 2026-01-23) added the memo as written; its body, review comments and reviews never mention cache helpers, null or wrapper readers. Its closed predecessor #15436 memoised by LeafReaderContext.ord. TestSegmentReordering opens its reader with DirectoryReader.open(dir), whose leaves always have a helper.
• No test relies on it. With the fix, TestSegmentReordering and the whole lucene/memory suite pass.

Strongest case (from the verifiers and the adjudication): LUCENE-7410 lets APIs for which "not being able to cache is a problem" refuse leaves without a helper; SegmentOrder memoises per core key, so it is such an API, and it was written for plain DirectoryReaders.

Why it fails: the APIs that refuse do it on purpose with a descriptive exception (for example "StandardDirectoryReader must support caching", SegmentInfosSearcherManager.java:123-126), never with an NPE; SegmentOrder documents no such restriction and accepts any IndexReader; its memo only saves a per-leaf doc-values-skipper or point min/max lookup (SegmentOrder.java:144-172), so computing without it gives the same order (the fix passes the repro and TestSegmentReordering); and for non-numeric sorts the class already degrades to a no-op instead of rejecting input. No verifier rated the intended argument better than weak. To concede: an explicit, documented rejection of such readers would be a legitimate design choice (LUCENE-7410 allows it; the refuse option under Fixes); what is hard to defend is the current accidental, undocumented NPE.

The recommended fix was applied to 645d4fe14a20 with git apply in a fresh scratch worktree and run through gradle. A second option also removes the NPE but keeps short-lived wrappers alive; two alternatives were not run.

In NumericFieldReaderContextComparator.getSortValue (SegmentOrder.java:136-142), read the helper once; when it is null, return loadSortValue(reader) directly; otherwise memoise under cacheHelper.getKey() as today.

    private long getSortValue(LeafReader reader) {
      IndexReader.CacheHelper cacheHelper = reader.getCoreCacheHelper();
      if (cacheHelper == null) {
        // not suited for caching: compute the value without memoizing it, so that short-lived
        // wrappers are not kept alive by this (never evicted) map
        return loadSortValue(reader);
      }
      IndexReader.CacheKey key = cacheHelper.getKey();
      if (cachedSortValues.containsKey(key) == false) {
        cachedSortValues.put(key, loadSortValue(reader));
      }
      return cachedSortValues.get(key);
    }

Trade-offs

• Does what LRUQueryCache and QueryBitSetProducer do when the helper is null: compute without caching.
• Cost: for a leaf without a helper, loadSortValue (a doc-values-skipper or point min/max lookup) runs on every comparison that involves it, so up to O(n log n) such lookups for n leaves per reorder() (TimSort makes between n - 1 and O(n log n) comparisons, depending on the input), instead of at most one per leaf. Cacheable leaves are unchanged. No benchmark was run.

What the checks validated

• Gradle, fresh scratch worktree of 645d4fe14a20 + the test (2026-10-30): stock, the repro fails 1 of 1 with assertions on and off; + fix, :lucene:memory:test (whole module) and TestSegmentReordering pass with assertions on and off: 5 suites, 55 tests, 0 failures each time (the repro 1, TestMemoryIndex 30, TestMemoryIndexAgainstDirectory 16, TestSlicedIntBlockPool 5, TestSegmentReordering 3).
• Earlier the same day, + fix: :lucene:core:check :lucene:memory:check -x test (google-java-format, ecjLint, forbiddenApis) passes with the test in the tree.
• Standalone runner with the fixed class shadowed: the repro passes; earlier the finder's repro, a verifier's 4 tests and TestSegmentReordering passed with it (8 of 8), and the re-ordered wrappers were garbage-collected.
• Not run: the full Lucene test suite, nightly tests, any benchmark, and any post-fix test of an INT, FLOAT or DOUBLE sort over leaves without a helper (every post-fix null-helper test uses LONG; all four types go through the same getSortValue, and TestSegmentReordering covers all four on cacheable leaves).

The finder's sketch: Object key = cacheHelper == null ? reader : cacheHelper.getKey();.

Trade-offs

Removes the NPE and passes the finder's repro and TestSegmentReordering, but the memo lives as long as the SegmentOrder and is never evicted, so every non-cacheable wrapper ever re-ordered stays strongly reachable: in a probe two re-ordered wrappers stayed reachable after repeated GC while an identical control wrapper was collected. Those are exactly the short-lived wrappers the null helper exists for; growth beyond the two probed wrappers is inferred from the never-evicted map, not measured.

What the checks validated

Finder's repro 1 of 1 and TestSegmentReordering 3 of 3 pass; retention probe: retained.

Compute each leaf's value once per reorder() call (for example into a local identity map or an array before sorting) for leaves without a helper. Not run.

Trade-offs

Avoids both the repeated lookups of the recommended fix and the retention of the key-by-leaf fix, but changes how the comparator is built (today one comparator per SegmentOrder is passed to the BaseCompositeReader constructor), so it is a larger change.

Check the helpers up front and throw IllegalArgumentException with a clear message; document it. Not run.

Trade-offs

Matches LUCENE-7410's "fail with an exception" option, but correctness does not need it: without the memo the same order comes out. The cost of the extra lookups was not measured, and other LeafReader implementations may make the min/max lookup more expensive than the readers tested here.

Method: non-formal (code reading, then a directed JUnit), a hunt unit on query-cache correctness, 2026-10-26, main ea57360912ed. The unit listed every getCoreCacheHelper() and getReaderCacheHelper() consumer in all module sources. LRUQueryCache, QueryBitSetProducer and the others check for null; SegmentOrder.java:137 does not. The finder's JUnit wraps two 2-segment indexes in ParallelCompositeReader(false, r1, r2), asserts that the leaves have a null helper, and re-orders with a descending LONG sort: it fails 1 of 1 with -ea and -da, 20 of 20 over 20 iterations, and on 10.4.0 and 10.5.2. A later minimisation step replaced it with the two-leaf MemoryIndex test shown here.