You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by kr...@apache.org on 2023/01/18 19:22:12 UTC

[lucene-solr] branch branch_8_11 updated: SOLR-13219: Fix NPE in FieldLengthFeature with non-stored/missing fields (#1051)

This is an automated email from the ASF dual-hosted git repository.

krisden pushed a commit to branch branch_8_11
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8_11 by this push:
     new 5d8e1c5e0b4 SOLR-13219: Fix NPE in FieldLengthFeature with non-stored/missing fields (#1051)
5d8e1c5e0b4 is described below

commit 5d8e1c5e0b4e8aa69d836d7be662a94aefcb96d0
Author: Tomasz Elendt <85...@users.noreply.github.com>
AuthorDate: Fri Oct 7 13:18:28 2022 +0200

    SOLR-13219: Fix NPE in FieldLengthFeature with non-stored/missing fields (#1051)
    
    This commit removes unnecessary check for omitNorm from
    FieldLengthFeatureScorer that otherwise causes NullPointerException
    in FieldLengthFeature for non-stored or missing fields.
---
 solr/CHANGES.txt                                   |  2 +
 .../solr/ltr/feature/FieldLengthFeature.java       | 12 +-----
 .../test-files/solr/collection1/conf/schema.xml    |  4 +-
 .../solr/ltr/feature/TestFeatureLogging.java       |  4 +-
 .../solr/ltr/feature/TestFieldLengthFeature.java   | 50 +++++++++++++++++++---
 5 files changed, 52 insertions(+), 20 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 9c2f37017cf..5e738f42073 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -20,6 +20,8 @@ Bug Fixes
 
 * SOLR-16274: HEAD request for managed resource returns 500 Server Error (Kevin Risden)
 
+* SOLR-13219: Fix NPE in FieldLengthFeature with non-stored/missing fields (Nick Veenhof, Tomasz Elendt)
+
 Optimizations
 ---------------------
 * SOLR-16555: SolrIndexSearcher - FilterCache intersections/andNot should not clone bitsets repeatedly (Kevin Risden, David Smiley)
diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/FieldLengthFeature.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/FieldLengthFeature.java
index 7bbfdb813af..ce09bc93fc1 100644
--- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/FieldLengthFeature.java
+++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/FieldLengthFeature.java
@@ -20,7 +20,6 @@ import java.io.IOException;
 import java.util.LinkedHashMap;
 import java.util.Map;
 
-import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.NumericDocValues;
 import org.apache.lucene.search.DocIdSetIterator;
@@ -124,18 +123,9 @@ public class FieldLengthFeature extends Feature {
 
       NumericDocValues norms = null;
 
-      public FieldLengthFeatureScorer(FeatureWeight weight,
-          NumericDocValues norms) throws IOException {
+      public FieldLengthFeatureScorer(FeatureWeight weight, NumericDocValues norms) {
         super(weight, norms);
         this.norms = norms;
-
-        // In the constructor, docId is -1, so using 0 as default lookup
-        final IndexableField idxF = searcher.doc(0).getField(field);
-        if (idxF.fieldType().omitNorms()) {
-          throw new IOException(
-              "FieldLengthFeatures can't be used if omitNorms is enabled (field="
-                  + field + ")");
-        }
       }
 
       @Override
diff --git a/solr/contrib/ltr/src/test-files/solr/collection1/conf/schema.xml b/solr/contrib/ltr/src/test-files/solr/collection1/conf/schema.xml
index 288a953c85d..37e984ac3fd 100644
--- a/solr/contrib/ltr/src/test-files/solr/collection1/conf/schema.xml
+++ b/solr/contrib/ltr/src/test-files/solr/collection1/conf/schema.xml
@@ -22,8 +22,8 @@
     <field name="finalScore" type="string" indexed="true" stored="true" multiValued="false"/>
     <field name="finalScoreFloat" type="float" indexed="true" stored="true" multiValued="false"/>
     <field name="field" type="text_general" indexed="true" stored="false" multiValued="false"/>
-    <field name="title" type="text_general" indexed="true" stored="true"/>
-    <field name="description" type="text_general" indexed="true" stored="true"/>
+    <field name="title" type="text_general" indexed="true" stored="false"/>
+    <field name="description" type="text_general" indexed="true" stored="false"/>
     <field name="keywords" type="text_general" indexed="true" stored="true" multiValued="true"/>
     <field name="popularity" type="int" indexed="true" stored="true" />
     <field name="dvIntPopularity" type="int" indexed="false" docValues="true" stored="false" multiValued="false"/>
diff --git a/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFeatureLogging.java b/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFeatureLogging.java
index e7af2506b5f..b0ca97b7851 100644
--- a/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFeatureLogging.java
+++ b/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFeatureLogging.java
@@ -74,7 +74,7 @@ public class TestFeatureLogging extends TestRerankBase {
 
     final SolrQuery query = new SolrQuery();
     query.setQuery("title:bloomberg");
-    query.add("fl", "title,description,id,popularity,[fv]");
+    query.add("fl", "id,popularity,[fv]");
     query.add("rows", "3");
     query.add("debugQuery", "on");
     query.add("rq", "{!ltr reRankDocs=3 model=sum1}");
@@ -82,7 +82,7 @@ public class TestFeatureLogging extends TestRerankBase {
     restTestHarness.query("/query" + query.toQueryString());
     assertJQ(
         "/query" + query.toQueryString(),
-        "/response/docs/[0]/=={'title':'bloomberg bloomberg ', 'description':'bloomberg','id':'7', 'popularity':2,  '[fv]':'"+docs0fv_default_csv+"'}");
+        "/response/docs/[0]/=={'id':'7', 'popularity':2,  '[fv]':'" + docs0fv_default_csv + "'}");
 
     query.remove("fl");
     query.add("fl", "[fv]");
diff --git a/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFieldLengthFeature.java b/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFieldLengthFeature.java
index b913d1bd72b..368e03bdc22 100644
--- a/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFieldLengthFeature.java
+++ b/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFieldLengthFeature.java
@@ -37,11 +37,19 @@ public class TestFieldLengthFeature extends TestRerankBase {
     assertU(adoc("id", "3", "title", "w3", "description", "w3"));
     assertU(adoc("id", "4", "title", "w4", "description", "w4"));
     assertU(adoc("id", "5", "title", "w5", "description", "w5"));
-    assertU(adoc("id", "6", "title", "w1 w2", "description", "w1 w2"));
-    assertU(adoc("id", "7", "title", "w1 w2 w3 w4 w5", "description",
-        "w1 w2 w3 w4 w5 w8"));
-    assertU(adoc("id", "8", "title", "w1 w1 w1 w2 w2 w8", "description",
-        "w1 w1 w1 w2 w2"));
+    assertU(adoc("id", "6", "title", "w1 w2", "description", "w1 w2", "x_t", "1 2"));
+    assertU(
+        adoc("id", "7", "title", "w1 w2 w3 w4 w5", "description", "w1 w2 w3 w4 w5 w8", "x_t", "1"));
+    assertU(
+        adoc(
+            "id",
+            "8",
+            "title",
+            "w1 w1 w1 w2 w2 w8",
+            "description",
+            "w1 w1 w1 w2 w2",
+            "x_t",
+            "1 2 3"));
     assertU(commit());
   }
 
@@ -151,6 +159,38 @@ public class TestFieldLengthFeature extends TestRerankBase {
     assertJQ("/query" + query.toQueryString(), "/response/docs/[3]/id=='1'");
   }
 
+  @Test
+  public void testRankingDynamicField() throws Exception {
+    loadFeature("dynfield-length", FieldLengthFeature.class.getName(), "{\"field\":\"x_t\"}");
+
+    loadModel(
+        "dynfield-model",
+        LinearModel.class.getName(),
+        new String[] {"dynfield-length"},
+        "{\"weights\":{\"dynfield-length\":1.0}}");
+
+    final SolrQuery query = new SolrQuery();
+    query.setQuery("title:w1");
+    query.add("fl", "*, score");
+    query.add("rows", "4");
+
+    // Normal term match
+    assertJQ("/query" + query.toQueryString(), "/response/numFound/==4");
+    assertJQ("/query" + query.toQueryString(), "/response/docs/[0]/id=='1'");
+    assertJQ("/query" + query.toQueryString(), "/response/docs/[1]/id=='8'");
+    assertJQ("/query" + query.toQueryString(), "/response/docs/[2]/id=='6'");
+    assertJQ("/query" + query.toQueryString(), "/response/docs/[3]/id=='7'");
+    // Normal term match
+
+    query.add("rq", "{!ltr model=dynfield-model reRankDocs=4}");
+
+    assertJQ("/query" + query.toQueryString(), "/response/numFound/==4");
+    assertJQ("/query" + query.toQueryString(), "/response/docs/[0]/id=='8'");
+    assertJQ("/query" + query.toQueryString(), "/response/docs/[1]/id=='6'");
+    assertJQ("/query" + query.toQueryString(), "/response/docs/[2]/id=='7'");
+    assertJQ("/query" + query.toQueryString(), "/response/docs/[3]/id=='1'");
+  }
+
   @Test
   public void testParamsToMap() throws Exception {
     final LinkedHashMap<String,Object> params = new LinkedHashMap<String,Object>();