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/10/29 08:36:01 UTC
[lucene-solr] branch master updated (d53e877 -> 24134cf)
This is an automated email from the ASF dual-hosted git repository.
munendrasn pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git.
from d53e877 LUCENE-9024: Optimize IntroSelector to use median of medians (#966)
new 0a4f6c5 SOLR-13877: fix NPE in expand component
new 24134cf SOLR-13823: fix ClassCastEx in group.query when score is requested
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
solr/CHANGES.txt | 9 +++-
.../solr/handler/component/ExpandComponent.java | 18 +++-----
.../solr/handler/component/QueryComponent.java | 2 +
.../src/java/org/apache/solr/search/Grouping.java | 5 +-
.../grouping/distributed/command/QueryCommand.java | 53 ++++++++++++++++++----
.../TopGroupsResultTransformer.java | 2 +-
.../org/apache/solr/TestDistributedGrouping.java | 13 ++++++
.../test/org/apache/solr/TestGroupingSearch.java | 15 ++++++
.../handler/component/TestExpandComponent.java | 53 +++++++++++++---------
9 files changed, 124 insertions(+), 46 deletions(-)
[lucene-solr] 01/02: SOLR-13877: fix NPE in expand component
Posted by mu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
munendrasn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit 0a4f6c566e450f89745439f07d4c22119cf977b9
Author: Munendra S N <mu...@apache.org>
AuthorDate: Tue Oct 29 13:45:53 2019 +0530
SOLR-13877: fix NPE in expand component
* This could happen when expand component is not used with collapse
and matched docs have fewer unique values
---
solr/CHANGES.txt | 2 +
.../solr/handler/component/ExpandComponent.java | 18 +++-----
.../handler/component/TestExpandComponent.java | 53 +++++++++++++---------
3 files changed, 40 insertions(+), 33 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 23cbefb..a41d2ea 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -136,6 +136,8 @@ 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)
+
Other Changes
---------------------
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 5bf8a3a..2a58248 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
@@ -327,11 +327,7 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
}
if(count > 0 && count < 200) {
- try {
- groupQuery = getGroupQuery(field, count, ordBytes);
- } catch(Exception e) {
- throw new IOException(e);
- }
+ groupQuery = getGroupQuery(field, count, ordBytes);
}
} else {
groupSet = new LongHashSet(docList.size());
@@ -689,16 +685,15 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
int size,
LongHashSet groupSet) {
- BytesRef[] bytesRefs = new BytesRef[size];
+ List<BytesRef> bytesRefs = new ArrayList<>(size);
BytesRefBuilder term = new BytesRefBuilder();
Iterator<LongCursor> it = groupSet.iterator();
- int index = -1;
while (it.hasNext()) {
LongCursor cursor = it.next();
String stringVal = numericToString(ft, cursor.value);
ft.readableToIndexed(stringVal, term);
- bytesRefs[++index] = term.toBytesRef();
+ bytesRefs.add(term.toBytesRef());
}
return new TermInSetQuery(fname, bytesRefs);
@@ -738,13 +733,12 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
private Query getGroupQuery(String fname,
int size,
- IntObjectHashMap<BytesRef> ordBytes) throws Exception {
- BytesRef[] bytesRefs = new BytesRef[size];
- int index = -1;
+ IntObjectHashMap<BytesRef> ordBytes) {
+ List<BytesRef> bytesRefs = new ArrayList<>(size);
Iterator<IntObjectCursor<BytesRef>>it = ordBytes.iterator();
while (it.hasNext()) {
IntObjectCursor<BytesRef> cursor = it.next();
- bytesRefs[++index] = cursor.value;
+ bytesRefs.add(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 4ba2bd4..336d608 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
@@ -91,17 +91,7 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
{"id","7", "term_s", "YYYY", group, "1"+floatAppend, "test_i", "1", "test_l", "100000", "test_f", "2000", "type_s", "child"},
{"id","8", "term_s","YYYY", group, "2"+floatAppend, "test_i", "2", "test_l", "100000", "test_f", "200", "type_s", "child"}
};
- // randomize addition of docs into bunch of segments
- // TODO there ought to be a test utility to do this; even add in batches
- Collections.shuffle(Arrays.asList(docs), random());
- for (String[] doc : docs) {
- assertU(adoc(doc));
- if (random().nextBoolean()) {
- assertU(commit());
- }
- }
-
- assertU(commit());
+ createIndex(docs);
ModifiableSolrParams params = new ModifiableSolrParams();
params.add("q", "*:*");
@@ -165,7 +155,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
//Test override expand.q
-
params = new ModifiableSolrParams();
params.add("q", "type_s:parent");
params.add("defType", "edismax");
@@ -186,7 +175,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
//Test override expand.fq
-
params = new ModifiableSolrParams();
params.add("q", "*:*");
params.add("fq", "type_s:parent");
@@ -207,7 +195,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
);
//Test override expand.fq and expand.q
-
params = new ModifiableSolrParams();
params.add("q", "*:*");
params.add("fq", "type_s:parent");
@@ -229,7 +216,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
);
//Test expand.rows
-
params = new ModifiableSolrParams();
params.add("q", "*:*");
params.add("fq", "{!collapse field="+group+hint+"}");
@@ -250,7 +236,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
//Test no group results
-
params = new ModifiableSolrParams();
params.add("q", "test_i:5");
params.add("fq", "{!collapse field="+group+hint+"}");
@@ -264,7 +249,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
);
//Test zero results
-
params = new ModifiableSolrParams();
params.add("q", "test_i:5532535");
params.add("fq", "{!collapse field="+group+hint+"}");
@@ -278,7 +262,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
);
//Test key-only fl
-
params = new ModifiableSolrParams();
params.add("q", "*:*");
params.add("fq", "{!collapse field="+group+hint+"}");
@@ -299,7 +282,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
);
//Test key-only fl with score but no sorting
-
assertQ(req(params, "fl", "id,score"), "*[count(/response/result/doc)=2]",
"*[count(/response/lst[@name='expanded']/result)=2]",
"/response/result/doc[1]/str[@name='id'][.='2']",
@@ -326,7 +308,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
);
//Test fl with score, sort by non-score
-
assertQ(req(params, "expand.sort", "test_l desc", "fl", "id,test_i,score"),
"*[count(/response/result/doc)=2]",
"count(/response/lst[@name='expanded']/result)=2",
@@ -342,7 +323,6 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
);
//Test fl with score with multi-sort
-
assertQ(req(params, "expand.sort", "test_l desc, score asc", "fl", "id,test_i,score"),
"*[count(/response/result/doc)=2]",
"count(/response/lst[@name='expanded']/result)=2",
@@ -356,6 +336,22 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
"count(//*[@name='score' and .='NaN'])=0",
"count(/response/lst[@name='expanded']/result/doc[number(*/@name='score')!=number(*/@name='test_i')])=0"
);
+
+ // Test for expand with collapse
+ // when matched docs have fewer unique values
+ params = params("q", "*:*", "sort", "id asc", "fl", "id", "rows", "6", "expand", "true", "expand.sort", "id asc");
+ assertQ(req(params, "expand.field", "term_s"),
+ "*[count(/response/result/doc)=6]",
+ "/response/lst[@name='expanded']/result[@name='YYYY']/doc[1]/str[@name='id'][.='7']",
+ "/response/lst[@name='expanded']/result[@name='YYYY']/doc[2]/str[@name='id'][.='8']",
+ "count(//*[@name='score'])=0"
+ );
+ assertQ(req(params, "expand.field", "test_f"),
+ "*[count(/response/result/doc)=6]",
+ "/response/lst[@name='expanded']/result[@name='200.0']/doc[1]/str[@name='id'][.='8']",
+ "/response/lst[@name='expanded']/result[@name='2000.0']/doc[1]/str[@name='id'][.='7']",
+ "count(//*[@name='score'])=0"
+ );
}
@Test
@@ -416,4 +412,19 @@ public class TestExpandComponent extends SolrTestCaseJ4 {
resetExceptionIgnores();
}
+
+ /**
+ * randomize addition of docs into bunch of segments
+ * TODO: there ought to be a test utility to do this; even add in batches
+ */
+ private void createIndex(String[][] docs) {
+ Collections.shuffle(Arrays.asList(docs), random());
+ for (String[] doc : docs) {
+ assertU(adoc(doc));
+ if (random().nextBoolean()) {
+ assertU(commit());
+ }
+ }
+ assertU(commit());
+ }
}
[lucene-solr] 02/02: SOLR-13823: fix ClassCastEx in group.query
when score is requested
Posted by mu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
munendrasn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit 24134cf9b17cd7e7e0d404a92a361466ab26b6e3
Author: Munendra S N <mu...@apache.org>
AuthorDate: Tue Oct 29 13:52:49 2019 +0530
SOLR-13823: fix ClassCastEx in group.query when score is requested
* This makes sures score computed for standalone and distributed
is same for group.query. This is done by using mainQuery to compute
scores
---
solr/CHANGES.txt | 7 ++-
.../solr/handler/component/QueryComponent.java | 2 +
.../src/java/org/apache/solr/search/Grouping.java | 5 +-
.../grouping/distributed/command/QueryCommand.java | 53 ++++++++++++++++++----
.../TopGroupsResultTransformer.java | 2 +-
.../org/apache/solr/TestDistributedGrouping.java | 13 ++++++
.../test/org/apache/solr/TestGroupingSearch.java | 15 ++++++
7 files changed, 84 insertions(+), 13 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index a41d2ea..765e006 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -100,7 +100,9 @@ Jetty 9.4.19.v20190610
Upgrade Notes
---------------------
-(No changes)
+
+* org.apache.solr.search.grouping.distributed.command.QueryCommand.Builder has new method 'setMainQuery' which is used
+ to set top-level query. build() would fail if called without setting mainQuery
New Features
---------------------
@@ -138,6 +140,9 @@ Bug Fixes
* SOLR-13877: Fix NPE in expand component when matched docs have fewer unique values. (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)
+
Other Changes
---------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java b/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
index 7ebe7d1..5f17a10 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
@@ -1392,6 +1392,8 @@ public class QueryComponent extends SearchComponent
.setSort(groupSortSpec.getSort())
.setQuery(query, rb.req)
.setDocSet(searcher)
+ .setMainQuery(rb.getQuery())
+ .setNeedScores(needScores)
.build()
);
}
diff --git a/solr/core/src/java/org/apache/solr/search/Grouping.java b/solr/core/src/java/org/apache/solr/search/Grouping.java
index 06a7ed0..2e43da1 100644
--- a/solr/core/src/java/org/apache/solr/search/Grouping.java
+++ b/solr/core/src/java/org/apache/solr/search/Grouping.java
@@ -887,12 +887,13 @@ public class Grouping {
@Override
protected void finish() throws IOException {
- TopDocsCollector topDocsCollector = (TopDocsCollector) collector.getDelegate();
- TopDocs topDocs = topDocsCollector.topDocs();
+ TopDocs topDocs = topCollector.topDocs();
float maxScore;
if (withinGroupSort == null || withinGroupSort.equals(Sort.RELEVANCE)) {
maxScore = topDocs.scoreDocs.length == 0 ? Float.NaN : topDocs.scoreDocs[0].score;
} else if (needScores) {
+ // use top-level query to populate the scores
+ TopFieldCollector.populateScores(topDocs.scoreDocs, searcher, Grouping.this.query);
maxScore = maxScoreCollector.getMaxScore();
} else {
maxScore = Float.NaN;
diff --git a/solr/core/src/java/org/apache/solr/search/grouping/distributed/command/QueryCommand.java b/solr/core/src/java/org/apache/solr/search/grouping/distributed/command/QueryCommand.java
index 6503d86..06b49f6 100644
--- a/solr/core/src/java/org/apache/solr/search/grouping/distributed/command/QueryCommand.java
+++ b/solr/core/src/java/org/apache/solr/search/grouping/distributed/command/QueryCommand.java
@@ -16,7 +16,19 @@
*/
package org.apache.solr.search.grouping.distributed.command;
-import org.apache.lucene.search.*;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
+
+import org.apache.lucene.search.Collector;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.MultiCollector;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.TopDocs;
+import org.apache.lucene.search.TopDocsCollector;
+import org.apache.lucene.search.TopFieldCollector;
+import org.apache.lucene.search.TopScoreDocCollector;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.search.DocSet;
import org.apache.solr.search.MaxScoreCollector;
@@ -26,10 +38,6 @@ import org.apache.solr.search.SyntaxError;
import org.apache.solr.search.grouping.Command;
import org.apache.solr.search.grouping.collector.FilterCollector;
-import java.io.IOException;
-import java.util.Arrays;
-import java.util.List;
-
/**
*
*/
@@ -40,6 +48,7 @@ public class QueryCommand implements Command<QueryCommandResult> {
private Sort sort;
private String queryString;
private Query query;
+ private Query mainQuery;
private DocSet docSet;
private Integer docsToCollect;
private boolean needScores;
@@ -49,12 +58,29 @@ public class QueryCommand implements Command<QueryCommandResult> {
return this;
}
+ /**
+ * Sets the group query.
+ *
+ * @param query The {@link Query} used for grouping
+ * @return this
+ */
public Builder setQuery(Query query) {
this.query = query;
return this;
}
/**
+ * Sets the main query used for fetching results. This is mainly used for computing the scores.
+ *
+ * @param mainQuery The top-level query
+ * @return this
+ */
+ public Builder setMainQuery(Query mainQuery) {
+ this.mainQuery = mainQuery;
+ return this;
+ }
+
+ /**
* Sets the group query from the specified groupQueryString.
* The groupQueryString is parsed into a query.
*
@@ -95,11 +121,11 @@ public class QueryCommand implements Command<QueryCommandResult> {
}
public QueryCommand build() {
- if (sort == null || query == null || docSet == null || docsToCollect == null) {
+ if (sort == null || query == null || docSet == null || docsToCollect == null || mainQuery == null) {
throw new IllegalStateException("All fields must be set");
}
- return new QueryCommand(sort, query, docsToCollect, needScores, docSet, queryString);
+ return new QueryCommand(sort, query, docsToCollect, needScores, docSet, queryString, mainQuery);
}
}
@@ -110,19 +136,27 @@ public class QueryCommand implements Command<QueryCommandResult> {
private final int docsToCollect;
private final boolean needScores;
private final String queryString;
+ private final Query mainQuery;
private TopDocsCollector topDocsCollector;
private FilterCollector filterCollector;
private MaxScoreCollector maxScoreCollector;
private TopDocs topDocs;
- private QueryCommand(Sort sort, Query query, int docsToCollect, boolean needScores, DocSet docSet, String queryString) {
+ private QueryCommand(Sort sort,
+ Query query,
+ int docsToCollect,
+ boolean needScores,
+ DocSet docSet,
+ String queryString,
+ Query mainQuery) {
this.sort = sort;
this.query = query;
this.docsToCollect = docsToCollect;
this.needScores = needScores;
this.docSet = docSet;
this.queryString = queryString;
+ this.mainQuery = mainQuery;
}
@Override
@@ -147,7 +181,8 @@ public class QueryCommand implements Command<QueryCommandResult> {
public void postCollect(IndexSearcher searcher) throws IOException {
topDocs = topDocsCollector.topDocs();
if (needScores) {
- TopFieldCollector.populateScores(topDocs.scoreDocs, searcher, query);
+ // use mainQuery to populate the scores
+ TopFieldCollector.populateScores(topDocs.scoreDocs, searcher, mainQuery);
}
}
diff --git a/solr/core/src/java/org/apache/solr/search/grouping/distributed/shardresultserializer/TopGroupsResultTransformer.java b/solr/core/src/java/org/apache/solr/search/grouping/distributed/shardresultserializer/TopGroupsResultTransformer.java
index e736a9b..fb5bd32 100644
--- a/solr/core/src/java/org/apache/solr/search/grouping/distributed/shardresultserializer/TopGroupsResultTransformer.java
+++ b/solr/core/src/java/org/apache/solr/search/grouping/distributed/shardresultserializer/TopGroupsResultTransformer.java
@@ -52,7 +52,7 @@ import static org.apache.solr.common.params.CommonParams.ID;
/**
* Implementation for transforming {@link TopGroups} and {@link TopDocs} into a {@link NamedList} structure and
- * visa versa.
+ * vice versa.
*/
public class TopGroupsResultTransformer implements ShardResultTransformer<List<Command>, Map<String, ?>> {
diff --git a/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java b/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java
index 1cba72f..7b759d0 100644
--- a/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java
+++ b/solr/core/src/test/org/apache/solr/TestDistributedGrouping.java
@@ -324,6 +324,19 @@ public class TestDistributedGrouping extends BaseDistributedSearchTestCase {
query("q", "{!func}id_i1", "rows", 100, "fl", "score,id," + i1, "group", "true", "group.field", i1, "group.limit", -1, "sort", "score desc, _docid_ asc, id asc");
query("q", "{!func}id_i1", "rows", 100, "fl", "score,id," + i1, "group", "true", "group.field", i1, "group.limit", -1);
+ query("q", "*:*",
+ "group", "true",
+ "group.query", t1 + ":kings OR " + t1 + ":eggs", "group.limit", "3",
+ "fl", "id,score", "sort", i1 + " asc, id asc");
+ query("q", "*:*",
+ "group", "true",
+ "group.query", t1 + ":kings OR " + t1 + ":eggs", "group.limit", "3",
+ "fl", "id,score", "group.format", "simple", "sort", i1 + " asc, id asc");
+ query("q", "*:*",
+ "group", "true",
+ "group.query", t1 + ":kings OR " + t1 + ":eggs", "group.limit", "3",
+ "fl", "id,score", "group.main", "true", "sort", i1 + " asc, id asc");
+
// grouping shouldn't care if there are multiple fl params, or what order the fl field names are in
variantQuery(params("q", "*:*",
"group", "true", "group.field", i1dv, "group.limit", "10",
diff --git a/solr/core/src/test/org/apache/solr/TestGroupingSearch.java b/solr/core/src/test/org/apache/solr/TestGroupingSearch.java
index d885484..35ef3c6 100644
--- a/solr/core/src/test/org/apache/solr/TestGroupingSearch.java
+++ b/solr/core/src/test/org/apache/solr/TestGroupingSearch.java
@@ -575,6 +575,21 @@ public class TestGroupingSearch extends SolrTestCaseJ4 {
,"/grouped/id:1000=={'matches':10,'doclist':{'numFound':0,'start':0,'docs':[]}}"
);
+ // group.query and sort
+ assertJQ(req("fq",filt, "q","{!func}"+f2, "group","true", "group.query",f+":1", "fl","id,score", "rows","2", "group.limit","2", "sort",f+" desc, score desc", "indent","off")
+ ,"/grouped/"+f+":1==" +
+ "{'matches':10,'doclist':{'numFound':3,'start':0,'maxScore':10.0,'docs':[{'id':'8','score':10.0},{'id':'10','score':3.0}]}},"
+ );
+ // group.query with fl=score and default sort
+ assertJQ(req("fq",filt, "q","{!func}"+f2, "group","true", "group.query",f+":1", "fl","id,score", "rows","2", "group.limit","2", "sort", "score desc", "indent","off")
+ ,"/grouped/"+f+":1==" +
+ "{'matches':10,'doclist':{'numFound':3,'start':0,'maxScore':10.0,'docs':[{'id':'8','score':10.0},{'id':'10','score':3.0}]}},"
+ );
+ assertJQ(req("fq",filt, "q","{!func}"+f2, "group","true", "group.query",f+":1", "fl","id", "rows","2", "group.limit","2", "indent","off")
+ ,"/grouped/"+f+":1==" +
+ "{'matches':10,'doclist':{'numFound':3,'start':0,'docs':[{'id':'8'},{'id':'10'}]}},"
+ );
+
// group.query and offset
assertJQ(req("fq",filt, "q","{!func}"+f2, "group","true", "group.query","id:[2 TO 5]", "fl","id", "group.limit","3", "group.offset","2")
,"/grouped=={'id:[2 TO 5]':{'matches':10," +