lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From yo...@apache.org
Subject lucene-solr:branch_5x: SOLR-8155: fix UnInvertedField.collectDocsGeneric, used for facet.prefix or non-count sorting
Date Mon, 11 Apr 2016 18:08:03 GMT
Repository: lucene-solr
Updated Branches:
  refs/heads/branch_5x 521b9b101 -> 0073ba970


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


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/0073ba97
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/0073ba97
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/0073ba97

Branch: refs/heads/branch_5x
Commit: 0073ba9700d5df7b00d77a5d5dd831316f138042
Parents: 521b9b1
Author: yonik <yonik@apache.org>
Authored: Sat Mar 5 18:47:11 2016 -0500
Committer: yonik <yonik@apache.org>
Committed: Mon Apr 11 14:07:29 2016 -0400

----------------------------------------------------------------------
 solr/CHANGES.txt                                |  6 ++++
 .../org/apache/solr/request/SimpleFacets.java   | 12 +------
 .../solr/search/facet/UnInvertedField.java      |  9 ++---
 .../org/apache/solr/TestRandomDVFaceting.java   |  3 --
 .../apache/solr/request/SimpleFacetsTest.java   | 12 +------
 .../org/apache/solr/request/TestFaceting.java   |  8 ++---
 .../solr/search/facet/TestJsonFacets.java       | 36 ++++++++++++++++++++
 7 files changed, 51 insertions(+), 35 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/0073ba97/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 93bc722..8b80220 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -45,6 +45,12 @@ Bug Fixes
 * SOLR-8870: AngularJS Query tab no longer URL-encodes the /select part of the request, fixing
possible 404 issue
   when Solr is behind a proxy. Also, now supports old-style &qt param when handler not
prefixed with "/" (janhoy)
 
+* 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)
+
+
 ======================= 5.5.0 =======================
 
 Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/0073ba97/solr/core/src/java/org/apache/solr/request/SimpleFacets.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/request/SimpleFacets.java b/solr/core/src/java/org/apache/solr/request/SimpleFacets.java
index 7d6f369..2e7e53a 100644
--- a/solr/core/src/java/org/apache/solr/request/SimpleFacets.java
+++ b/solr/core/src/java/org/apache/solr/request/SimpleFacets.java
@@ -497,17 +497,7 @@ public class SimpleFacets {
             jsonFacet.put("limit", limit);
             jsonFacet.put("mincount", mincount);
             jsonFacet.put("missing", missing);
-            
-            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("prefix", prefix);
             jsonFacet.put("numBuckets", params.getFieldBool(field, "numBuckets", false));
             jsonFacet.put("allBuckets", params.getFieldBool(field, "allBuckets", false));
             jsonFacet.put("method", "uif");

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/0073ba97/solr/core/src/java/org/apache/solr/search/facet/UnInvertedField.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/search/facet/UnInvertedField.java b/solr/core/src/java/org/apache/solr/search/facet/UnInvertedField.java
index 9294792..c1613cd 100644
--- a/solr/core/src/java/org/apache/solr/search/facet/UnInvertedField.java
+++ b/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 >= nTerms) break;
-              countAcc.incrementCount(arrIdx, 1);
-              processor.collectFirstPhase(segDoc, arrIdx);
+              if (arrIdx >= 0) {
+                if (arrIdx >= nTerms) break;
+                countAcc.incrementCount(arrIdx, 1);
+                processor.collectFirstPhase(segDoc, arrIdx);
+              }
               delta = 0;
             }
             code >>>= 8;

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/0073ba97/solr/core/src/test/org/apache/solr/TestRandomDVFaceting.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/TestRandomDVFaceting.java b/solr/core/src/test/org/apache/solr/TestRandomDVFaceting.java
index 7decfce..90c2394 100644
--- a/solr/core/src/test/org/apache/solr/TestRandomDVFaceting.java
+++ b/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);

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/0073ba97/solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java b/solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java
index 042e840..f19d056 100644
--- a/solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java
+++ b/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);
@@ -2038,16 +2038,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() {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/0073ba97/solr/core/src/test/org/apache/solr/request/TestFaceting.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/request/TestFaceting.java b/solr/core/src/test/org/apache/solr/request/TestFaceting.java
index a557dc1..56bb611 100644
--- a/solr/core/src/test/org/apache/solr/request/TestFaceting.java
+++ b/solr/core/src/test/org/apache/solr/request/TestFaceting.java
@@ -626,12 +626,8 @@ public class TestFaceting extends SolrTestCaseJ4 {
   }
       
   private void assertQforUIF(String message, SolrQueryRequest request, String ... tests)
{
-    final String paramString = request.getParamString();
-    if (paramString.contains("uif") && paramString.contains("prefix")){
-      assertQEx("uif prohibits prefix", "not supported", request, ErrorCode.BAD_REQUEST);
-    }else{
-      assertQ(message,request, tests);
-    }
+    // handle any differences for uif here, like skipping unsupported options
+    assertQ(message,request, tests);
   }
 
   private void add50ocs() {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/0073ba97/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java b/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java
index 9982d72..99877c0 100644
--- a/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java
+++ b/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java
@@ -632,6 +632,42 @@ public class TestJsonFacets extends SolrTestCaseHS {
             "'f1':{ 'buckets':[]} } "
     );
 
+    // test prefix on where field
+    client.testJQ(params(p, "q", "*:*"
+        , "json.facet", "{" +
+            " f1:{${terms} type:terms, field:${where_s}, prefix:N  }" +
+            ",f2:{${terms} type:terms, field:${where_s}, prefix:NY }" +
+            ",f3:{${terms} type:terms, field:${where_s}, prefix:NJ }" +
+            "}"
+        )
+        , "facets=={ 'count':6 " +
+            ",f1:{ 'buckets':[ {val:NJ,count:3}, {val:NY,count:2} ]}" +
+            ",f2:{ 'buckets':[ {val:NY,count:2} ]}" +
+            ",f3:{ 'buckets':[ {val:NJ,count:3} ]}" +
+            " } "
+    );
+
+    // 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
     //


Mime
View raw message