You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mk...@apache.org on 2018/04/26 06:41:21 UTC
lucene-solr:branch_7x: SOLR-12275: fix caching for \{!filters} and
{{filters}} in \{!parent} and \{!child}
Repository: lucene-solr
Updated Branches:
refs/heads/branch_7x c1865dfea -> ae9f79f76
SOLR-12275: fix caching for \{!filters} and {{filters}} in \{!parent} and \{!child}
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/ae9f79f7
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/ae9f79f7
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/ae9f79f7
Branch: refs/heads/branch_7x
Commit: ae9f79f7647e5fff98341608972286db68d3fd86
Parents: c1865df
Author: Mikhail Khludnev <mk...@apache.org>
Authored: Thu Apr 26 09:40:26 2018 +0300
Committer: Mikhail Khludnev <mk...@apache.org>
Committed: Thu Apr 26 09:41:48 2018 +0300
----------------------------------------------------------------------
solr/CHANGES.txt | 3 ++
.../apache/solr/search/join/FiltersQParser.java | 38 ++-----------
.../apache/solr/search/join/BJQParserTest.java | 57 ++++++++++++++++++--
3 files changed, 61 insertions(+), 37 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ae9f79f7/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 0aa8f4e..bc6702b 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -171,6 +171,9 @@ Bug Fixes
get an error that's it's a member of that alias. This check now ensures the alias state is sync()'ed with ZK first.
(David Smiley)
+* SOLR-12275: wrong caching for {!filters} as well as for `filters` local param in {!parent} and {!child}
+ (David Smiley, Mikhail Khluldnev)
+
Optimizations
----------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ae9f79f7/solr/core/src/java/org/apache/solr/search/join/FiltersQParser.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/search/join/FiltersQParser.java b/solr/core/src/java/org/apache/solr/search/join/FiltersQParser.java
index 6c2172e..a3557ff 100644
--- a/solr/core/src/java/org/apache/solr/search/join/FiltersQParser.java
+++ b/solr/core/src/java/org/apache/solr/search/join/FiltersQParser.java
@@ -17,8 +17,6 @@
package org.apache.solr.search.join;
-import java.io.IOException;
-import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.IdentityHashMap;
@@ -36,31 +34,10 @@ import org.apache.solr.common.util.StrUtils;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.search.QParser;
import org.apache.solr.search.QueryParsing;
-import org.apache.solr.search.SolrConstantScoreQuery;
import org.apache.solr.search.SyntaxError;
public class FiltersQParser extends QParser {
- private static final class FiltersQuery extends SolrConstantScoreQuery {
-
- private Set<Query> filters;
-
- private FiltersQuery(SolrQueryRequest req, Set<Query> filters) throws IOException {
- super(req.getSearcher().getDocSet(new ArrayList<>(filters)).getTopFilter());
- this.filters = filters;
- }
-
- @Override
- public boolean equals(Object other) {
- return sameClassAs(other) && filters.equals(((FiltersQuery)other).filters);
- }
-
- @Override
- public int hashCode() {
- return 31 * classHash() + filters.hashCode();
- }
- }
-
protected String getFiltersParamName() {
return "param";
}
@@ -104,21 +81,14 @@ public class FiltersQParser extends QParser {
/** @return number of added clauses */
protected int addFilters(BooleanQuery.Builder builder, Map<Query,Occur> clauses) throws SyntaxError {
- Set<Query> filters = new HashSet<>();
+ int count=0;
for (Map.Entry<Query, Occur> clause: clauses.entrySet()) {
if (clause.getValue() == Occur.FILTER) {
- filters.add(clause.getKey());
- }
- }
- if (!filters.isEmpty()) {
- try {
- final SolrConstantScoreQuery intersQuery = new FiltersQuery(req, filters);
- builder.add( intersQuery, Occur.FILTER);
- } catch (IOException e) {
- throw new SyntaxError("Exception occurs while parsing " + stringIncludingLocalParams, e);
+ builder.add( clause.getKey(), Occur.FILTER);
+ count++;
}
}
- return filters.size();
+ return count;
}
protected void exclude(Map<Query,Occur> clauses) {
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ae9f79f7/solr/core/src/test/org/apache/solr/search/join/BJQParserTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/search/join/BJQParserTest.java b/solr/core/src/test/org/apache/solr/search/join/BJQParserTest.java
index 97101f1..c9dfe7a 100644
--- a/solr/core/src/test/org/apache/solr/search/join/BJQParserTest.java
+++ b/solr/core/src/test/org/apache/solr/search/join/BJQParserTest.java
@@ -18,6 +18,7 @@ package org.apache.solr.search.join;
import javax.xml.xpath.XPathConstants;
import java.io.IOException;
+import java.lang.invoke.MethodHandles;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -26,18 +27,26 @@ import java.util.ListIterator;
import java.util.Locale;
import java.util.Map;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.TopDocs;
import org.apache.lucene.search.join.ScoreMode;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.metrics.MetricsMap;
import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.search.QParser;
+import org.apache.solr.search.SyntaxError;
import org.apache.solr.util.BaseTestHarness;
import org.junit.BeforeClass;
import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class BJQParserTest extends SolrTestCaseJ4 {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
private static final String[] klm = new String[] {"k", "l", "m"};
private static final List<String> xyz = Arrays.asList("x", "y", "z");
private static final String[] abcdef = new String[] {"a", "b", "c", "d", "e", "f"};
@@ -389,8 +398,8 @@ public class BJQParserTest extends SolrTestCaseJ4 {
}
private final static String elChild[] = new String[]{"//*[@numFound='1']",
- "//doc[" +
- "arr[@name=\"child_s\"]/str='l' and child::arr[@name=\"childparent_s\"]/str='e']"};
+ "//doc[" +
+ "arr[@name=\"child_s\"]/str='l' and arr[@name=\"childparent_s\"]/str='e']"};
@Test
@@ -441,10 +450,52 @@ public class BJQParserTest extends SolrTestCaseJ4 {
"child.fq", "{!tag=secondTag}child_s:l", // 6 ls remains
"gchq", "{!tag=top}childparent_s:e"), "//*[@numFound='6']");
- assertQ(req("q", // top and filter are excluded, got zero result, but is it right?
+ assertQ(req("q", // top and filter are excluded, got all results
"{!filters excludeTags=bot,secondTag,top v=$gchq}" ,
"child.fq", "{!tag=secondTag}child_s:l",
"gchq", "{!tag=top}childparent_s:e"), "//*[@numFound='42']");
}
+
+ @Test
+ public void testFiltersCache() throws SyntaxError, IOException {
+ final String [] elFilterQuery = new String[] {"q", "{!filters param=$child.fq v=$gchq}",
+ "child.fq", "childparent_s:e",
+ "child.fq", "child_s:l",
+ "gchq", "child_s:[* TO *]"};
+ assertQ(req(elFilterQuery), elChild);
+ final Query query;
+ {
+ final SolrQueryRequest req = req(elFilterQuery);
+ try {
+ QParser parser = QParser.getParser(req.getParams().get("q"), null, req);
+ query = parser.getQuery();
+ final TopDocs topDocs = req.getSearcher().search(query, 10);
+ assertEquals(1, topDocs.totalHits);
+ }finally {
+ req.close();
+ }
+ }
+ assertU(adoc("id", "12275",
+ "child_s", "l", "childparent_s", "e"));
+ assertU(commit());
+ try {
+ assertQ("here we rely on autowarming for cathing cache leak", //cache=false
+ req(elFilterQuery), "//*[@numFound='2']");
+ final SolrQueryRequest req = req();
+ try {
+ final TopDocs topDocs = req.getSearcher().search(query, 10);
+ assertEquals("expecting new doc is visible to old query", 2, topDocs.totalHits);
+ }finally {
+ req.close();
+ }
+ }finally {
+ try {
+ assertU(delI("12275"));
+ assertU(commit());
+ } catch(Throwable t) {
+ log.error("ignoring exception occuring in compensation routine", t);
+ }
+ }
+ }
}