Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
* Re-enable flaky BWC test testKNNWarmupCustomLegacyFieldMapping and restore legacy index settings in BWC test fixtures [#2415](https://github.com/opensearch-project/k-NN/issues/2415)

### Bug Fixes
* Return a non-matching explanation from `KNNWeight.explain()` for documents that are not nearest neighbors [#3480](https://github.com/opensearch-project/k-NN/pull/3480)
* Preserve raw non-XContent `_source` fields when derived source is enabled [#3402](https://github.com/opensearch-project/k-NN/pull/3402)
* Fix dimension-based oversampling not applying for 32x compression [#3455](https://github.com/opensearch-project/k-NN/pull/3455)
* Fix NPE in nested kNN search when index contains documents without nested object [#3368](https://github.com/opensearch-project/k-NN/pull/3368)
Expand Down
12 changes: 7 additions & 5 deletions src/main/java/org/opensearch/knn/index/query/KNNWeight.java
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,13 @@ public Explanation explain(LeafReaderContext context, int doc, float score) {
// calculate score only when its 0 as for disk-based search,
// score will be passed from the caller and there is no need to re-compute the score
if (score == 0) {
score = getKnnScore(knnScorer, doc);
// explain() can be called for a document this query does not match, for instance when the knn clause is
// one of several should clauses. Reporting a match for such a document breaks the contract that
// Weight.explain() and the scorer of the same Weight have to agree on which documents match.
if (knnScorer.iterator().advance(doc) != doc) {
return Explanation.noMatch("the document is not a nearest neighbor result for the field [" + knnQuery.getField() + "]");
}
score = knnScorer.score();
}
} catch (IOException e) {
throw new RuntimeException(String.format("Error while explaining KNN score for doc [%d], score [%f]", doc, score), e);
Expand Down Expand Up @@ -273,10 +279,6 @@ private Scorer getOrCreateKnnScorer(LeafReaderContext context) throws IOExceptio
return scorer;
}

private float getKnnScore(Scorer knnScorer, int doc) throws IOException {
return (knnScorer.iterator().advance(doc) == doc) ? knnScorer.score() : 0;
}

@Override
public ScorerSupplier scorerSupplier(LeafReaderContext context) {
return new ScorerSupplier() {
Expand Down
49 changes: 49 additions & 0 deletions src/test/java/org/opensearch/knn/index/query/ExplainTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Locale;
Expand Down Expand Up @@ -376,6 +377,54 @@ public void testDefaultANNSearch() {
assertTrue(Comparators.isInOrder(actualDocIds, Comparator.naturalOrder()));
}

@SneakyThrows
public void testExplain_whenDocIsNotANearestNeighbor_thenNoMatch() {
// Given
int k = 3;
jniServiceMockedStatic.when(
() -> JNIService.queryIndex(anyLong(), eq(QUERY_VECTOR), eq(k), eq(HNSW_METHOD_PARAMETERS), any(), eq(null), anyInt(), any())
).thenReturn(getFilteredKNNQueryResults());

final int[] filterDocIds = new int[] { 0, 1, 2, 3, 4, 5 };
final Map<String, String> attributesMap = ImmutableMap.of(
KNN_ENGINE,
KNNEngine.FAISS.getName(),
SPACE_TYPE,
SpaceType.L2.getValue()
);

setupTest(filterDocIds, attributesMap);

final KNNQuery query = KNNQuery.builder()
.field(FIELD_NAME)
.queryVector(QUERY_VECTOR)
.k(k)
.indexName(INDEX_NAME)
.filterQuery(FILTER_QUERY)
.methodParameters(HNSW_METHOD_PARAMETERS)
.vectorDataType(VectorDataType.FLOAT)
.explain(true)
.build();
query.setExplain(true);

final KNNWeight knnWeight = new DefaultKNNWeight(query, 1f, filterQueryWeight);
final KNNScorer knnScorer = (KNNScorer) knnWeight.scorer(leafReaderContext);
assertNotNull(knnScorer);
knnWeight.getKnnExplanation().addKnnScorer(leafReaderContext, knnScorer);

// The query returns FILTERED_DOC_ID_TO_SCORES, so this document is not one of the nearest neighbors. Advancing
// the scorer to it lands on the next matching document instead.
final int docIdWithoutMatch = Collections.min(FILTERED_DOC_ID_TO_SCORES.keySet()) - 1;

// When
final Explanation explanation = knnWeight.explain(leafReaderContext, docIdWithoutMatch);

// Then
assertNotNull(explanation);
assertFalse(explanation.isMatch());
assertTrue(explanation.getDescription().contains(FIELD_NAME));
}

@SneakyThrows
public void testANN_FilteredExactSearchAfterANN() {
ExactSearcher mockedExactSearcher = mock(ExactSearcher.class);
Expand Down