You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by yo...@apache.org on 2016/04/11 20:11:15 UTC
lucene-solr:branch_5_5: SOLR-8155: fix
UnInvertedField.collectDocsGeneric, used for facet.prefix or non-count sorting
Repository: lucene-solr
Updated Branches:
refs/heads/branch_5_5 f0fffb602 -> c3446734a
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/c3446734
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/c3446734
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/c3446734
Branch: refs/heads/branch_5_5
Commit: c3446734a2b6dccd8092f5672651779223076dac
Parents: f0fffb6
Author: yonik <yo...@apache.org>
Authored: Sat Mar 5 18:47:11 2016 -0500
Committer: yonik <yo...@apache.org>
Committed: Mon Apr 11 14:10:23 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/c3446734/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 28344f7..a4b77f3 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -40,6 +40,12 @@ Bug Fixes
* SOLR-8725: Allow hyphen in collection, core, shard, and alias name as the non-first character
(Anshum Gupta) (from 6.0)
+* 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/c3446734/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/c3446734/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/c3446734/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/c3446734/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/c3446734/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/c3446734/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
//