You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2020/04/03 23:45:10 UTC
[lucene-solr] branch branch_8x updated: SOLR-14364: LTR SolrFeature
fq improvements Mostly general code improvements,
though it should support postFilters now Add
QueryUtils.combineQueryAndFilter
This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/branch_8x by this push:
new 8536215 SOLR-14364: LTR SolrFeature fq improvements Mostly general code improvements, though it should support postFilters now Add QueryUtils.combineQueryAndFilter
8536215 is described below
commit 853621561e9a5274d93dda03978963b69b9bf6d7
Author: David Smiley <ds...@apache.org>
AuthorDate: Thu Mar 26 00:29:46 2020 -0400
SOLR-14364: LTR SolrFeature fq improvements
Mostly general code improvements, though it should support postFilters now
Add QueryUtils.combineQueryAndFilter
(cherry picked from commit 7b3980c0804b0de16841faf16478b2b1bd217088)
---
solr/CHANGES.txt | 2 +
.../org/apache/solr/ltr/feature/SolrFeature.java | 201 ++++++---------------
.../solr/handler/component/ExpandComponent.java | 10 +-
.../src/java/org/apache/solr/search/Grouping.java | 10 +-
.../java/org/apache/solr/search/QueryUtils.java | 31 +++-
.../org/apache/solr/search/SolrIndexSearcher.java | 4 +-
.../solr/search/grouping/CommandHandler.java | 10 +-
7 files changed, 95 insertions(+), 173 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 0274caa..7f8bab0 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -28,6 +28,8 @@ Improvements
* SOLR-14307: User defined "<cache/>" entries in solrconfig.xml now support enabled="true|false" just like
core searcher caches. (hossman)
+* SOLR-14364: LTR's SolrFeature "fq" now supports PostFilters (e.g. collapse). (David Smiley)
+
Optimizations
---------------------
* SOLR-8306: Do not collect expand documents when expand.rows=0 (Marshall Sanders, Amelia Henderson)
diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/SolrFeature.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/SolrFeature.java
index ea69bcf..17c8fcb 100644
--- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/SolrFeature.java
+++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/SolrFeature.java
@@ -25,22 +25,23 @@ import java.util.Set;
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.index.Term;
-import org.apache.lucene.search.DocIdSet;
-import org.apache.lucene.search.DocIdSetIterator;
import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.MatchNoDocsQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.ScoreMode;
import org.apache.lucene.search.Scorer;
import org.apache.lucene.search.Weight;
-import org.apache.lucene.util.Bits;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.core.SolrCore;
import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.search.DocSet;
import org.apache.solr.search.QParser;
+import org.apache.solr.search.QueryUtils;
import org.apache.solr.search.SolrIndexSearcher;
import org.apache.solr.search.SyntaxError;
+
/**
* This feature allows you to reuse any Solr query as a feature. The value
* of the feature will be the score of the given query for the current document.
@@ -64,6 +65,8 @@ public class SolrFeature extends Feature {
private String q;
private List<String> fq;
+ // The setters will be invoked via reflection from the passed in params
+
public String getDf() {
return df;
}
@@ -111,7 +114,7 @@ public class SolrFeature extends Feature {
public FeatureWeight createWeight(IndexSearcher searcher, boolean needsScores,
SolrQueryRequest request, Query originalQuery, Map<String,String[]> efi)
throws IOException {
- return new SolrFeatureWeight(searcher, request, originalQuery, efi);
+ return new SolrFeatureWeight((SolrIndexSearcher) searcher, request, originalQuery, efi);
}
@Override
@@ -122,67 +125,68 @@ public class SolrFeature extends Feature {
": Q or FQ must be provided");
}
}
+
/**
* Weight for a SolrFeature
**/
public class SolrFeatureWeight extends FeatureWeight {
- final private Weight solrQueryWeight;
- final private Query query;
- final private List<Query> queryAndFilters;
+ private final Weight solrQueryWeight;
- public SolrFeatureWeight(IndexSearcher searcher,
- SolrQueryRequest request, Query originalQuery, Map<String,String[]> efi) throws IOException {
+ public SolrFeatureWeight(SolrIndexSearcher searcher,
+ SolrQueryRequest request, Query originalQuery, Map<String, String[]> efi) throws IOException {
super(SolrFeature.this, searcher, request, originalQuery, efi);
try {
- String solrQuery = q;
- final List<String> fqs = fq;
-
- if ((solrQuery == null) || solrQuery.isEmpty()) {
- solrQuery = "*:*";
- }
-
- solrQuery = macroExpander.expand(solrQuery);
- if (solrQuery == null) {
- throw new FeatureException(this.getClass().getSimpleName()+" requires efi parameter that was not passed in request.");
- }
-
- final SolrQueryRequest req = makeRequest(request.getCore(), solrQuery,
- fqs, df);
+ final SolrQueryRequest req = makeRequest(request.getCore(), q, fq, df);
if (req == null) {
throw new IOException("ERROR: No parameters provided");
}
+ // Build the scoring query
+ Query scoreQuery;
+ String qStr = q;
+ if (qStr == null || qStr.isEmpty()) {
+ scoreQuery = null; // ultimately behaves like MatchAllDocsQuery
+ } else {
+ qStr = macroExpander.expand(qStr);
+ if (qStr == null) {
+ throw new FeatureException(this.getClass().getSimpleName() + " requires efi parameter that was not passed in request.");
+ }
+ scoreQuery = QParser.getParser(qStr, req).getQuery();
+ // note: QParser can return a null Query sometimes, such as if the query is a stopword or just symbols
+ if (scoreQuery == null) {
+ scoreQuery = new MatchNoDocsQuery(); // debatable; all or none?
+ }
+ }
+
// Build the filter queries
- queryAndFilters = new ArrayList<Query>(); // If there are no fqs we just want an empty list
- if (fqs != null) {
- for (String fq : fqs) {
- if ((fq != null) && (fq.trim().length() != 0)) {
- fq = macroExpander.expand(fq);
- if (fq == null) {
- throw new FeatureException(this.getClass().getSimpleName()+" requires efi parameter that was not passed in request.");
+ Query filterDocSetQuery = null;
+ if (fq != null) {
+ List<Query> filterQueries = new ArrayList<>(); // If there are no fqs we just want an empty list
+ for (String fqStr : fq) {
+ if (fqStr != null) {
+ fqStr = macroExpander.expand(fqStr);
+ if (fqStr == null) {
+ throw new FeatureException(this.getClass().getSimpleName() + " requires efi parameter that was not passed in request.");
}
- final QParser fqp = QParser.getParser(fq, req);
- final Query filterQuery = fqp.getQuery();
+ final Query filterQuery = QParser.getParser(fqStr, req).getQuery();
if (filterQuery != null) {
- queryAndFilters.add(filterQuery);
+ filterQueries.add(filterQuery);
}
}
}
+
+ if (filterQueries.isEmpty() == false) { // TODO optimize getDocSet to make this check unnecessary SOLR-14376
+ DocSet filtersDocSet = searcher.getDocSet(filterQueries); // execute
+ if (filtersDocSet != searcher.getLiveDocSet()) {
+ filterDocSetQuery = filtersDocSet.getTopFilter();
+ }
+ }
}
- final QParser parser = QParser.getParser(solrQuery, req);
- query = parser.parse();
+ Query query = QueryUtils.combineQueryAndFilter(scoreQuery, filterDocSetQuery);
+
+ solrQueryWeight = searcher.createWeight(searcher.rewrite(query), ScoreMode.COMPLETE, 1);
- // Query can be null if there was no input to parse, for instance if you
- // make a phrase query with "to be", and the analyzer removes all the
- // words
- // leaving nothing for the phrase query to parse.
- if (query != null) {
- queryAndFilters.add(query);
- solrQueryWeight = searcher.createWeight(searcher.rewrite(query), ScoreMode.COMPLETE, 1);
- } else {
- solrQueryWeight = null;
- }
} catch (final SyntaxError e) {
throw new FeatureException("Failed to parse feature query.", e);
}
@@ -218,67 +222,26 @@ public class SolrFeature extends Feature {
@Override
public FeatureScorer scorer(LeafReaderContext context) throws IOException {
- Scorer solrScorer = null;
- if (solrQueryWeight != null) {
- solrScorer = solrQueryWeight.scorer(context);
- }
-
- final DocIdSetIterator idItr = getDocIdSetIteratorFromQueries(
- queryAndFilters, context);
- if (idItr != null) {
- return solrScorer == null ? new ValueFeatureScorer(this, 1f, idItr)
- : new SolrFeatureScorer(this, solrScorer,
- new SolrFeatureScorerIterator(idItr, solrScorer.iterator()));
- } else {
+ Scorer solrScorer = solrQueryWeight.scorer(context);
+ if (solrScorer == null) {
return null;
}
- }
-
- /**
- * Given a list of Solr filters/queries, return a doc iterator that
- * traverses over the documents that matched all the criteria of the
- * queries.
- *
- * @param queries
- * Filtering criteria to match documents against
- * @param context
- * Index reader
- * @return DocIdSetIterator to traverse documents that matched all filter
- * criteria
- */
- private DocIdSetIterator getDocIdSetIteratorFromQueries(List<Query> queries,
- LeafReaderContext context) throws IOException {
- final SolrIndexSearcher.ProcessedFilter pf = ((SolrIndexSearcher) searcher)
- .getProcessedFilter(null, queries);
- final Bits liveDocs = context.reader().getLiveDocs();
-
- DocIdSetIterator idIter = null;
- if (pf.filter != null) {
- final DocIdSet idSet = pf.filter.getDocIdSet(context, liveDocs);
- if (idSet != null) {
- idIter = idSet.iterator();
- }
- }
-
- return idIter;
+ return new SolrFeatureScorer(this, solrScorer);
}
/**
* Scorer for a SolrFeature
- **/
- public class SolrFeatureScorer extends FeatureScorer {
- final private Scorer solrScorer;
+ */
+ public class SolrFeatureScorer extends FilterFeatureScorer {
- public SolrFeatureScorer(FeatureWeight weight, Scorer solrScorer,
- SolrFeatureScorerIterator itr) {
- super(weight, itr);
- this.solrScorer = solrScorer;
+ public SolrFeatureScorer(FeatureWeight weight, Scorer solrScorer) {
+ super(weight, solrScorer);
}
@Override
public float score() throws IOException {
try {
- return solrScorer.score();
+ return in.score();
} catch (UnsupportedOperationException e) {
throw new FeatureException(
e.toString() + ": " +
@@ -287,54 +250,6 @@ public class SolrFeature extends Feature {
}
}
- @Override
- public float getMaxScore(int upTo) throws IOException {
- return Float.POSITIVE_INFINITY;
- }
- }
-
- /**
- * An iterator that allows to iterate only on the documents for which a feature has
- * a value.
- **/
- public class SolrFeatureScorerIterator extends DocIdSetIterator {
-
- final private DocIdSetIterator filterIterator;
- final private DocIdSetIterator scorerFilter;
-
- SolrFeatureScorerIterator(DocIdSetIterator filterIterator,
- DocIdSetIterator scorerFilter) {
- this.filterIterator = filterIterator;
- this.scorerFilter = scorerFilter;
- }
-
- @Override
- public int docID() {
- return filterIterator.docID();
- }
-
- @Override
- public int nextDoc() throws IOException {
- int docID = filterIterator.nextDoc();
- scorerFilter.advance(docID);
- return docID;
- }
-
- @Override
- public int advance(int target) throws IOException {
- // We use iterator to catch the scorer up since
- // that checks if the target id is in the query + all the filters
- int docID = filterIterator.advance(target);
- scorerFilter.advance(docID);
- return docID;
- }
-
- @Override
- public long cost() {
- return filterIterator.cost() + scorerFilter.cost();
- }
-
}
}
-
-}
+}
\ No newline at end of file
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 bca7e40..b15206a 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
@@ -39,8 +39,6 @@ import org.apache.lucene.index.MultiDocValues;
import org.apache.lucene.index.NumericDocValues;
import org.apache.lucene.index.OrdinalMap;
import org.apache.lucene.index.SortedDocValues;
-import org.apache.lucene.search.BooleanClause.Occur;
-import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.Collector;
import org.apache.lucene.search.DocIdSetIterator;
import org.apache.lucene.search.LeafCollector;
@@ -81,6 +79,7 @@ import org.apache.solr.search.DocIterator;
import org.apache.solr.search.DocList;
import org.apache.solr.search.DocSlice;
import org.apache.solr.search.QParser;
+import org.apache.solr.search.QueryUtils;
import org.apache.solr.search.ReturnFields;
import org.apache.solr.search.SolrIndexSearcher;
import org.apache.solr.search.SortSpecParsing;
@@ -416,12 +415,7 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia
collector = groupExpandCollector;
}
- if (pfilter.filter != null) {
- query = new BooleanQuery.Builder()
- .add(query, Occur.MUST)
- .add(pfilter.filter, Occur.FILTER)
- .build();
- }
+ query = QueryUtils.combineQueryAndFilter(query, pfilter.filter);
searcher.search(query, collector);
ReturnFields returnFields = rb.rsp.getReturnFields();
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 2e43da1..55e511c 100644
--- a/solr/core/src/java/org/apache/solr/search/Grouping.java
+++ b/solr/core/src/java/org/apache/solr/search/Grouping.java
@@ -33,8 +33,6 @@ import org.apache.lucene.index.IndexableField;
import org.apache.lucene.queries.function.FunctionQuery;
import org.apache.lucene.queries.function.ValueSource;
import org.apache.lucene.queries.function.valuesource.QueryValueSource;
-import org.apache.lucene.search.BooleanClause.Occur;
-import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.CachingCollector;
import org.apache.lucene.search.Collector;
import org.apache.lucene.search.MultiCollector;
@@ -440,13 +438,7 @@ public class Grouping {
collector = timeLimitingCollector;
}
try {
- Query q = query;
- if (luceneFilter != null) {
- q = new BooleanQuery.Builder()
- .add(q, Occur.MUST)
- .add(luceneFilter, Occur.FILTER)
- .build();
- }
+ Query q = QueryUtils.combineQueryAndFilter(query, luceneFilter);
searcher.search(q, collector);
} catch (TimeLimitingCollector.TimeExceededException | ExitableDirectoryReader.ExitingReaderException x) {
log.warn( "Query: " + query + "; " + x.getMessage() );
diff --git a/solr/core/src/java/org/apache/solr/search/QueryUtils.java b/solr/core/src/java/org/apache/solr/search/QueryUtils.java
index 3e2b6a0..daab005 100644
--- a/solr/core/src/java/org/apache/solr/search/QueryUtils.java
+++ b/solr/core/src/java/org/apache/solr/search/QueryUtils.java
@@ -16,16 +16,17 @@
*/
package org.apache.solr.search;
+import java.util.Collection;
+
import org.apache.lucene.search.BooleanClause;
import org.apache.lucene.search.BooleanClause.Occur;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.BoostQuery;
+import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.MatchAllDocsQuery;
import org.apache.lucene.search.Query;
import org.apache.solr.common.SolrException;
-import java.util.Collection;
-
/**
*
*/
@@ -140,4 +141,30 @@ public class QueryUtils {
}
return bq;
}
+
+ /**
+ * Combines a scoring query with a non-scoring (filter) query.
+ * If both parameters are null then return a {@link MatchAllDocsQuery}.
+ * If only {@code scoreQuery} is present then return it.
+ * If only {@code filterQuery} is present then return it wrapped with constant scoring.
+ * If neither are null then we combine with a BooleanQuery.
+ */
+ public static Query combineQueryAndFilter(Query scoreQuery, Query filterQuery) {
+ if (scoreQuery == null) {
+ if (filterQuery == null) {
+ return new MatchAllDocsQuery(); // default if nothing -- match everything
+ } else {
+ return new ConstantScoreQuery(filterQuery);
+ }
+ } else {
+ if (filterQuery == null) {
+ return scoreQuery;
+ } else {
+ return new BooleanQuery.Builder()
+ .add(scoreQuery, Occur.MUST)
+ .add(filterQuery, Occur.FILTER)
+ .build();
+ }
+ }
+ }
}
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 1b0fcdb..ecca345 100644
--- a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
+++ b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
@@ -1552,9 +1552,7 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI
Query query = QueryUtils.makeQueryable(cmd.getQuery());
ProcessedFilter pf = getProcessedFilter(cmd.getFilter(), cmd.getFilterList());
- if (pf.filter != null) {
- query = new BooleanQuery.Builder().add(query, Occur.MUST).add(pf.filter, Occur.FILTER).build();
- }
+ query = QueryUtils.combineQueryAndFilter(query, pf.filter);
// handle zero case...
if (lastDocRequested <= 0) {
diff --git a/solr/core/src/java/org/apache/solr/search/grouping/CommandHandler.java b/solr/core/src/java/org/apache/solr/search/grouping/CommandHandler.java
index 336c27b..8fe1a6b 100644
--- a/solr/core/src/java/org/apache/solr/search/grouping/CommandHandler.java
+++ b/solr/core/src/java/org/apache/solr/search/grouping/CommandHandler.java
@@ -24,8 +24,6 @@ import java.util.List;
import org.apache.lucene.index.ExitableDirectoryReader;
import org.apache.lucene.queries.function.ValueSource;
-import org.apache.lucene.search.BooleanClause.Occur;
-import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.Collector;
import org.apache.lucene.search.MultiCollector;
import org.apache.lucene.search.Query;
@@ -228,12 +226,8 @@ public class CommandHandler {
collector = MultiCollector.wrap(collector, hitCountCollector);
}
- if (filter.filter != null) {
- query = new BooleanQuery.Builder()
- .add(query, Occur.MUST)
- .add(filter.filter, Occur.FILTER)
- .build();
- }
+ query = QueryUtils.combineQueryAndFilter(query, filter.filter);
+
if (filter.postFilter != null) {
filter.postFilter.setLastDelegate(collector);
collector = filter.postFilter;