LantaoJin opened a new pull request, #16830: URL: https://github.com/apache/lucene/pull/16830
### Description **This is a correctness bug.** The backward-codecs `OffHeapQuantizedByteVectorValues` caches one vector and its score correction, keyed by `lastOrd`. On a cache miss, `getScoreCorrectionConstant(j)` read `j`'s correction into the cached field without updating `lastOrd`. So after `vectorValue(i)` and `getScoreCorrectionConstant(j)`, a later `getScoreCorrectionConstant(i)` returned `j`'s correction. The compressed int4 scorer in `Lucene99ScalarQuantizedVectorScorer` reads only the correction in `score()`, while its `setScoringOrdinal()` calls `vectorValue()`. As a result, a supplier scorer silently returns wrong scores in two sequences: 1. `setScoringOrdinal(i)`, `score(j)`, `score(i)`: the self-score uses `j`'s correction. 2. `setScoringOrdinal(i)`, `score(j)`, `setScoringOrdinal(i)`: the query keeps `j`'s correction, so every later score is shifted. **Impact** - **Versions**: the cache bug exists since 9.11.0. Lucene's own scorer can reach it from 10.2.0 on, when the supplier scorer started to set and score through one values instance. That covers 10.2.x, 10.3.x, 10.4.0, 10.5.x and main. Versions 9.11 to 10.1 kept the query on a separate copy, so they are not affected. - **Formats**: `Lucene99HnswScalarQuantizedVectorsFormat` and `Lucene99ScalarQuantizedVectorsFormat` with 4 bits and `compress=true`, for `DOT_PRODUCT`, `COSINE` and `MAXIMUM_INNER_PRODUCT`, when the default `Lucene99ScalarQuantizedVectorScorer` does the scoring. In 10.2 and 10.3 it is the only scorer for these formats. From 10.4 on, it is used when the Panama off-heap scorer does not apply. Not affected: 7 bits, uncompressed 4 bits, `EUCLIDEAN`, query-time scoring (the query scorer never calls `vectorValue()`), and the off-heap scorer. - **Where it triggers**: only during graph building, through a supplier scorer. Merge-time link repair (10.4.0 and later) hits sequence 1. It calls `setScoringOrdinal(node)`, searches from the node's remaining neighbors, and scores the node itself when one of those neighbors links back. Repair then skips the node as its own neighbor, so the wrong score only changes which candidates stay in the beam. Ordinary insertion hits neither sequence. For 10.2.x and 10.3.x, where these formats can still be written, I found no path that hits either sequence, but I have not ruled out component connection. - **Lucene users**: these formats have been read-only since 10.4. On 10.4 and later, link repair reaches the bug only in tests (through the RW codecs) and in downstream projects that still write these formats. Searching an existing index is not affected. OpenSearch k-NN still writes these formats with its own copy of the legacy writer, but only with 7 bits, so it is not affected. - **Symptom**: no error. A merged graph can get slightly different neighbors; stored vectors and search scores do not change. **Fix** On a miss, `getScoreCorrectionConstant` now returns the value it reads from the slice, `Float.intBitsToFloat(slice.readInt())`, without touching the cache. `readInt` reads the same little-endian bytes as `readFloats`. The dense and sparse subclasses inherit the fix. I found this bug while writing the tests for #16829, which compare the Panama off-heap scorer with `Lucene99ScalarQuantizedVectorScorer`. Over the real `OffHeapQuantizedByteVectorValues`, the default scorer disagreed with the off-heap scorer on self-scores. The two bugs are in different classes and the fixes are independent, so the PRs can be merged in either order. The #16829 tests use their own uncached values class, so they do not depend on this fix. The same caching pattern exists in `getCorrectiveTerms` of Lucene104's `OffHeapScalarQuantizedVectorValues`, `OffHeapScalarQuantizedFloatVectorValues` and `OffHeapScalarQuantizedFloat16VectorValues`, and of Lucene102's `OffHeapBinarizedVectorValues`. Every in-tree caller reads `vectorValue()` first, so nothing is affected today. I left those classes for a follow-up so that this change stays in the legacy format. **Testing** This PR adds three tests to `TestLucene99ScalarQuantizedVectorScorer`. Each runs over hand-written `DenseOffHeapVectorValues` with corrections 1, 2 and 3. One covers the values-level sequence (compressed and uncompressed). The other two cover the two scorer sequences, and compare each result with a scorer from `supplier.copy()` that has nothing cached. | Code | Result | |---|---| | main | all three fail: `expected:<1.0> but was:<2.0>`, and both scores are off by exactly (c1 − c0) / 2 = 0.5 | | this PR | pass, also with `-Dtests.iters=20` | `./gradlew :lucene:backward-codecs:check -x test`, `./gradlew verifyEcLint` and the full `:lucene:backward-codecs:test` also pass. -- 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]
