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/03/06 00:47:56 UTC
lucene-solr git commit: SOLR-8155: fix
UnInvertedField.collectDocsGeneric, used for facet.prefix or non-count sorting
Repository: lucene-solr
Updated Branches:
refs/heads/master 74f4f9f98 -> 855572614
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/85557261
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/85557261
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/85557261
Branch: refs/heads/master
Commit: 85557261431f9314253ebe282eb6d400bf7cae03
Parents: 74f4f9f
Author: yonik <yo...@apache.org>
Authored: Sat Mar 5 18:47:11 2016 -0500
Committer: yonik <yo...@apache.org>
Committed: Sat Mar 5 18:47:33 2016 -0500
----------------------------------------------------------------------
solr/CHANGES.txt | 5 +++++
.../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 | 21 ++++++++++++++++++++
7 files changed, 35 insertions(+), 35 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/85557261/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 813a0b7..6834eb5 100644
--- a/solr/CHANGES.txt
+++ b/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
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/85557261/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 2c61f6d..a9ce11d 100644
--- a/solr/core/src/java/org/apache/solr/request/SimpleFacets.java
+++ b/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);
-
- 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/85557261/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/85557261/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/85557261/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 1541a46..3b99eb8 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);
@@ -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() {
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/85557261/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 16a6b13..97dcedf 100644
--- a/solr/core/src/test/org/apache/solr/request/TestFaceting.java
+++ b/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();
- 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/85557261/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 7df03f1..83220ed 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
@@ -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
//