HoustonPutman commented on PR #16581: URL: https://github.com/apache/lucene/pull/16581#issuecomment-5515304630
> **3. The removal branch in `MergedSortedNumericDocValues` looks unreachable.** `updateSortedNumericDocValue` always sets a value and the `Field[]` path reads `f.numericValue()`, so nothing produces a sorted-numeric removal and `anyRemoval` is always false for this type. The `hasValue == false` skip loop is then dead. Either drop it (and assert `hasValue`), or keep it with a comment that it is currently unreachable, so it does not read as covered behavior. > **4. `advance` / `advanceExact` throw on a `DocIdSetIterator`.** Safe today (the codec flush and the default `intoBitSet` only walk `nextDoc`), but it is a latent trap and diverges from the numeric sibling that supports both via `MergedDocValues`. Either implement `advance` by delegating, or note "flush-only, forward iteration" so the next caller is not surprised. Same thought on overriding `intoBitSet` for parity with the numeric path. `MergedDocValues` also has both of the same issue, but I will just update the code to use MergedDocValues instead. > **2. The multi-valued-base merge is the trickiest new code and is barely covered.** `testRandom` and both fold tests start docs single-valued (`doc(i, i)`), so they take the `singletonOnDisk != null` numeric branch and never reach `MergedSortedNumericDocValues`. That class is only exercised by two small deterministic cases with no fold and no stacking. Seeding some genuinely multi-valued, un-updated docs in `testRandom` and running with a low `maxDocValuesOverlays` would drive the folded rewrite over a multi-valued base and check it against the model. Yeah, more tests were certainly needed. Added more. > 1. Interaction with #16570 (merge carry-over): sorted-numeric updates that resolve during a merge are dropped or mis-typed. I've merged and now it should support sorted-numeric updates. Also changed `IndexWriter.addDiskDiffToPacket()` to be per-dv-type. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
