This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new aec6e8f7500 Refactor: QueryResult: don't provide to SolrIndexSearcher
(#2524)
aec6e8f7500 is described below
commit aec6e8f750037fea5f8d01dc49dabf28bf512d68
Author: David Smiley <[email protected]>
AuthorDate: Wed Jul 3 10:16:22 2024 +0200
Refactor: QueryResult: don't provide to SolrIndexSearcher (#2524)
QueryResult refactoring so that it's only returned from SolrIndexSearcher
instead of
being provided to it.
Deprecated APIs in 9x; should be removed later.
Deprecated old methods in SolrIndexSearcher that are less clear than using
QueryCommand.
Deprecated related old methods in QueryPluginUtils; some should move to
DebugComponent.
---
solr/CHANGES.txt | 5 +-
.../apache/solr/handler/MoreLikeThisHandler.java | 16 +--
.../solr/handler/component/QueryComponent.java | 39 ++++---
.../java/org/apache/solr/search/QueryCommand.java | 6 ++
.../org/apache/solr/search/SolrIndexSearcher.java | 116 ++++++++++++---------
.../java/org/apache/solr/util/SolrPluginUtils.java | 14 ++-
.../org/apache/solr/search/TestSearchPerf.java | 9 +-
7 files changed, 125 insertions(+), 80 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 481c91eea78..b4c7dd8301a 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -136,7 +136,7 @@ Improvements
* SOLR-15591: Make using debugger in Solr easier by avoiding NPE in
ExternalPaths.determineSourceHome. (@charlygrappa via Eric Pugh)
* SOLR-16824: Adopt Linux standard pattern of -- for long option commands, and
make all commands "kebab" formatting. I.e -zkHost is now -zk-host. The old
parameters
- such as -zkHost continue to be supported in the 9.x line of Solr. -u is now
used to specify user credentials everywhere, this only impacts the bin/solr
assert
+ such as -zkHost continue to be supported in the 9.x line of Solr. -u is now
used to specify user credentials everywhere, this only impacts the bin/solr
assert
commands "same user" check which has -u as the short form of --same-user.
(Eric Pugh, janhoy, Jason Gerlowski)
Optimizations
@@ -203,6 +203,9 @@ Other Changes
* SOLR-17321: Remove Deprecated URL and replace it with URI in Preparation for
Java 21 (Sanjay Dutt, David Smiley, Uwe Schindler)
+* PR#2524: QueryResult refactoring so that it's only returned from
SolrIndexSearcher instead of
+ being provided to it. Deprecated APIs in 9x; should be removed later.
(David Smiley)
+
================== 9.6.1 ==================
Bug Fixes
---------------------
diff --git
a/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java
b/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java
index 8b4a292b0a1..e2a9572c7ab 100644
--- a/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java
@@ -63,6 +63,7 @@ import org.apache.solr.search.DocList;
import org.apache.solr.search.DocListAndSet;
import org.apache.solr.search.QParser;
import org.apache.solr.search.QParserPlugin;
+import org.apache.solr.search.QueryCommand;
import org.apache.solr.search.QueryLimits;
import org.apache.solr.search.QueryParsing;
import org.apache.solr.search.QueryUtils;
@@ -162,14 +163,15 @@ public class MoreLikeThisHandler extends
RequestHandlerBase {
if (reader != null) {
mltDocs = mlt.getMoreLikeThis(reader, start, rows, filters, flags);
} else if (q != null) {
- // Matching options
- boolean includeMatch =
params.getBool(MoreLikeThisParams.MATCH_INCLUDE, true);
- int matchOffset = params.getInt(MoreLikeThisParams.MATCH_OFFSET, 0);
- // Find the base match
DocList match =
- searcher.getDocList(
- query, null, null, matchOffset, 1, flags); // only get the
first one...
- if (includeMatch) {
+ new QueryCommand()
+ .setQuery(query)
+ .setOffset(params.getInt(MoreLikeThisParams.MATCH_OFFSET, 0))
+ .setLen(1) // only get the first one...
+ .setFlags(flags)
+ .search(searcher)
+ .getDocList();
+ if (params.getBool(MoreLikeThisParams.MATCH_INCLUDE, true)) {
rsp.add("match", match);
}
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 a421dcc42df..b7c080ec3b5 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
@@ -403,14 +403,9 @@ public class QueryComponent extends SearchComponent {
req.getContext().put(SolrIndexSearcher.STATS_SOURCE, statsCache.get(req));
- QueryResult result = new QueryResult();
-
cmd.setSegmentTerminateEarly(
params.getBool(
CommonParams.SEGMENT_TERMINATE_EARLY,
CommonParams.SEGMENT_TERMINATE_EARLY_DEFAULT));
- if (cmd.getSegmentTerminateEarly()) {
- result.setSegmentTerminatedEarly(Boolean.FALSE);
- }
//
// grouping / field collapsing
@@ -421,22 +416,20 @@ public class QueryComponent extends SearchComponent {
cmd.setSegmentTerminateEarly(false);
try {
if (params.getBool(GroupParams.GROUP_DISTRIBUTED_FIRST, false)) {
- doProcessGroupedDistributedSearchFirstPhase(rb, cmd, result);
- return;
+ doProcessGroupedDistributedSearchFirstPhase(rb, cmd);
} else if (params.getBool(GroupParams.GROUP_DISTRIBUTED_SECOND,
false)) {
- doProcessGroupedDistributedSearchSecondPhase(rb, cmd, result);
- return;
+ doProcessGroupedDistributedSearchSecondPhase(rb, cmd);
+ } else {
+ doProcessGroupedSearch(rb, cmd);
}
-
- doProcessGroupedSearch(rb, cmd, result);
- return;
} catch (SyntaxError e) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, e);
}
+ return;
}
// normal search result
- doProcessUngroupedSearch(rb, cmd, result);
+ doProcessUngroupedSearch(rb, cmd);
}
private int getMinExactCount(SolrParams params) {
@@ -1468,8 +1461,8 @@ public class QueryComponent extends SearchComponent {
return true;
}
- private void doProcessGroupedDistributedSearchFirstPhase(
- ResponseBuilder rb, QueryCommand cmd, QueryResult result) throws
IOException {
+ private void doProcessGroupedDistributedSearchFirstPhase(ResponseBuilder rb,
QueryCommand cmd)
+ throws IOException {
GroupingSpecification groupingSpec = rb.getGroupingSpec();
assert null != groupingSpec : "GroupingSpecification is null";
@@ -1501,13 +1494,14 @@ public class QueryComponent extends SearchComponent {
commandHandler.execute();
SearchGroupsResultTransformer serializer = new
SearchGroupsResultTransformer(searcher);
+ var result = new QueryResult();
rsp.add("firstPhase", commandHandler.processResult(result, serializer));
rsp.add("totalHitCount", commandHandler.getTotalHitCount());
rb.setResult(result);
}
- private void doProcessGroupedDistributedSearchSecondPhase(
- ResponseBuilder rb, QueryCommand cmd, QueryResult result) throws
IOException, SyntaxError {
+ private void doProcessGroupedDistributedSearchSecondPhase(ResponseBuilder
rb, QueryCommand cmd)
+ throws IOException, SyntaxError {
GroupingSpecification groupingSpec = rb.getGroupingSpec();
assert null != groupingSpec : "GroupingSpecification is null";
@@ -1590,11 +1584,12 @@ public class QueryComponent extends SearchComponent {
CommandHandler commandHandler = secondPhaseBuilder.build();
commandHandler.execute();
TopGroupsResultTransformer serializer = new TopGroupsResultTransformer(rb);
+ var result = new QueryResult();
rsp.add("secondPhase", commandHandler.processResult(result, serializer));
rb.setResult(result);
}
- private void doProcessGroupedSearch(ResponseBuilder rb, QueryCommand cmd,
QueryResult result)
+ private void doProcessGroupedSearch(ResponseBuilder rb, QueryCommand cmd)
throws IOException, SyntaxError {
GroupingSpecification groupingSpec = rb.getGroupingSpec();
@@ -1607,6 +1602,8 @@ public class QueryComponent extends SearchComponent {
SolrIndexSearcher searcher = req.getSearcher();
+ var result = new QueryResult();
+
int maxDocsPercentageToCache =
params.getInt(GroupParams.GROUP_CACHE_PERCENTAGE, 0);
boolean cacheSecondPassSearch =
maxDocsPercentageToCache >= 1 && maxDocsPercentageToCache <= 100;
@@ -1681,16 +1678,16 @@ public class QueryComponent extends SearchComponent {
}
}
- private void doProcessUngroupedSearch(ResponseBuilder rb, QueryCommand cmd,
QueryResult result)
- throws IOException {
+ private void doProcessUngroupedSearch(ResponseBuilder rb, QueryCommand cmd)
throws IOException {
SolrQueryRequest req = rb.req;
SolrQueryResponse rsp = rb.rsp;
SolrIndexSearcher searcher = req.getSearcher();
+ QueryResult result;
try {
- searcher.search(result, cmd);
+ result = searcher.search(cmd);
} catch (FuzzyTermsEnum.FuzzyTermsException e) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, e);
}
diff --git a/solr/core/src/java/org/apache/solr/search/QueryCommand.java
b/solr/core/src/java/org/apache/solr/search/QueryCommand.java
index 0622bb76567..b3daa182998 100755
--- a/solr/core/src/java/org/apache/solr/search/QueryCommand.java
+++ b/solr/core/src/java/org/apache/solr/search/QueryCommand.java
@@ -16,6 +16,7 @@
*/
package org.apache.solr.search;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import org.apache.lucene.search.Query;
@@ -239,4 +240,9 @@ public class QueryCommand {
public boolean isDistribStatsDisabled() {
return distribStatsDisabled;
}
+
+ /** Calls {@link SolrIndexSearcher#search(QueryCommand)}. */
+ public QueryResult search(SolrIndexSearcher searcher) throws IOException {
+ return searcher.search(this);
+ }
}
diff --git a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
index a8bc38b5699..8204487fed3 100644
--- a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
+++ b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
@@ -716,6 +716,12 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
}
}
+ /** Primary entrypoint for searching, using a {@link QueryCommand}. */
+ public QueryResult search(QueryCommand cmd) throws IOException {
+ return search(new QueryResult(), cmd);
+ }
+
+ @Deprecated
public QueryResult search(QueryResult qr, QueryCommand cmd) throws
IOException {
getDocListC(qr, cmd);
return qr;
@@ -1468,13 +1474,17 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
* @return DocList meeting the specified criteria, should <b>not</b> be
modified by the caller.
* @throws IOException If there is a low-level I/O error.
*/
+ @Deprecated
public DocList getDocList(Query query, Query filter, Sort lsort, int offset,
int len)
throws IOException {
- QueryCommand qc = new QueryCommand();
-
qc.setQuery(query).setFilterList(filter).setSort(lsort).setOffset(offset).setLen(len);
- QueryResult qr = new QueryResult();
- search(qr, qc);
- return qr.getDocList();
+ return new QueryCommand()
+ .setQuery(query)
+ .setFilterList(filter)
+ .setSort(lsort)
+ .setOffset(offset)
+ .setLen(len)
+ .search(this)
+ .getDocList();
}
/**
@@ -1493,19 +1503,19 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
* @return DocList meeting the specified criteria, should <b>not</b> be
modified by the caller.
* @throws IOException If there is a low-level I/O error.
*/
+ @Deprecated
public DocList getDocList(
Query query, List<Query> filterList, Sort lsort, int offset, int len,
int flags)
throws IOException {
- QueryCommand qc = new QueryCommand();
- qc.setQuery(query)
+ return new QueryCommand()
+ .setQuery(query)
.setFilterList(filterList)
.setSort(lsort)
.setOffset(offset)
.setLen(len)
- .setFlags(flags);
- QueryResult qr = new QueryResult();
- search(qr, qc);
- return qr.getDocList();
+ .setFlags(flags)
+ .search(this)
+ .getDocList();
}
public static final int NO_CHECK_QCACHE = 0x80000000;
@@ -1549,7 +1559,11 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
* getDocList version that uses+populates query and filter caches. In the
event of a timeout, the
* cache is not populated.
*/
- private void getDocListC(QueryResult qr, QueryCommand cmd) throws
IOException {
+ private QueryResult getDocListC(QueryResult qr, QueryCommand cmd) throws
IOException {
+ // TODO don't take QueryResult as arg; create one here
+ if (cmd.getSegmentTerminateEarly()) {
+ qr.setSegmentTerminatedEarly(Boolean.FALSE);
+ }
DocListAndSet out = new DocListAndSet();
qr.setDocListAndSet(out);
QueryResultKey key = null;
@@ -1606,7 +1620,7 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
out.docSet = getDocSet(newList);
}
}
- return;
+ return qr;
}
}
@@ -1743,6 +1757,7 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
if (key != null && superset.size() <= queryResultMaxDocsCached &&
!qr.isPartialResults()) {
queryResultCache.put(key, superset);
}
+ return qr;
}
private Relation populateScoresIfNeeded(
@@ -2120,12 +2135,15 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
* @return DocList meeting the specified criteria, should <b>not</b> be
modified by the caller.
* @throws IOException If there is a low-level I/O error.
*/
+ @Deprecated
public DocList getDocList(Query query, Sort lsort, int offset, int len)
throws IOException {
- QueryCommand qc = new QueryCommand();
- qc.setQuery(query).setSort(lsort).setOffset(offset).setLen(len);
- QueryResult qr = new QueryResult();
- search(qr, qc);
- return qr.getDocList();
+ return new QueryCommand()
+ .setQuery(query)
+ .setSort(lsort)
+ .setOffset(offset)
+ .setLen(len)
+ .search(this)
+ .getDocList();
}
/**
@@ -2148,18 +2166,18 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
* caller.
* @throws IOException If there is a low-level I/O error.
*/
+ @Deprecated
public DocListAndSet getDocListAndSet(Query query, Query filter, Sort lsort,
int offset, int len)
throws IOException {
- QueryCommand qc = new QueryCommand();
- qc.setQuery(query)
+ return new QueryCommand()
+ .setQuery(query)
.setFilterList(filter)
.setSort(lsort)
.setOffset(offset)
.setLen(len)
- .setNeedDocSet(true);
- QueryResult qr = new QueryResult();
- search(qr, qc);
- return qr.getDocListAndSet();
+ .setNeedDocSet(true)
+ .search(this)
+ .getDocListAndSet();
}
/**
@@ -2183,19 +2201,19 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
* caller.
* @throws IOException If there is a low-level I/O error.
*/
+ @Deprecated
public DocListAndSet getDocListAndSet(
Query query, Query filter, Sort lsort, int offset, int len, int flags)
throws IOException {
- QueryCommand qc = new QueryCommand();
- qc.setQuery(query)
+ return new QueryCommand()
+ .setQuery(query)
.setFilterList(filter)
.setSort(lsort)
.setOffset(offset)
.setLen(len)
.setFlags(flags)
- .setNeedDocSet(true);
- QueryResult qr = new QueryResult();
- search(qr, qc);
- return qr.getDocListAndSet();
+ .setNeedDocSet(true)
+ .search(this)
+ .getDocListAndSet();
}
/**
@@ -2218,18 +2236,18 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
* caller.
* @throws IOException If there is a low-level I/O error.
*/
+ @Deprecated
public DocListAndSet getDocListAndSet(
Query query, List<Query> filterList, Sort lsort, int offset, int len)
throws IOException {
- QueryCommand qc = new QueryCommand();
- qc.setQuery(query)
+ return new QueryCommand()
+ .setQuery(query)
.setFilterList(filterList)
.setSort(lsort)
.setOffset(offset)
.setLen(len)
- .setNeedDocSet(true);
- QueryResult qr = new QueryResult();
- search(qr, qc);
- return qr.getDocListAndSet();
+ .setNeedDocSet(true)
+ .search(this)
+ .getDocListAndSet();
}
/**
@@ -2254,20 +2272,20 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
* caller.
* @throws IOException If there is a low-level I/O error.
*/
+ @Deprecated
public DocListAndSet getDocListAndSet(
Query query, List<Query> filterList, Sort lsort, int offset, int len,
int flags)
throws IOException {
- QueryCommand qc = new QueryCommand();
- qc.setQuery(query)
+ return new QueryCommand()
+ .setQuery(query)
.setFilterList(filterList)
.setSort(lsort)
.setOffset(offset)
.setLen(len)
.setFlags(flags)
- .setNeedDocSet(true);
- QueryResult qr = new QueryResult();
- search(qr, qc);
- return qr.getDocListAndSet();
+ .setNeedDocSet(true)
+ .search(this)
+ .getDocListAndSet();
}
/**
@@ -2284,13 +2302,17 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
* caller.
* @throws IOException If there is a low-level I/O error.
*/
+ @Deprecated
public DocListAndSet getDocListAndSet(Query query, Sort lsort, int offset,
int len)
throws IOException {
- QueryCommand qc = new QueryCommand();
-
qc.setQuery(query).setSort(lsort).setOffset(offset).setLen(len).setNeedDocSet(true);
- QueryResult qr = new QueryResult();
- search(qr, qc);
- return qr.getDocListAndSet();
+ return new QueryCommand()
+ .setQuery(query)
+ .setSort(lsort)
+ .setOffset(offset)
+ .setLen(len)
+ .setNeedDocSet(true)
+ .search(this)
+ .getDocListAndSet();
}
private DocList constantScoreDocList(int offset, int length, DocSet docs) {
diff --git a/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java
b/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java
index a76ebe237a3..495e05da101 100644
--- a/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java
+++ b/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java
@@ -74,6 +74,7 @@ import org.apache.solr.search.DocIterator;
import org.apache.solr.search.DocList;
import org.apache.solr.search.FieldParams;
import org.apache.solr.search.QParser;
+import org.apache.solr.search.QueryCommand;
import org.apache.solr.search.QueryParsing;
import org.apache.solr.search.ReturnFields;
import org.apache.solr.search.SolrDocumentFetcher;
@@ -304,6 +305,7 @@ public class SolrPluginUtils {
* @return The debug info
* @throws java.io.IOException if there was an IO error
*/
+ @Deprecated // move to DebugComponent
public static NamedList<Object> doStandardDebug(
SolrQueryRequest req,
String userQuery,
@@ -318,6 +320,7 @@ public class SolrPluginUtils {
return dbg;
}
+ @Deprecated // move to DebugComponent
public static void doStandardQueryDebug(
SolrQueryRequest req,
String userQuery,
@@ -338,6 +341,7 @@ public class SolrPluginUtils {
}
}
+ @Deprecated
public static void doStandardResultsDebug(
SolrQueryRequest req, Query query, DocList results, boolean dbgResults,
NamedList<Object> dbg)
throws IOException {
@@ -430,6 +434,7 @@ public class SolrPluginUtils {
}
/** Executes a basic query */
+ @Deprecated
public static DocList doSimpleQuery(String sreq, SolrQueryRequest req, int
start, int limit)
throws IOException {
List<String> commands = StrUtils.splitSmart(sreq, ';');
@@ -445,8 +450,13 @@ public class SolrPluginUtils {
sort = SortSpecParsing.parseSortSpec(commands.get(1), req).getSort();
}
- DocList results = req.getSearcher().getDocList(query, sort, start,
limit);
- return results;
+ return new QueryCommand()
+ .setQuery(query)
+ .setSort(sort)
+ .setOffset(start)
+ .setLen(limit)
+ .search(req.getSearcher())
+ .getDocList();
} catch (SyntaxError e) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Error
parsing query: " + qs);
}
diff --git a/solr/core/src/test/org/apache/solr/search/TestSearchPerf.java
b/solr/core/src/test/org/apache/solr/search/TestSearchPerf.java
index bbbbf5f84b3..61ff7075aef 100644
--- a/solr/core/src/test/org/apache/solr/search/TestSearchPerf.java
+++ b/solr/core/src/test/org/apache/solr/search/TestSearchPerf.java
@@ -28,7 +28,6 @@ import org.apache.lucene.index.Term;
import org.apache.lucene.search.BooleanClause;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.Query;
-import org.apache.lucene.search.Sort;
import org.apache.lucene.search.TermQuery;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.common.SolrInputDocument;
@@ -173,7 +172,13 @@ public class TestSearchPerf extends SolrTestCaseJ4 {
long ret = 0;
for (int i = 0; i < ITERATIONS; i++) {
DocList l =
- searcher.getDocList(q, filt, (Sort) null, 0, 10,
SolrIndexSearcher.NO_CHECK_QCACHE);
+ new QueryCommand()
+ .setQuery(q)
+ .setFilterList(filt)
+ .setLen(10)
+ .setFlags(SolrIndexSearcher.NO_CHECK_QCACHE)
+ .search(searcher)
+ .getDocList();
ret += l.matches();
}
