You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mu...@apache.org on 2019/12/08 04:51:20 UTC

[lucene-solr] branch branch_8x updated: SOLR-7798: robust support for expand when used w/o collapsing (#325)

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

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


The following commit(s) were added to refs/heads/branch_8x by this push:
     new 65a1804  SOLR-7798: robust support for expand when used w/o collapsing (#325)
65a1804 is described below

commit 65a1804aebd77e6b088da362d119de07917d5d8f
Author: Michael Gibney <mi...@michaelgibney.net>
AuthorDate: Sat Dec 7 23:29:33 2019 -0500

    SOLR-7798: robust support for expand when used w/o collapsing (#325)
    
    There are applications of ExpandComponent that intentionally do not
    involve prior collapsing of results on the expand field, which can lead
    to an NPE in expand component when expand.field (for matched docs) has
    fewer unique values than the number of matched docs.
    
    This commit refines the approach taken in SOLR-13877, which addressed
    the same underlying issue.
---
 solr/CHANGES.txt                                   |  2 +-
 .../solr/handler/component/ExpandComponent.java    | 35 +++++++++++-----------
 .../handler/component/TestExpandComponent.java     | 13 ++++++++
 3 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index fa64424..0c3ca70 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -109,7 +109,7 @@ Bug Fixes
 * SOLR-12393: Compute score if requested even when expanded docs not sorted by score in ExpandComponent.
   (David Smiley, Munendra S N)
 
-* SOLR-13877: Fix NPE in expand component when matched docs have fewer unique values. (Munendra S N)
+* SOLR-13877, SOLR-7798: Robust support for ExpandComponent when used independently of CollapsingPostFilter. (Jörg Rathlev, Michael Gibney, Munendra S N)
 
 * SOLR-13823: Fix ClassCastEx when score is requested with group.query. This also fixes score not being generated
   for distributed group.query case. (Uwe Jäger, Munendra S N)
diff --git a/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java b/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java
index 2a58248..0440a3e 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java
@@ -281,7 +281,6 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
         currentValues = sortedDocValues[currentContext];
         segmentOrdinalMap = ordinalMap.getGlobalOrds(currentContext);
       }
-      int count = 0;
 
       ordBytes = new IntObjectHashMap<>();
 
@@ -303,12 +302,12 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
             currentValues.advance(contextDoc);
           }
           if (contextDoc == currentValues.docID()) {
-            int ord = currentValues.ordValue();
-            ++count;
-            BytesRef ref = currentValues.lookupOrd(ord);
-            ord = (int)segmentOrdinalMap.get(ord);
-            ordBytes.put(ord, BytesRef.deepCopyOf(ref));
-            groupBits.set(ord);
+            int contextOrd = currentValues.ordValue();
+            int ord = (int)segmentOrdinalMap.get(contextOrd);
+            if (!groupBits.getAndSet(ord)) {
+              BytesRef ref = currentValues.lookupOrd(contextOrd);
+              ordBytes.put(ord, BytesRef.deepCopyOf(ref));
+            }
             collapsedSet.add(globalDoc);
           }
         } else {
@@ -317,22 +316,22 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
           }
           if (globalDoc == values.docID()) {
             int ord = values.ordValue();
-            ++count;
-            BytesRef ref = values.lookupOrd(ord);
-            ordBytes.put(ord, BytesRef.deepCopyOf(ref));
-            groupBits.set(ord);
+            if (!groupBits.getAndSet(ord)) {
+              BytesRef ref = values.lookupOrd(ord);
+              ordBytes.put(ord, BytesRef.deepCopyOf(ref));
+            }
             collapsedSet.add(globalDoc);
           }
         }
       }
 
+      int count = ordBytes.size();
       if(count > 0 && count < 200) {
         groupQuery = getGroupQuery(field, count, ordBytes);
       }
     } else {
       groupSet = new LongHashSet(docList.size());
       NumericDocValues collapseValues = contexts.get(currentContext).reader().getNumericDocValues(field);
-      int count = 0;
       for(int i=0; i<globalDocs.length; i++) {
         int globalDoc = globalDocs[i];
         while(globalDoc >= nextDocBase) {
@@ -353,12 +352,12 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
           value = 0;
         }
         if(value != nullValue) {
-          ++count;
           groupSet.add(value);
           collapsedSet.add(globalDoc);
         }
       }
 
+      int count = groupSet.size();
       if(count > 0 && count < 200) {
         if (fieldType.isPointField()) {
           groupQuery = getPointGroupQuery(schemaField, count, groupSet);
@@ -685,7 +684,8 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
                            int size,
                            LongHashSet groupSet) {
 
-    List<BytesRef> bytesRefs = new ArrayList<>(size);
+    BytesRef[] bytesRefs = new BytesRef[size];
+    int index = -1;
     BytesRefBuilder term = new BytesRefBuilder();
     Iterator<LongCursor> it = groupSet.iterator();
 
@@ -693,7 +693,7 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
       LongCursor cursor = it.next();
       String stringVal = numericToString(ft, cursor.value);
       ft.readableToIndexed(stringVal, term);
-      bytesRefs.add(term.toBytesRef());
+      bytesRefs[++index] = term.toBytesRef();
     }
 
     return new TermInSetQuery(fname, bytesRefs);
@@ -734,11 +734,12 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
   private Query getGroupQuery(String fname,
                               int size,
                               IntObjectHashMap<BytesRef> ordBytes) {
-    List<BytesRef> bytesRefs = new ArrayList<>(size);
+    BytesRef[] bytesRefs = new BytesRef[size];
+    int index = -1;
     Iterator<IntObjectCursor<BytesRef>>it = ordBytes.iterator();
     while (it.hasNext()) {
       IntObjectCursor<BytesRef> cursor = it.next();
-      bytesRefs.add(cursor.value);
+      bytesRefs[++index] = cursor.value;
     }
     return new TermInSetQuery(fname, bytesRefs);
   }
diff --git a/solr/core/src/test/org/apache/solr/handler/component/TestExpandComponent.java b/solr/core/src/test/org/apache/solr/handler/component/TestExpandComponent.java
index 336d608..690ad60 100644
--- a/solr/core/src/test/org/apache/solr/handler/component/TestExpandComponent.java
+++ b/solr/core/src/test/org/apache/solr/handler/component/TestExpandComponent.java
@@ -352,6 +352,19 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
         "/response/lst[@name='expanded']/result[@name='2000.0']/doc[1]/str[@name='id'][.='7']",
         "count(//*[@name='score'])=0"
     );
+
+    // Support expand enabled without previous collapse
+    assertQ(req("q", "type_s:child", "sort", group+" asc, test_l desc", "defType", "edismax",
+        "expand", "true", "expand.q", "type_s:parent", "expand.field", group),
+        "*[count(/response/result/doc)=4]",
+        "*[count(/response/lst[@name='expanded']/result)=2]",
+        "/response/result/doc[1]/str[@name='id'][.='7']",
+        "/response/result/doc[2]/str[@name='id'][.='2']",
+        "/response/result/doc[3]/str[@name='id'][.='8']",
+        "/response/result/doc[4]/str[@name='id'][.='6']",
+        "/response/lst[@name='expanded']/result[@name='1"+floatAppend+"']/doc[1]/str[@name='id'][.='1']",
+        "/response/lst[@name='expanded']/result[@name='2"+floatAppend+"']/doc[1]/str[@name='id'][.='5']"
+    );
   }
 
   @Test