madrob commented on a change in pull request #592:
URL: https://github.com/apache/solr/pull/592#discussion_r799513478
##########
File path: solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
##########
@@ -1299,6 +1302,41 @@ public DocList getDocList(Query query, List<Query>
filterList, Sort lsort, int o
public static final int GET_DOCLIST = 0x02; // get the documents actually
returned in a response
public static final int GET_SCORES = 0x01;
+ private static boolean isConstantScoreQuery(Query q) {
+ if (q instanceof BoostQuery) {
+ // ConstantScoreQueries are often (always?) wrapped in BoostQuery to
assign specific score
+ q = ((BoostQuery)q).getQuery();
+ }
+ return q instanceof ConstantScoreQuery;
+ }
+
+ private static boolean sortIncludesOtherThanScore(final Sort sort) {
+ if (sort == null) {
+ return false;
+ }
+ final SortField[] sortFields = sort.getSort();
+ return sortFields.length > 1 || sortFields[0].getType() != Type.SCORE;
+ }
+
+ private boolean useFilterCacheForDynamicScoreQuery(boolean needSort,
QueryCommand cmd) {
Review comment:
👍
##########
File path:
solr/core/src/test/org/apache/solr/core/ExitableDirectoryReaderTest.java
##########
@@ -98,7 +102,15 @@ public void testCacheAssumptions() throws Exception {
// This gets 0 docs back. Use 10000 instead of 1 for timeAllowed and it
gets 100 back and the for loop below
// succeeds.
- String response = JQ(req("q", "*:*", "fq", fq, "indent", "true",
"timeAllowed", "1", "sleep", sleep));
+ // TODO: address crosstalk between test methods; failures here can be
triggered by cache consultation
Review comment:
Is this still an issue after you added the special MatchAllDocs cases?
##########
File path: solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
##########
@@ -1299,6 +1302,41 @@ public DocList getDocList(Query query, List<Query>
filterList, Sort lsort, int o
public static final int GET_DOCLIST = 0x02; // get the documents actually
returned in a response
public static final int GET_SCORES = 0x01;
+ private static boolean isConstantScoreQuery(Query q) {
+ if (q instanceof BoostQuery) {
+ // ConstantScoreQueries are often (always?) wrapped in BoostQuery to
assign specific score
Review comment:
I suspect this partially goes away after #529
##########
File path: solr/core/src/test/org/apache/solr/search/TestMainQueryCaching.java
##########
@@ -0,0 +1,202 @@
+/*
+ * 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.util.Map;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.apache.solr.common.util.Utils.fromJSONString;
+
+/**
+ * Verify caching interactions between main query and filterCache
+ */
+public class TestMainQueryCaching extends SolrTestCaseJ4 {
+
+ static int NUM_DOCS = 100;
+ private static final String TEST_UFFSQ_PROPNAME =
"solr.test.useFilterForSortedQuery";
+ static String RESTORE_UFFSQ_PROP;
+ static boolean USE_FILTER_FOR_SORTED_QUERY;
+
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ final String uffsq = System.getProperty(TEST_UFFSQ_PROPNAME,
Boolean.toString(random().nextBoolean()));
+ USE_FILTER_FOR_SORTED_QUERY = Boolean.parseBoolean(uffsq);
+ RESTORE_UFFSQ_PROP = System.setProperty(TEST_UFFSQ_PROPNAME, uffsq);
+ initCore("solrconfig-deeppaging.xml", "schema-sorts.xml");
+ createIndex();
+ }
+
+ @AfterClass
+ public static void afterClass() throws Exception {
+ if (RESTORE_UFFSQ_PROP == null) {
+ System.clearProperty(TEST_UFFSQ_PROPNAME);
+ } else {
+ System.setProperty(TEST_UFFSQ_PROPNAME, RESTORE_UFFSQ_PROP);
+ }
+ }
+
+ public static void createIndex() {
+ for (int i = 0; i < NUM_DOCS; i++) {
+ assertU(adoc("id", Integer.toString(i), "str", "d" + i));
+ if (random().nextInt(NUM_DOCS) == 0) {
+ assertU(commit()); // sometimes make multiple segments
+ }
+ }
+ assertU(commit());
+ }
+
+ private static long coreToInserts(SolrCore core) {
+ return (long)((MetricsMap)((SolrMetricManager.GaugeWrapper<?>)core
+
.getCoreMetricManager().getRegistry().getMetrics().get("CACHE.searcher.filterCache")).getGauge())
+ .getValue().get("inserts");
+ }
+
+ private static long coreToSortCount(SolrCore core, String skipOrFull) {
+ return (long)((SolrMetricManager.GaugeWrapper<?>)core
+
.getCoreMetricManager().getRegistry().getMetrics().get("SEARCHER.searcher." +
skipOrFull + "SortCount")).getGauge()
+ .getValue();
+ }
+
+ @Test
+ public void testQueryCaching() throws Exception {
+ String q = "str:d*";
+ String constQ = "("+q+")^=1.0"; // wrapped as a ConstantScoreQuery
+
+ for (int i = 0; i < 6; i++) {
Review comment:
An interesting idea I just had was to randomize the order that we run
all of these cases. It _shouldn't_ matter because you reload in between but who
knows.
The other idea would be to split all of these out into separate test
methods, and give them a common assertMetricCounts helper method or something
like that.
##########
File path: solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
##########
@@ -1405,14 +1446,32 @@ private void getDocListC(QueryResult qr, QueryCommand
cmd) throws IOException {
out.docSet = getDocSet(cmd.getQuery(), cmd.getFilter());
List<Query> filterList = cmd.getFilterList();
if (filterList != null && !filterList.isEmpty()) {
- out.docSet = out.docSet.intersection(getDocSet(cmd.getFilterList()));
+ out.docSet =
DocSetUtil.getDocSet(out.docSet.intersection(getDocSet(filterList)), this);
}
}
// todo: there could be a sortDocSet that could take a list of
// the filters instead of anding them first...
// perhaps there should be a multi-docset-iterator
- sortDocSet(qr, cmd);
+ if (needSort) {
+ fullSortCount++;
+ sortDocSet(qr, cmd);
+ } else {
+ skipSortCount++;
+ // put unsorted list in place
+ out.docList = constantScoreDocList(cmd.getOffset(), cmd.getLen(),
out.docSet);
+ if (0 == cmd.getSupersetMaxDoc()) {
+ // this is the only case where `cursorMark && !needSort`
+ qr.setNextCursorMark(cmd.getCursorMark());
Review comment:
Is there a test that tries to do `cursorMark=*&rows=0`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
