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 b27b587 SOLR-14166 fq cache=false should use TwoPhaseIterator (#57)
b27b587 is described below
commit b27b58704c84b67375080a6e9f818fb7987c203b
Author: David Smiley <[email protected]>
AuthorDate: Sat May 1 18:48:52 2021 -0400
SOLR-14166 fq cache=false should use TwoPhaseIterator (#57)
instead of custom leapfrogging iterator thing, FilterImpl.
* Simplify SolrIndexSearcher.getDocSet and makeBitDocSet
* ExtendedQuery: remove getCacheSep & setCacheSep which never did anything
* Javadocs: Clarify ExtendedQuery vs PostFilter
* Add MatchCostQuery wrapper so that the cost local-param to "fq" is used
for TPI.matchCost
---
solr/CHANGES.txt | 3 +
.../solr/search/CollapsingQParserPlugin.java | 8 -
.../java/org/apache/solr/search/ExtendedQuery.java | 27 +-
.../org/apache/solr/search/ExtendedQueryBase.java | 14 -
.../solr/search/GraphTermsQParserPlugin.java | 17 +-
.../src/java/org/apache/solr/search/Grouping.java | 14 +-
.../org/apache/solr/search/MatchCostQuery.java | 158 ++++++++++
.../src/java/org/apache/solr/search/QParser.java | 2 -
.../apache/solr/search/SolrConstantScoreQuery.java | 9 -
.../org/apache/solr/search/SolrIndexSearcher.java | 328 ++++-----------------
.../src/test/org/apache/solr/core/SOLR749Test.java | 13 +-
.../apache/solr/search/SolrIndexSearcherTest.java | 9 -
12 files changed, 271 insertions(+), 331 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index d73b82c..1488563 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -283,6 +283,9 @@ Other Changes
use. The Jaeger plugin is now completely configurable via system properties
or environment vars,
not by values in solr.xml. (David Smiley)
+* SOLR-14166: Non-cached filter queries are now pushed down to Lucene,
possibly benefiting from
+ TwoPhaseIterator, which can make a big difference for some queries. (David
Smiley)
+
* SOLR-15369: PackageStoreAPI will only be loaded in SolrCloud mode (Mike Drob)
Bug Fixes
diff --git
a/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
b/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
index 9709f68..e1a64b0 100644
--- a/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
+++ b/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
@@ -310,14 +310,6 @@ public class CollapsingQParserPlugin extends QParserPlugin
{
}
- public void setCacheSep(boolean cacheSep) {
-
- }
-
- public boolean getCacheSep() {
- return false;
- }
-
public boolean getCache() {
return false;
}
diff --git a/solr/core/src/java/org/apache/solr/search/ExtendedQuery.java
b/solr/core/src/java/org/apache/solr/search/ExtendedQuery.java
index 5d1c069..e7346f8 100644
--- a/solr/core/src/java/org/apache/solr/search/ExtendedQuery.java
+++ b/solr/core/src/java/org/apache/solr/search/ExtendedQuery.java
@@ -16,6 +16,9 @@
*/
package org.apache.solr.search;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.TwoPhaseIterator;
+
/** The ExtendedQuery interface provides extra metadata to a query.
* Implementations of ExtendedQuery must also extend Query.
*/
@@ -25,15 +28,27 @@ public interface ExtendedQuery {
public void setCache(boolean cache);
- /** Returns the cost of this query, used to order checking of filters that
are not cached.
- * If getCache()==false && getCost()>=100 && this
instanceof PostFilter, then
- * the PostFilter interface will be used for filtering.
+ /**
+ * Returns the cost of this query, used to order checking of filters that
are not cached. If
+ * getCache()==false && getCost()>=100 && this instanceof
PostFilter, then the
+ * PostFilter interface will be used for filtering. Otherwise, for smaller
costs, this cost will
+ * be used for {@link TwoPhaseIterator#matchCost()}.
*/
public int getCost();
public void setCost(int cost);
- /** If true, the clauses of this boolean query should be cached separately.
This is not yet implemented. */
- public boolean getCacheSep();
- public void setCacheSep(boolean cacheSep);
+ /**
+ * Returns this Query, applying {@link QueryUtils#makeQueryable(Query)} and
maybe wrapping it to
+ * apply the {@link #getCost()} if it's above zero. Subclasses may customize
this if the query
+ * internally applies the configurable cost on the underlying {@link
+ * TwoPhaseIterator#matchCost()}.
+ */
+ public default Query getCostAppliedQuery() {
+ Query me = QueryUtils.makeQueryable((Query) this);
+ if (getCost() <= 0) {
+ return me;
+ }
+ return new MatchCostQuery(me, getCost());
+ }
}
diff --git a/solr/core/src/java/org/apache/solr/search/ExtendedQueryBase.java
b/solr/core/src/java/org/apache/solr/search/ExtendedQueryBase.java
index 06d2401..2a4a386 100644
--- a/solr/core/src/java/org/apache/solr/search/ExtendedQueryBase.java
+++ b/solr/core/src/java/org/apache/solr/search/ExtendedQueryBase.java
@@ -21,7 +21,6 @@ import org.apache.lucene.search.Query;
public abstract class ExtendedQueryBase extends Query implements ExtendedQuery
{
private int cost;
private boolean cache = true;
- private boolean cacheSep;
@Override
public void setCache(boolean cache) {
@@ -34,16 +33,6 @@ public abstract class ExtendedQueryBase extends Query
implements ExtendedQuery {
}
@Override
- public void setCacheSep(boolean cacheSep) {
- this.cacheSep = cacheSep;
- }
-
- @Override
- public boolean getCacheSep() {
- return cacheSep;
- }
-
- @Override
public void setCost(int cost) {
this.cost = cost;
}
@@ -67,9 +56,6 @@ public abstract class ExtendedQueryBase extends Query
implements ExtendedQuery {
sb.append(q.getCost());
}
sb.append("}");
- } else if (q.getCacheSep()) {
- sb.append("{!cache=sep");
- sb.append("}");
}
return sb.toString();
}
diff --git
a/solr/core/src/java/org/apache/solr/search/GraphTermsQParserPlugin.java
b/solr/core/src/java/org/apache/solr/search/GraphTermsQParserPlugin.java
index 2d3573d..151b660 100644
--- a/solr/core/src/java/org/apache/solr/search/GraphTermsQParserPlugin.java
+++ b/solr/core/src/java/org/apache/solr/search/GraphTermsQParserPlugin.java
@@ -155,7 +155,9 @@ public class GraphTermsQParserPlugin extends QParserPlugin {
};
}
+ /** Similar to {@code TermsQuery} but adds a {@code maxDocFreq}. */
private class GraphTermsQuery extends Query implements ExtendedQuery {
+ // Not a post filter. This will typically be used as the main query.
private Term[] queryTerms;
private String field;
@@ -181,20 +183,13 @@ public class GraphTermsQParserPlugin extends
QParserPlugin {
return false;
}
- public boolean getCacheSep() {
- return false;
- }
-
- public void setCacheSep(boolean sep) {
-
- }
-
public void setCache(boolean cache) {
-
+ // TODO support user choice
}
public int getCost() {
- return 1; // Not a post filter. The GraphTermsQuery will typically be
used as the main query.
+ // 0 is the default and keeping it avoids a needless wrapper for
TwoPhaseIterator matchCost.
+ return 0;
}
public void setCost(int cost) {
@@ -325,7 +320,7 @@ public class GraphTermsQParserPlugin extends QParserPlugin {
-// modified version of PointInSetQuery
+/** Modified version of {@code PointInSetQuery} to support {@code maxDocFreq}.
*/
abstract class PointSetQuery extends Query implements DocSetProducer,
Accountable {
protected static final long BASE_RAM_BYTES =
RamUsageEstimator.shallowSizeOfInstance(PointSetQuery.class);
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 343461d..8bbaf4d 100644
--- a/solr/core/src/java/org/apache/solr/search/Grouping.java
+++ b/solr/core/src/java/org/apache/solr/search/Grouping.java
@@ -102,8 +102,6 @@ public class Grouping {
private boolean getGroupedDocSet;
private boolean getDocList; // doclist needed for debugging or highlighting
private Query query;
- private DocSet filter;
- private Filter luceneFilter;
private NamedList<Object> grouped = new SimpleOrderedMap<>();
private Set<Integer> idSet = new LinkedHashSet<>(); // used for tracking
unique docs when we need a doclist
private int maxMatches; // max number of matches from any grouping command
@@ -304,7 +302,7 @@ public class Grouping {
qr.setDocListAndSet(out);
SolrIndexSearcher.ProcessedFilter pf =
searcher.getProcessedFilter(cmd.getFilter(), cmd.getFilterList());
- final Filter luceneFilter = pf.filter;
+ final Query filterQuery = pf.filter;
maxDoc = searcher.maxDoc();
needScores = (cmd.getFlags() & SolrIndexSearcher.GET_SCORES) != 0;
@@ -359,7 +357,7 @@ public class Grouping {
}
if (allCollectors != null) {
- searchWithTimeLimiter(luceneFilter, allCollectors);
+ searchWithTimeLimiter(filterQuery, allCollectors);
if(allCollectors instanceof DelegatingCollector) {
((DelegatingCollector) allCollectors).finish();
@@ -389,14 +387,14 @@ public class Grouping {
signalCacheWarning = true;
log.warn(String.format(Locale.ROOT, "The grouping cache is active,
but not used because it exceeded the max cache limit of %d percent",
maxDocsPercentageToCache));
log.warn("Please increase cache size or disable group caching.");
- searchWithTimeLimiter(luceneFilter, secondPhaseCollectors);
+ searchWithTimeLimiter(filterQuery, secondPhaseCollectors);
}
} else {
if (pf.postFilter != null) {
pf.postFilter.setLastDelegate(secondPhaseCollectors);
secondPhaseCollectors = pf.postFilter;
}
- searchWithTimeLimiter(luceneFilter, secondPhaseCollectors);
+ searchWithTimeLimiter(filterQuery, secondPhaseCollectors);
}
if (secondPhaseCollectors instanceof DelegatingCollector) {
((DelegatingCollector) secondPhaseCollectors).finish();
@@ -425,7 +423,7 @@ public class Grouping {
* Invokes search with the specified filter and collector.
* If a time limit has been specified, wrap the collector in a
TimeLimitingCollector
*/
- private void searchWithTimeLimiter(final Filter luceneFilter, Collector
collector) throws IOException {
+ private void searchWithTimeLimiter(final Query filterQuery, Collector
collector) throws IOException {
if (cmd.getTimeAllowed() > 0) {
if (timeLimitingCollector == null) {
timeLimitingCollector = new TimeLimitingCollector(collector,
TimeLimitingCollector.getGlobalCounter(), cmd.getTimeAllowed());
@@ -441,7 +439,7 @@ public class Grouping {
collector = timeLimitingCollector;
}
try {
- searcher.search(QueryUtils.combineQueryAndFilter(query, luceneFilter),
collector);
+ searcher.search(QueryUtils.combineQueryAndFilter(query, filterQuery),
collector);
} catch (TimeLimitingCollector.TimeExceededException |
ExitableDirectoryReader.ExitingReaderException x) {
log.warn("Query: {}; ", query, x);
qr.setPartialResults(true);
diff --git a/solr/core/src/java/org/apache/solr/search/MatchCostQuery.java
b/solr/core/src/java/org/apache/solr/search/MatchCostQuery.java
new file mode 100644
index 0000000..2523941
--- /dev/null
+++ b/solr/core/src/java/org/apache/solr/search/MatchCostQuery.java
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.search;
+
+import java.io.IOException;
+import java.util.Objects;
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.search.BulkScorer;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.Matches;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.QueryVisitor;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.search.Scorer;
+import org.apache.lucene.search.TwoPhaseIterator;
+import org.apache.lucene.search.Weight;
+
+/** Wraps a {@link Query} to customize the {@link
TwoPhaseIterator#matchCost()}. */
+public class MatchCostQuery extends Query {
+ private final Query delegate;
+ private final float matchCost;
+
+ public MatchCostQuery(Query delegate, float matchCost) {
+ this.delegate = delegate;
+ this.matchCost = matchCost;
+ assert matchCost >= 0;
+ }
+
+ @Override
+ public String toString(String field) {
+ return delegate.toString(field);
+ }
+
+ @Override
+ public void visit(QueryVisitor visitor) {
+ // don't visit our wrapper, just delegate
+ delegate.visit(visitor);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ return sameClassAs(other) &&
+ Objects.equals(delegate, ((MatchCostQuery) other).delegate);
+ }
+
+ @Override
+ public int hashCode() {
+ return 31 * classHash() + delegate.hashCode();
+ }
+
+ @Override
+ public Query rewrite(IndexReader reader) throws IOException {
+ final Query rewrite = delegate.rewrite(reader);
+ if (delegate == rewrite) {
+ return this; // unchanged
+ }
+ return new MatchCostQuery(rewrite, matchCost);
+ }
+
+ @Override
+ public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode,
float boost) throws IOException {
+ return new Weight(this) {
+ final Weight weight = delegate.createWeight(searcher, scoreMode, boost);
+
+ @Override
+ public boolean isCacheable(LeafReaderContext ctx) {
+ return weight.isCacheable(ctx);
+ }
+
+ @Override
+ public Matches matches(LeafReaderContext context, int doc) throws
IOException {
+ return weight.matches(context, doc);
+ }
+
+ @Override
+ public Explanation explain(LeafReaderContext context, int doc) throws
IOException {
+ return weight.explain(context, doc);
+ }
+
+ // do not delegate scorerSupplier(); use the default implementation that
calls our
+ // scorer() so that we can wrap TPI
+
+ @Override
+ public Scorer scorer(LeafReaderContext context) throws IOException {
+ final Scorer scorer = weight.scorer(context);
+ if (scorer == null) {
+ return null;
+ }
+ final TwoPhaseIterator tpi = scorer.twoPhaseIterator();
+ if (tpi == null || tpi.matchCost() == matchCost) {
+ return scorer; // needn't wrap/delegate
+ }
+ return new Scorer(weight) { // pass delegated weight
+
+ @Override
+ public TwoPhaseIterator twoPhaseIterator() {
+ return new TwoPhaseIterator(tpi.approximation()) {
+ @Override
+ public boolean matches() throws IOException {
+ return tpi.matches();
+ }
+
+ @Override
+ public float matchCost() {
+ return matchCost;
+ }
+ };
+ }
+
+ @Override
+ public DocIdSetIterator iterator() {
+ return scorer.iterator();
+ }
+
+ @Override
+ public float getMaxScore(int upTo) throws IOException {
+ return scorer.getMaxScore(upTo);
+ }
+
+ @Override
+ public float score() throws IOException {
+ return scorer.score();
+ }
+
+ @Override
+ public int docID() {
+ return scorer.docID();
+ }
+ };
+ }
+
+ // delegate because thus there's no need to care about TPI matchCost if
called
+ @Override
+ public BulkScorer bulkScorer(LeafReaderContext context) throws
IOException {
+ return weight.bulkScorer(context);
+ }
+ };
+
+ }
+}
diff --git a/solr/core/src/java/org/apache/solr/search/QParser.java
b/solr/core/src/java/org/apache/solr/search/QParser.java
index 5ebb05e..c472f4c 100644
--- a/solr/core/src/java/org/apache/solr/search/QParser.java
+++ b/solr/core/src/java/org/apache/solr/search/QParser.java
@@ -180,8 +180,6 @@ public abstract class QParser {
extendedQuery().setCache(false);
} else if (CommonParams.TRUE.equals(cacheStr)) {
extendedQuery().setCache(true);
- } else if ("sep".equals(cacheStr)) {
- extendedQuery().setCacheSep(true);
}
}
diff --git
a/solr/core/src/java/org/apache/solr/search/SolrConstantScoreQuery.java
b/solr/core/src/java/org/apache/solr/search/SolrConstantScoreQuery.java
index 82beecb..bc3b191 100644
--- a/solr/core/src/java/org/apache/solr/search/SolrConstantScoreQuery.java
+++ b/solr/core/src/java/org/apache/solr/search/SolrConstantScoreQuery.java
@@ -65,15 +65,6 @@ public class SolrConstantScoreQuery extends Query implements
ExtendedQuery {
}
@Override
- public void setCacheSep(boolean cacheSep) {
- }
-
- @Override
- public boolean getCacheSep() {
- return false;
- }
-
- @Override
public void setCost(int cost) {
this.cost = cost;
}
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 e1c9c29..b7fc3bc 100644
--- a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
+++ b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
@@ -28,16 +28,13 @@ import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
-
import com.codahale.metrics.Gauge;
import com.google.common.collect.Iterables;
-
import org.apache.lucene.document.Document;
import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.index.ExitableDirectoryReader;
@@ -51,6 +48,7 @@ import org.apache.lucene.index.StoredFieldVisitor;
import org.apache.lucene.index.Term;
import org.apache.lucene.index.Terms;
import org.apache.lucene.index.TermsEnum;
+import org.apache.lucene.search.BooleanClause.Occur;
import org.apache.lucene.search.*;
import org.apache.lucene.search.TotalHits.Relation;
import org.apache.lucene.store.AlreadyClosedException;
@@ -167,7 +165,7 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
UninvertingReader.wrap(reader,
core.getLatestSchema().getUninversionMapper()),
SolrQueryTimeoutImpl.getInstance());
}
-
+
/**
* Builds the necessary collector chain (via delegate wrapping) and executes
the query against it. This method takes
* into consideration both the explicitly provided collector and postFilter
as well as any needed collector wrappers
@@ -809,49 +807,6 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
return answerBits;
}
- /**
- * Returns the set of document ids matching a query. This method is
cache-aware and attempts to retrieve the answer
- * from the cache if possible. If the answer was not cached, it may have
been inserted into the cache as a result of
- * this call. This method can handle negative queries.
- * <p>
- * The DocSet returned should <b>not</b> be modified.
- */
- public DocSet getDocSet(Query query) throws IOException {
- if (query instanceof ExtendedQuery) {
- ExtendedQuery eq = (ExtendedQuery) query;
- if (!eq.getCache()) {
- if (query instanceof WrappedQuery) {
- query = ((WrappedQuery) query).getWrappedQuery();
- }
- query = QueryUtils.makeQueryable(query);
- return getDocSetNC(query, null);
- }
- }
-
- // Get the absolute value (positive version) of this query. If we
- // get back the same reference, we know it's positive.
- Query absQ = QueryUtils.getAbs(query);
- boolean positive = query == absQ;
-
- if (filterCache != null) {
- DocSet absAnswer = filterCache.get(absQ);
- if (absAnswer != null) {
- if (positive) return absAnswer;
- else return getLiveDocSet().andNot(absAnswer);
- }
- }
-
- DocSet absAnswer = getDocSetNC(absQ, null);
- DocSet answer = positive ? absAnswer : getLiveDocSet().andNot(absAnswer);
-
- if (filterCache != null) {
- // cache negative queries as positive
- filterCache.put(absQ, absAnswer);
- }
-
- return answer;
- }
-
// only handle positive (non negative) queries
DocSet getPositiveDocSet(Query q) throws IOException {
DocSet answer;
@@ -910,7 +865,8 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
this.liveDocs = makeBitDocSet(docs);
}
- private static Comparator<Query> sortByCost = (q1, q2) -> ((ExtendedQuery)
q1).getCost() - ((ExtendedQuery) q2).getCost();
+ private static Comparator<ExtendedQuery> sortByCost =
+ Comparator.comparingInt(ExtendedQuery::getCost);
/**
* Returns the set of document ids matching all queries. This method is
cache-aware and attempts to retrieve the
@@ -958,7 +914,7 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
*/
public static class ProcessedFilter {
public DocSet answer; // maybe null. Sometimes we have a docSet answer
that represents the complete answer / result.
- public Filter filter; // maybe null
+ public Query filter; // maybe null
public DelegatingCollector postFilter; // maybe null
}
@@ -978,13 +934,13 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
}
// We combine all the filter queries that come from the filter cache &
setFilter into "answer".
- // This might become pf.filterAsDocSet but not if there are any non-cached
filters
+ // This might become pf.answer but not if there are any non-cached filters
DocSet answer = null;
boolean[] neg = new boolean[queries.size() + 1];
DocSet[] sets = new DocSet[queries.size() + 1];
- List<Query> notCached = null;
- List<Query> postFilters = null;
+ List<ExtendedQuery> notCached = null;
+ List<PostFilter> postFilters = null;
int end = 0;
int smallestIndex = -1;
@@ -1001,10 +957,10 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
if (!eq.getCache()) {
if (eq.getCost() >= 100 && eq instanceof PostFilter) {
if (postFilters == null) postFilters = new ArrayList<>(sets.length
- end);
- postFilters.add(q);
+ postFilters.add((PostFilter) q);
} else {
if (notCached == null) notCached = new ArrayList<>(sets.length -
end);
- notCached.add(q);
+ notCached.add((ExtendedQuery) q);
}
continue;
}
@@ -1078,13 +1034,16 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
pf.filter = answer.getTopFilter();
}
} else {
- Collections.sort(notCached, sortByCost);
- List<Weight> weights = new ArrayList<>(notCached.size());
- for (Query q : notCached) {
- Query qq = QueryUtils.makeQueryable(q);
- weights.add(createWeight(rewrite(qq), ScoreMode.COMPLETE_NO_SCORES,
1));
+ notCached.sort(sortByCost); // pointless?
+ final BooleanQuery.Builder builder = new BooleanQuery.Builder();
+ if (answer != null) {
+ builder.add(answer.getTopFilter(), Occur.FILTER);
}
- pf.filter = new FilterImpl(answer, weights);
+ for (ExtendedQuery eq : notCached) {
+ Query q = eq.getCostAppliedQuery();
+ builder.add(q, Occur.FILTER);
+ }
+ pf.filter = builder.build();
}
// Set pf.postFilter
@@ -1092,7 +1051,7 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
Collections.sort(postFilters, sortByCost);
for (int i = postFilters.size() - 1; i >= 0; i--) {
DelegatingCollector prev = pf.postFilter;
- pf.postFilter = ((PostFilter)
postFilters.get(i)).getFilterCollector(this);
+ pf.postFilter = postFilters.get(i).getFilterCollector(this);
if (prev != null) pf.postFilter.setDelegate(prev);
}
}
@@ -1187,44 +1146,58 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
}
/**
- * Returns the set of document ids matching both the query and the filter.
This method is cache-aware and attempts to
- * retrieve the answer from the cache if possible. If the answer was not
cached, it may have been inserted into the
- * cache as a result of this call.
+ * Returns the set of document ids matching both the query.
+ * This method is cache-aware and attempts to retrieve a DocSet of the query
from the cache if possible.
+ * If the answer was not cached, it may have been inserted into the cache as
a result of this call.
*
- * @param filter
- * may be null
- * @return DocSet meeting the specified criteria, should <b>not</b> be
modified by the caller.
+ * @return Non-null DocSet meeting the specified criteria. Should
<b>not</b> be modified by the caller.
+ * @see #getDocSet(Query,DocSet)
*/
- public DocSet getDocSet(Query query, DocSet filter) throws IOException {
- if (filter == null) return getDocSet(query);
+ public DocSet getDocSet(Query query) throws IOException {
+ return getDocSet(query, null);
+ }
+ /**
+ * Returns the set of document ids matching both the query and the filter.
+ * This method is cache-aware and attempts to retrieve a DocSet of the query
from the cache if possible.
+ * If the answer was not cached, it may have been inserted into the cache as
a result of this call.
+ *
+ * @param filter may be null if none
+ * @return Non-null DocSet meeting the specified criteria. Should
<b>not</b> be modified by the caller.
+ */
+ public DocSet getDocSet(Query query, DocSet filter) throws IOException {
+ boolean doCache = filterCache != null;
if (query instanceof ExtendedQuery) {
- ExtendedQuery eq = (ExtendedQuery) query;
- if (!eq.getCache()) {
- if (query instanceof WrappedQuery) {
- query = ((WrappedQuery) query).getWrappedQuery();
- }
- query = QueryUtils.makeQueryable(query);
- return getDocSetNC(query, filter);
+ if (!((ExtendedQuery) query).getCache()) {
+ doCache = false;
+ }
+ if (query instanceof WrappedQuery) {
+ query = ((WrappedQuery) query).getWrappedQuery();
}
}
- // Negative query if absolute value different from original
+ if (!doCache) {
+ query = QueryUtils.makeQueryable(query);
+ return getDocSetNC(query, filter);
+ }
+
+ // Get the absolute value (positive version) of this query. If we
+ // get back the same reference, we know it's positive.
Query absQ = QueryUtils.getAbs(query);
boolean positive = absQ == query;
- DocSet first;
- if (filterCache != null) {
- first = filterCache.get(absQ);
- if (first == null) {
- first = getDocSetNC(absQ, null);
- filterCache.put(absQ, first);
- }
- return positive ? first.intersection(filter) : filter.andNot(first);
+ // note: can't use computeIfAbsent because can be recursive
+ DocSet absAnswer = filterCache.get(absQ);
+ if (absAnswer == null) {
+ absAnswer = getDocSetNC(absQ, null);
+ filterCache.put(absQ, absAnswer);
}
- // If there isn't a cache, then do a single filtered query if positive.
- return positive ? getDocSetNC(absQ, filter) :
filter.andNot(getPositiveDocSet(absQ));
+ if (filter == null) {
+ return positive ? absAnswer : getLiveDocSet().andNot(absAnswer);
+ } else {
+ return positive ? absAnswer.intersection(filter) :
filter.andNot(absAnswer);
+ }
}
/**
@@ -2086,8 +2059,8 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
// have deleted documents, but UninvertedField's doNegative has sets
with deleted docs
TotalHitCountCollector collector = new TotalHitCountCollector();
BooleanQuery.Builder bq = new BooleanQuery.Builder();
- bq.add(QueryUtils.makeQueryable(a), BooleanClause.Occur.MUST);
- bq.add(new ConstantScoreQuery(b.getTopFilter()),
BooleanClause.Occur.MUST);
+ bq.add(QueryUtils.makeQueryable(a), Occur.MUST);
+ bq.add(new ConstantScoreQuery(b.getTopFilter()), Occur.MUST);
super.search(bq.build(), collector);
return collector.getTotalHits();
}
@@ -2316,9 +2289,9 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
}), true, "statsCache", Category.CACHE.toString(), scope);
}
- /**
- * wraps a gauge (related to an IndexReader) and swallows any {@link
AlreadyClosedException} that
- * might be thrown, returning the specified default in it's place.
+ /**
+ * wraps a gauge (related to an IndexReader) and swallows any {@link
AlreadyClosedException} that
+ * might be thrown, returning the specified default in it's place.
*/
private <T> Gauge<T> rgauge(T closedDefault, Gauge<T> g) {
return () -> {
@@ -2329,178 +2302,7 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
}
};
}
-
- private static class FilterImpl extends Filter {
- private final Filter topFilter;
- private final List<Weight> weights;
-
- public FilterImpl(DocSet filter, List<Weight> weights) {
- this.weights = weights;
- this.topFilter = filter == null ? null : filter.getTopFilter();
- }
- @Override
- public DocIdSet getDocIdSet(LeafReaderContext context, Bits acceptDocs)
throws IOException {
- final DocIdSet sub = topFilter == null ? null :
topFilter.getDocIdSet(context, acceptDocs);
- if (weights.size() == 0) return sub;
- return new FilterSet(sub, context);
- }
-
- @Override
- public String toString(String field) {
- return "SolrFilter";
- }
-
- @Override
- public void visit(QueryVisitor visitor) {
- visitor.visitLeaf(this);
- }
-
- private class FilterSet extends DocIdSet {
- private final DocIdSet docIdSet;
- private final LeafReaderContext context;
-
- public FilterSet(DocIdSet docIdSet, LeafReaderContext context) {
- this.docIdSet = docIdSet;
- this.context = context;
- }
-
- @Override
- public DocIdSetIterator iterator() throws IOException {
- List<DocIdSetIterator> iterators = new ArrayList<>(weights.size() + 1);
- if (docIdSet != null) {
- final DocIdSetIterator iter = docIdSet.iterator();
- if (iter == null) return null;
- iterators.add(iter);
- }
- for (Weight w : weights) {
- final Scorer scorer = w.scorer(context);
- if (scorer == null) return null;
- iterators.add(scorer.iterator());
- }
- if (iterators.isEmpty()) return null;
- if (iterators.size() == 1) return iterators.get(0);
- if (iterators.size() == 2) return new
DualFilterIterator(iterators.get(0), iterators.get(1));
- return new FilterIterator(iterators.toArray(new
DocIdSetIterator[iterators.size()]));
- }
-
- @Override
- public Bits bits() throws IOException {
- return null; // don't use random access
- }
-
- @Override
- public long ramBytesUsed() {
- return docIdSet != null ? docIdSet.ramBytesUsed() : 0L;
- }
- }
-
- private static class FilterIterator extends DocIdSetIterator {
- private final DocIdSetIterator[] iterators;
- private final DocIdSetIterator first;
-
- public FilterIterator(DocIdSetIterator[] iterators) {
- this.iterators = iterators;
- this.first = iterators[0];
- }
-
- @Override
- public int docID() {
- return first.docID();
- }
-
- private int advanceAllTo(int doc) throws IOException {
- int highestDocIter = 0; // index of the iterator with the highest id
- int i = 1; // We already advanced the first iterator before calling
this method
- while (i < iterators.length) {
- if (i != highestDocIter) {
- final int next = iterators[i].advance(doc);
- if (next != doc) { // We need to advance all iterators to a new
target
- doc = next;
- highestDocIter = i;
- i = 0;
- continue;
- }
- }
- ++i;
- }
- return doc;
- }
-
- @Override
- public int nextDoc() throws IOException {
- return advanceAllTo(first.nextDoc());
- }
-
- @Override
- public int advance(int target) throws IOException {
- return advanceAllTo(first.advance(target));
- }
-
- @Override
- public long cost() {
- return first.cost();
- }
- }
-
- private static class DualFilterIterator extends DocIdSetIterator {
- private final DocIdSetIterator a;
- private final DocIdSetIterator b;
-
- public DualFilterIterator(DocIdSetIterator a, DocIdSetIterator b) {
- this.a = a;
- this.b = b;
- }
-
- @Override
- public int docID() {
- return a.docID();
- }
-
- @Override
- public int nextDoc() throws IOException {
- return doNext(a.nextDoc());
- }
-
- @Override
- public int advance(int target) throws IOException {
- return doNext(a.advance(target));
- }
-
- @Override
- public long cost() {
- return Math.min(a.cost(), b.cost());
- }
-
- private int doNext(int doc) throws IOException {
- for (;;) {
- int other = b.advance(doc);
- if (other == doc) return doc;
- doc = a.advance(other);
- if (other == doc) return doc;
- }
- }
-
- }
-
- @Override
- public boolean equals(Object other) {
- return sameClassAs(other) &&
- equalsTo(getClass().cast(other));
- }
-
- private boolean equalsTo(FilterImpl other) {
- return Objects.equals(this.topFilter, other.topFilter) &&
- Objects.equals(this.weights, other.weights);
- }
-
- @Override
- public int hashCode() {
- return classHash()
- + 31 * Objects.hashCode(topFilter)
- + 31 * Objects.hashCode(weights);
- }
- }
public long getWarmupTime() {
return warmupTime;
}
diff --git a/solr/core/src/test/org/apache/solr/core/SOLR749Test.java
b/solr/core/src/test/org/apache/solr/core/SOLR749Test.java
index d802899..9a8218f 100644
--- a/solr/core/src/test/org/apache/solr/core/SOLR749Test.java
+++ b/solr/core/src/test/org/apache/solr/core/SOLR749Test.java
@@ -25,7 +25,7 @@ import org.junit.BeforeClass;
/**
* This class started life as a test for SOLR-749 to prove that value source
plugins were properly
* intialized, but it has since evolved to also help prove that ValueSource's
are not asked to compute
- * values for documents unneccessarily.
+ * values for documents unnecessarily.
*
* @see CountUsageValueSourceParser
* @see <a href="https://issues.apache.org/jira/browse/SOLR-749";>SOLR-749</a>
@@ -116,6 +116,17 @@ public class SOLR749Test extends SolrTestCaseJ4 {
"//result[@numFound=0]");
assertEquals(0,
CountUsageValueSourceParser.getAndClearCount("postfilt_match_all"));
+ // Tests that TwoPhaseIterator is employed optimally
+ // note: map(countUsage(lowCost,0),0,0,id_i1) == return "id_i1" value &
keep track of access
+ assertQ("query matching 20 -> 10 -> 5 docs; two non-cached queries",
+ req("q","{!notfoo cache=false}id_i1:[20 TO 39]", // match 20
+ // the below IDs have alternating even/odd pairings so as to
test possible sequencing of evaluation
+ "fq","{!notfoo cache=false}id_i1:(20 21 25 26 28 29 31 32 36
37)", // match 10 (subset of above)
+ "fq","{!frange cache=false cost=5 l=21 u=99
v='map(countUsage(lowCost,0),0,0,id_i1)'})", // eliminate #20
+ "fq","{!frange cache=false cost=10 l=1
v='mod(map(countUsage(lastFilter,0),0,0,id_i1),2)'}"), // match 5 -- (the odd
ones since l=1 thus don't match 0)
+ "//result[@numFound=5]");
+ assertEquals(10,
CountUsageValueSourceParser.getAndClearCount("lowCost"));
+ assertEquals(9,
CountUsageValueSourceParser.getAndClearCount("lastFilter"));
} finally {
CountUsageValueSourceParser.clearCounters();
}
diff --git
a/solr/core/src/test/org/apache/solr/search/SolrIndexSearcherTest.java
b/solr/core/src/test/org/apache/solr/search/SolrIndexSearcherTest.java
index 58b1901..735a14f 100644
--- a/solr/core/src/test/org/apache/solr/search/SolrIndexSearcherTest.java
+++ b/solr/core/src/test/org/apache/solr/search/SolrIndexSearcherTest.java
@@ -420,15 +420,6 @@ public class SolrIndexSearcherTest extends SolrTestCaseJ4 {
public void setCost(int cost) {}
@Override
- public boolean getCacheSep() {
- return false;
- }
-
- @Override
- public void setCacheSep(boolean cacheSep) {
- }
-
- @Override
public DelegatingCollector getFilterCollector(IndexSearcher searcher) {
return new DelegatingCollector() {
private int collected = 0;
