SOLR-8155: fix UnInvertedField.collectDocsGeneric, used for facet.prefix or non-count sorting

solr/CHANGES.txt

@@ -270,6 +270,11 @@ Bug Fixes
 * SOLR-8449: Fix the core restore functionality to allow restoring multiple times on the same core
   (Johannes Brucher, Varun Thacker)
 
+* SOLR-8155: JSON Facet API - field faceting on a multi-valued string field without
+  docValues (i.e. UnInvertedField implementation), but with a prefix or with a sort
+  other than count, resulted in incorrect results.  This has been fixed, and facet.prefix
+  support for facet.method=uif has been enabled. (Mikhail Khludnev, yonik)
+
 Optimizations
 ----------------------
 * SOLR-7876: Speed up queries and operations that use many terms when timeAllowed has not been

solr/core/src/java/org/apache/solr/request/SimpleFacets.java

@@ -487,17 +487,7 @@ public class SimpleFacets {
             jsonFacet.put("limit", limit);
             jsonFacet.put("mincount", mincount);
             jsonFacet.put("missing", missing);
-            
+            jsonFacet.put("prefix", prefix);
-            if (prefix!=null) {
-              // presumably it supports single-value, but at least now returns wrong results on multi-value
-              throw new SolrException (
-                  SolrException.ErrorCode.BAD_REQUEST,
-                  FacetParams.FACET_PREFIX+"="+prefix+
-                  " are not supported by "+FacetParams.FACET_METHOD+"="+FacetParams.FACET_METHOD_uif+
-                  " for field:"+ field
-                  //jsonFacet.put("prefix", prefix);
-              );
-            }
             jsonFacet.put("numBuckets", params.getFieldBool(field, "numBuckets", false));
             jsonFacet.put("allBuckets", params.getFieldBool(field, "allBuckets", false));
             jsonFacet.put("method", "uif");

solr/core/src/java/org/apache/solr/search/facet/UnInvertedField.java

@@ -499,10 +499,11 @@ public class UnInvertedField extends DocTermOrds {
               if (delta==0) break;
               tnum += delta - TNUM_OFFSET;
               int arrIdx = tnum - startTermIndex;
-              if (arrIdx < 0) continue;
+              if (arrIdx >= 0) {
-              if (arrIdx >= nTerms) break;
+                if (arrIdx >= nTerms) break;
-              countAcc.incrementCount(arrIdx, 1);
+                countAcc.incrementCount(arrIdx, 1);
-              processor.collectFirstPhase(segDoc, arrIdx);
+                processor.collectFirstPhase(segDoc, arrIdx);
+              }
               delta = 0;
             }
             code >>>= 8;

solr/core/src/test/org/apache/solr/TestRandomDVFaceting.java

@@ -215,9 +215,6 @@ public class TestRandomDVFaceting extends SolrTestCaseJ4 {
       List<String> methods = multiValued ? multiValuedMethods : singleValuedMethods;
       List<String> responses = new ArrayList<>(methods.size());
       for (String method : methods) {
-        if (method.equals("uif") && params.get("facet.prefix")!=null) {
-          continue; // it's not supported there
-        }
         if (method.equals("dv")) {
           params.set("facet.field", "{!key="+facet_field+"}"+facet_field+"_dv");
           params.set("facet.method",(String) null);

solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java

@@ -512,7 +512,7 @@ public class SimpleFacetsTest extends SolrTestCaseJ4 {
         params.set("facet.method", method);
       }
       for (String prefix : prefixes) {
-        if (prefix == null || "uif".equals(method)) {// there is no support 
+        if (prefix == null) {
           params.remove("facet.prefix");
         } else {
           params.set("facet.prefix", prefix);
@@ -2016,16 +2016,6 @@ public class SimpleFacetsTest extends SolrTestCaseJ4 {
     doFacetPrefix("tt_s1", "{!threads=2}", "", "facet.method","fcs");   // specific number of threads
   }
   
-  /** no prefix for uif */
-  @Test(expected=RuntimeException.class)
-  public void testNOFacetPrefixForUif() {
-    if (random().nextBoolean()) {
-      doFacetPrefix("tt_s1", null, "", "facet.method", "uif");
-    } else {
-      doFacetPrefix("t_s", null, "", "facet.method", "uif");
-    }
-  }
-  
   @Test
   @Ignore("SOLR-8466 - facet.method=uif ignores facet.contains")
   public void testFacetContainsUif() {

solr/core/src/test/org/apache/solr/request/TestFaceting.java

@@ -529,12 +529,8 @@ public class TestFaceting extends SolrTestCaseJ4 {
   }
       
   private void assertQforUIF(String message, SolrQueryRequest request, String ... tests) {
-    final String paramString = request.getParamString();
+    // handle any differences for uif here, like skipping unsupported options
-    if (paramString.contains("uif") && paramString.contains("prefix")){
+    assertQ(message,request, tests);
-      assertQEx("uif prohibits prefix", "not supported", request, ErrorCode.BAD_REQUEST);
-    }else{
-      assertQ(message,request, tests);
-    }
   }
 
   private void add50ocs() {

solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java

@@ -644,6 +644,27 @@ public class TestJsonFacets extends SolrTestCaseHS {
             " } "
     );
 
+    // test prefix on real multi-valued field
+    client.testJQ(params(p, "q", "*:*"
+        , "json.facet", "{" +
+            " f1:{${terms} type:terms, field:${multi_ss}, prefix:A  }" +
+            ",f2:{${terms} type:terms, field:${multi_ss}, prefix:z }" +
+            ",f3:{${terms} type:terms, field:${multi_ss}, prefix:aa }" +
+            ",f4:{${terms} type:terms, field:${multi_ss}, prefix:bb }" +
+            ",f5:{${terms} type:terms, field:${multi_ss}, prefix:a }" +
+            ",f6:{${terms} type:terms, field:${multi_ss}, prefix:b }" +
+            "}"
+        )
+        , "facets=={ 'count':6 " +
+            ",f1:{buckets:[]}" +
+            ",f2:{buckets:[]}" +
+            ",f3:{buckets:[]}" +
+            ",f4:{buckets:[]}" +
+            ",f5:{buckets:[ {val:a,count:3} ]}" +
+            ",f6:{buckets:[ {val:b,count:3} ]}" +
+            " } "
+    );
+
     //
     // missing
     //