From 9c9680a7d4df73c23e9df9a14d1e150f5b727758 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Aur=C3=A9lien=20FOUCRET?= <aurelien.foucret@gmail.com>
Date: Fri, 24 Jan 2025 18:56:04 +0100
Subject: [PATCH] LTR sometines throw NullPointerException: Cannot read field
 "approximation" because "top" is null (#120809)

* Add check on the DisiPriorityQueue size.

* Update docs/changelog/120809.yaml

* Add a unit test.
---
 docs/changelog/120809.yaml                    |  6 ++++
 .../inference/ltr/QueryFeatureExtractor.java  |  7 ++++-
 .../ltr/QueryFeatureExtractorTests.java       | 28 +++++++++++++++++++
 3 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 docs/changelog/120809.yaml

diff --git a/docs/changelog/120809.yaml b/docs/changelog/120809.yaml
new file mode 100644
index 0000000000000..30a3736dc93a4
--- /dev/null
+++ b/docs/changelog/120809.yaml
@@ -0,0 +1,6 @@
+pr: 120809
+summary: LTR sometines throw `NullPointerException:` Cannot read field "approximation"
+  because "top" is null
+area: Ranking
+type: bug
+issues: []
diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ltr/QueryFeatureExtractor.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ltr/QueryFeatureExtractor.java
index bbc377a67ec0b..08c141c0858ca 100644
--- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ltr/QueryFeatureExtractor.java
+++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ltr/QueryFeatureExtractor.java
@@ -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);
diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/ltr/QueryFeatureExtractorTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/ltr/QueryFeatureExtractorTests.java
index 3878ce5dab087..3b25a266bf412 100644
--- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/ltr/QueryFeatureExtractorTests.java
+++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/ltr/QueryFeatureExtractorTests.java
@@ -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;
 
@@ -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 {
 
@@ -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();
+    }
 }