Skip to content

Commit

Permalink
LTR sometines throw NullPointerException: Cannot read field "approxim…
Browse files Browse the repository at this point in the history
…ation" because "top" is null (#120809) (#120827)

* Add check on the DisiPriorityQueue size.

* Update docs/changelog/120809.yaml

* Add a unit test.
  • Loading branch information
afoucret authored Jan 24, 2025
1 parent 8adafb0 commit 149fbf2
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 1 deletion.
6 changes: 6 additions & 0 deletions docs/changelog/120809.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 120809
summary: LTR sometines throw `NullPointerException:` Cannot read field "approximation"
because "top" is null
area: Ranking
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,16 @@ public void setNextReader(LeafReaderContext segmentContext) throws IOException {
}
scorers.add(scorer);
}
rankerIterator = new DisjunctionDISIApproximation(disiPriorityQueue);

rankerIterator = disiPriorityQueue.size() > 0 ? new DisjunctionDISIApproximation(disiPriorityQueue) : null;
}

@Override
public void addFeatures(Map<String, Object> featureMap, int docId) throws IOException {
if (rankerIterator == null) {
return;
}

rankerIterator.advance(docId);
for (int i = 0; i < featureNames.size(); i++) {
Scorer scorer = scorers.get(i);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.elasticsearch.index.query.QueryRewriteContext;
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.test.AbstractBuilderTestCase;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.core.ml.inference.trainedmodel.ltr.QueryExtractorBuilder;
import org.elasticsearch.xpack.core.ml.utils.QueryProvider;

Expand All @@ -31,12 +32,14 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Stream;

import static org.hamcrest.Matchers.anEmptyMap;
import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.hasKey;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.not;
import static org.mockito.Mockito.mock;

public class QueryFeatureExtractorTests extends AbstractBuilderTestCase {

Expand Down Expand Up @@ -125,4 +128,29 @@ public void testQueryExtractor() throws IOException {
dir.close();
}

public void testEmptyDisiPriorityQueue() throws IOException {
addDocs(
new String[] { "the quick brown fox", "the slow brown fox", "the grey dog", "yet another string" },
new int[] { 5, 10, 12, 11 }
);

// Scorers returned by weights are null
List<String> featureNames = randomList(1, 10, ESTestCase::randomIdentifier);
List<Weight> weights = Stream.generate(() -> mock(Weight.class)).limit(featureNames.size()).toList();

QueryFeatureExtractor featureExtractor = new QueryFeatureExtractor(featureNames, weights);

for (LeafReaderContext leafReaderContext : searcher.getLeafContexts()) {
int maxDoc = leafReaderContext.reader().maxDoc();
featureExtractor.setNextReader(leafReaderContext);
for (int i = 0; i < maxDoc; i++) {
Map<String, Object> featureMap = new HashMap<>();
featureExtractor.addFeatures(featureMap, i);
assertThat(featureMap, anEmptyMap());
}
}

reader.close();
dir.close();
}
}

0 comments on commit 149fbf2

Please sign in to comment.