From e683af1c7bdece1d7100852323c0a32b12bfd566 Mon Sep 17 00:00:00 2001 From: Elia Porciani Date: Sat, 28 May 2022 00:16:54 +0900 Subject: [PATCH 01/10] SOLR-16567: KnnQueryParser support for both pre-filters and post-filters(cost>0) --- .../apache/solr/search/neural/KnnQParser.java | 49 ++++++++++++------- .../solr/search/neural/KnnQParserTest.java | 39 +++++++++++++++ 2 files changed, 71 insertions(+), 17 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java b/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java index 7255562d8fe..714cf65c277 100644 --- a/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java +++ b/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java @@ -16,7 +16,6 @@ */ package org.apache.solr.search.neural; -import java.util.List; import org.apache.commons.lang3.StringUtils; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; @@ -28,11 +27,15 @@ import org.apache.solr.schema.DenseVectorField; import org.apache.solr.schema.FieldType; import org.apache.solr.schema.SchemaField; +import org.apache.solr.search.ExtendedQuery; import org.apache.solr.search.QParser; import org.apache.solr.search.QueryParsing; import org.apache.solr.search.QueryUtils; import org.apache.solr.search.SyntaxError; +import java.util.List; +import java.util.stream.Collectors; + public class KnnQParser extends QParser { // retrieve the top K results based on the distance similarity function @@ -80,37 +83,49 @@ public Query parse() { float[] parsedVectorToSearch = parseVector(vectorToSearch, denseVectorType.getDimension()); return denseVectorType.getKnnVectorQuery( - schemaField.getName(), parsedVectorToSearch, topK, getFilterQuery()); + schemaField.getName(), parsedVectorToSearch, topK, buildPreFilter()); } - private Query getFilterQuery() throws SolrException { - if (!isFilter()) { + private Query buildPreFilter() throws SolrException { + boolean isSubQuery = recurseCount != 0; + if (!isFilter() && !isSubQuery) { String[] filterQueries = req.getParams().getParams(CommonParams.FQ); if (filterQueries != null && filterQueries.length != 0) { - List filters; + List preFilters; try { - filters = QueryUtils.parseFilterQueries(req, true); + preFilters = acceptPreFiltersOnly(QueryUtils.parseFilterQueries(req, true)); } catch (SyntaxError e) { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e); } - - if (filters.size() == 1) { - return filters.get(0); - } - - BooleanQuery.Builder builder = new BooleanQuery.Builder(); - for (Query query : filters) { - builder.add(query, BooleanClause.Occur.FILTER); + + if (preFilters.size() == 0) { + return null; + } else if (preFilters.size() == 1) { + return preFilters.get(0); + } else { + BooleanQuery.Builder builder = new BooleanQuery.Builder(); + for (Query query : preFilters) { + builder.add(query, BooleanClause.Occur.FILTER); + } + return builder.build(); } - - return builder.build(); } } - return null; } + private List acceptPreFiltersOnly(List filters) { + return filters.stream().filter(q -> { + if (q instanceof ExtendedQuery) { + ExtendedQuery eq = (ExtendedQuery) q; + return eq.getCost() == 0; + } else { + return true; + } + }).collect(Collectors.toList()); + } + /** * Parses a String vector. * diff --git a/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java b/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java index 48e8f3b9aa5..e1127d67353 100644 --- a/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java +++ b/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java @@ -335,6 +335,45 @@ public void knnQueryWithFilterQuery_shouldPerformKnnSearchInPreFilteredResults() "//result/doc[4]/str[@name='id'][.='9']"); } + @Test + public void knnQueryWithCostlyFq_shouldPerformKnnSearchWithPostFilter() { + String vectorToSearch = "[1.0, 2.0, 3.0, 4.0]"; + + assertQ( + req( + CommonParams.Q, + "{!knn f=vector topK=10}" + vectorToSearch, + "fq", + "{!frange l=0.99}$q", + "fl", + "*,score"), + "//result[@numFound='5']", + "//result/doc[1]/str[@name='id'][.='1']", + "//result/doc[2]/str[@name='id'][.='4']", + "//result/doc[3]/str[@name='id'][.='2']", + "//result/doc[4]/str[@name='id'][.='10']", + "//result/doc[5]/str[@name='id'][.='3']"); + } + + @Test + public void knnQueryWithFilterQueries_shouldPerformKnnSearchWithPreFiltersAndPostFilters() { + String vectorToSearch = "[1.0, 2.0, 3.0, 4.0]"; + + assertQ( + req( + CommonParams.Q, + "{!knn f=vector topK=4}" + vectorToSearch, + "fq", + "id:(3 4 9 2)", + "fq", + "{!frange l=0.99}$q", + "fl", + "id"), + "//result[@numFound='2']", + "//result/doc[1]/str[@name='id'][.='4']", + "//result/doc[2]/str[@name='id'][.='2']"); + } + @Test public void knnQueryWithNegativeFilterQuery_shouldPerformKnnSearchInPreFilteredResults() { String vectorToSearch = "[1.0, 2.0, 3.0, 4.0]"; From 999862cb601747947b7ea1e9f284603d9bd90372 Mon Sep 17 00:00:00 2001 From: Alessandro Benedetti Date: Mon, 19 Dec 2022 19:16:23 +0100 Subject: [PATCH 02/10] formatting --- .../apache/solr/search/neural/KnnQParser.java | 27 +++++----- .../solr/search/neural/KnnQParserTest.java | 50 +++++++++---------- 2 files changed, 40 insertions(+), 37 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java b/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java index 714cf65c277..a7b7984e042 100644 --- a/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java +++ b/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java @@ -16,6 +16,8 @@ */ package org.apache.solr.search.neural; +import java.util.List; +import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; @@ -33,9 +35,6 @@ import org.apache.solr.search.QueryUtils; import org.apache.solr.search.SyntaxError; -import java.util.List; -import java.util.stream.Collectors; - public class KnnQParser extends QParser { // retrieve the top K results based on the distance similarity function @@ -98,7 +97,7 @@ private Query buildPreFilter() throws SolrException { } catch (SyntaxError e) { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e); } - + if (preFilters.size() == 0) { return null; } else if (preFilters.size() == 1) { @@ -116,14 +115,17 @@ private Query buildPreFilter() throws SolrException { } private List acceptPreFiltersOnly(List filters) { - return filters.stream().filter(q -> { - if (q instanceof ExtendedQuery) { - ExtendedQuery eq = (ExtendedQuery) q; - return eq.getCost() == 0; - } else { - return true; - } - }).collect(Collectors.toList()); + return filters.stream() + .filter( + q -> { + if (q instanceof ExtendedQuery) { + ExtendedQuery eq = (ExtendedQuery) q; + return eq.getCost() == 0; + } else { + return true; + } + }) + .collect(Collectors.toList()); } /** @@ -133,6 +135,7 @@ private List acceptPreFiltersOnly(List filters) { * @return a float array */ private static float[] parseVector(String value, int dimension) { + if (!value.startsWith("[") || !value.endsWith("]")) { throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, diff --git a/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java b/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java index e1127d67353..2debf1b5314 100644 --- a/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java +++ b/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java @@ -340,19 +340,19 @@ public void knnQueryWithCostlyFq_shouldPerformKnnSearchWithPostFilter() { String vectorToSearch = "[1.0, 2.0, 3.0, 4.0]"; assertQ( - req( - CommonParams.Q, - "{!knn f=vector topK=10}" + vectorToSearch, - "fq", - "{!frange l=0.99}$q", - "fl", - "*,score"), - "//result[@numFound='5']", - "//result/doc[1]/str[@name='id'][.='1']", - "//result/doc[2]/str[@name='id'][.='4']", - "//result/doc[3]/str[@name='id'][.='2']", - "//result/doc[4]/str[@name='id'][.='10']", - "//result/doc[5]/str[@name='id'][.='3']"); + req( + CommonParams.Q, + "{!knn f=vector topK=10}" + vectorToSearch, + "fq", + "{!frange l=0.99}$q", + "fl", + "*,score"), + "//result[@numFound='5']", + "//result/doc[1]/str[@name='id'][.='1']", + "//result/doc[2]/str[@name='id'][.='4']", + "//result/doc[3]/str[@name='id'][.='2']", + "//result/doc[4]/str[@name='id'][.='10']", + "//result/doc[5]/str[@name='id'][.='3']"); } @Test @@ -360,18 +360,18 @@ public void knnQueryWithFilterQueries_shouldPerformKnnSearchWithPreFiltersAndPos String vectorToSearch = "[1.0, 2.0, 3.0, 4.0]"; assertQ( - req( - CommonParams.Q, - "{!knn f=vector topK=4}" + vectorToSearch, - "fq", - "id:(3 4 9 2)", - "fq", - "{!frange l=0.99}$q", - "fl", - "id"), - "//result[@numFound='2']", - "//result/doc[1]/str[@name='id'][.='4']", - "//result/doc[2]/str[@name='id'][.='2']"); + req( + CommonParams.Q, + "{!knn f=vector topK=4}" + vectorToSearch, + "fq", + "id:(3 4 9 2)", + "fq", + "{!frange l=0.99}$q", + "fl", + "id"), + "//result[@numFound='2']", + "//result/doc[1]/str[@name='id'][.='4']", + "//result/doc[2]/str[@name='id'][.='2']"); } @Test From 88954c8d13d3469019c85b217516a0928571d094 Mon Sep 17 00:00:00 2001 From: Alessandro Benedetti Date: Tue, 20 Dec 2022 07:37:17 +0100 Subject: [PATCH 03/10] better post-filter check according to review suggestions --- .../apache/solr/search/neural/KnnQParser.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java b/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java index a7b7984e042..6f3dc4956a2 100644 --- a/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java +++ b/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java @@ -29,7 +29,7 @@ import org.apache.solr.schema.DenseVectorField; import org.apache.solr.schema.FieldType; import org.apache.solr.schema.SchemaField; -import org.apache.solr.search.ExtendedQuery; +import org.apache.solr.search.PostFilter; import org.apache.solr.search.QParser; import org.apache.solr.search.QueryParsing; import org.apache.solr.search.QueryUtils; @@ -114,13 +114,21 @@ private Query buildPreFilter() throws SolrException { return null; } + /** + * This methods filters out all the post-filters from the filters in input, keeping only the + * pre-filters. According to the documentation a filter query is a post-filter if: it implements + * {@link PostFilter} and ExtendedQuery.getCost() returns no less than 100 + * + * @param filters filter queries in the knn query request + * @return the list of pre-filters + */ private List acceptPreFiltersOnly(List filters) { return filters.stream() .filter( q -> { - if (q instanceof ExtendedQuery) { - ExtendedQuery eq = (ExtendedQuery) q; - return eq.getCost() == 0; + if (q instanceof PostFilter) { + PostFilter filter = (PostFilter) q; + return filter.getCost() < 100; } else { return true; } From 84d9e71d6442ca7854a5e48ee002f59e635926c2 Mon Sep 17 00:00:00 2001 From: Alessandro Benedetti Date: Wed, 21 Dec 2022 16:57:08 +0100 Subject: [PATCH 04/10] workaround to improve pre-filtering support for Knn search --- .../java/org/apache/solr/search/neural/KnnQParser.java | 8 ++++---- .../org/apache/solr/search/neural/KnnQParserTest.java | 2 +- .../modules/query-guide/pages/dense-vector-search.adoc | 7 +++++++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java b/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java index 6f3dc4956a2..c4c68d2527d 100644 --- a/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java +++ b/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java @@ -82,10 +82,10 @@ public Query parse() { float[] parsedVectorToSearch = parseVector(vectorToSearch, denseVectorType.getDimension()); return denseVectorType.getKnnVectorQuery( - schemaField.getName(), parsedVectorToSearch, topK, buildPreFilter()); + schemaField.getName(), parsedVectorToSearch, topK, getFilterQuery()); } - private Query buildPreFilter() throws SolrException { + private Query getFilterQuery() throws SolrException { boolean isSubQuery = recurseCount != 0; if (!isFilter() && !isSubQuery) { String[] filterQueries = req.getParams().getParams(CommonParams.FQ); @@ -93,7 +93,7 @@ private Query buildPreFilter() throws SolrException { List preFilters; try { - preFilters = acceptPreFiltersOnly(QueryUtils.parseFilterQueries(req, true)); + preFilters = excludePostFilters(QueryUtils.parseFilterQueries(req, true)); } catch (SyntaxError e) { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e); } @@ -122,7 +122,7 @@ private Query buildPreFilter() throws SolrException { * @param filters filter queries in the knn query request * @return the list of pre-filters */ - private List acceptPreFiltersOnly(List filters) { + private List excludePostFilters(List filters) { return filters.stream() .filter( q -> { diff --git a/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java b/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java index 2debf1b5314..65098a92f70 100644 --- a/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java +++ b/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java @@ -366,7 +366,7 @@ public void knnQueryWithFilterQueries_shouldPerformKnnSearchWithPreFiltersAndPos "fq", "id:(3 4 9 2)", "fq", - "{!frange l=0.99}$q", + "{!frange cache=false l=0.99}$q", "fl", "id"), "//result[@numFound='2']", diff --git a/solr/solr-ref-guide/modules/query-guide/pages/dense-vector-search.adoc b/solr/solr-ref-guide/modules/query-guide/pages/dense-vector-search.adoc index 1484ab84ed7..b9e6106ffec 100644 --- a/solr/solr-ref-guide/modules/query-guide/pages/dense-vector-search.adoc +++ b/solr/solr-ref-guide/modules/query-guide/pages/dense-vector-search.adoc @@ -287,6 +287,13 @@ In &q={!knn f=vector topK=10}[1.0, 2.0, 3.0, 4.0]&fq=id:(1 2 3) The results are prefiltered by the fq=id:(1 2 3) and then only the documents from this subset are considered as candidates for the topK knn retrieval. + +If you want to run some of the filter queries as post-filters you can follow the standard approach for post-filtering in Apache Solr, using the cache and cost local parameters. + +e.g. + +[source,text] +&q={!knn f=vector topK=10}[1.0, 2.0, 3.0, 4.0]&fq={!frange cache=false l=0.99}$q ==== From 7129a86456e29a892c0cb1e4b1ac5dbf08bd532e Mon Sep 17 00:00:00 2001 From: Alessandro Benedetti Date: Thu, 22 Dec 2022 10:31:31 +0100 Subject: [PATCH 05/10] re-using getProcessedFilter --- .../apache/solr/search/neural/KnnQParser.java | 49 +++---------------- .../solr/search/neural/KnnQParserTest.java | 2 +- 2 files changed, 8 insertions(+), 43 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java b/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java index c4c68d2527d..3ed04bcbedc 100644 --- a/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java +++ b/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java @@ -16,11 +16,9 @@ */ package org.apache.solr.search.neural; +import java.io.IOException; import java.util.List; -import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; -import org.apache.lucene.search.BooleanClause; -import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.Query; import org.apache.solr.common.SolrException; import org.apache.solr.common.params.CommonParams; @@ -29,10 +27,10 @@ import org.apache.solr.schema.DenseVectorField; import org.apache.solr.schema.FieldType; import org.apache.solr.schema.SchemaField; -import org.apache.solr.search.PostFilter; import org.apache.solr.search.QParser; import org.apache.solr.search.QueryParsing; import org.apache.solr.search.QueryUtils; +import org.apache.solr.search.SolrIndexSearcher; import org.apache.solr.search.SyntaxError; public class KnnQParser extends QParser { @@ -90,52 +88,19 @@ private Query getFilterQuery() throws SolrException { if (!isFilter() && !isSubQuery) { String[] filterQueries = req.getParams().getParams(CommonParams.FQ); if (filterQueries != null && filterQueries.length != 0) { - List preFilters; - try { - preFilters = excludePostFilters(QueryUtils.parseFilterQueries(req, true)); - } catch (SyntaxError e) { + List filters = QueryUtils.parseFilterQueries(req, true); + SolrIndexSearcher.ProcessedFilter processedFilter = + req.getSearcher().getProcessedFilter(filters); + return processedFilter.filter; + } catch (SyntaxError | IOException e) { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e); } - - if (preFilters.size() == 0) { - return null; - } else if (preFilters.size() == 1) { - return preFilters.get(0); - } else { - BooleanQuery.Builder builder = new BooleanQuery.Builder(); - for (Query query : preFilters) { - builder.add(query, BooleanClause.Occur.FILTER); - } - return builder.build(); - } } } return null; } - /** - * This methods filters out all the post-filters from the filters in input, keeping only the - * pre-filters. According to the documentation a filter query is a post-filter if: it implements - * {@link PostFilter} and ExtendedQuery.getCost() returns no less than 100 - * - * @param filters filter queries in the knn query request - * @return the list of pre-filters - */ - private List excludePostFilters(List filters) { - return filters.stream() - .filter( - q -> { - if (q instanceof PostFilter) { - PostFilter filter = (PostFilter) q; - return filter.getCost() < 100; - } else { - return true; - } - }) - .collect(Collectors.toList()); - } - /** * Parses a String vector. * diff --git a/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java b/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java index 65098a92f70..8b94f4f5452 100644 --- a/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java +++ b/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java @@ -344,7 +344,7 @@ public void knnQueryWithCostlyFq_shouldPerformKnnSearchWithPostFilter() { CommonParams.Q, "{!knn f=vector topK=10}" + vectorToSearch, "fq", - "{!frange l=0.99}$q", + "{!frange cache=false l=0.99}$q", "fl", "*,score"), "//result[@numFound='5']", From d1e7c6a8596c974825123674555d8dd24b4a0968 Mon Sep 17 00:00:00 2001 From: Alessandro Benedetti Date: Thu, 22 Dec 2022 16:41:28 +0100 Subject: [PATCH 06/10] SyntaxError propagation --- .../src/java/org/apache/solr/search/neural/KnnQParser.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java b/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java index 3ed04bcbedc..410f341993d 100644 --- a/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java +++ b/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java @@ -53,7 +53,7 @@ public KnnQParser(String qstr, SolrParams localParams, SolrParams params, SolrQu } @Override - public Query parse() { + public Query parse() throws SyntaxError { String denseVectorField = localParams.get(QueryParsing.F); String vectorToSearch = localParams.get(QueryParsing.V); int topK = localParams.getInt(TOP_K, DEFAULT_TOP_K); @@ -83,7 +83,7 @@ public Query parse() { schemaField.getName(), parsedVectorToSearch, topK, getFilterQuery()); } - private Query getFilterQuery() throws SolrException { + private Query getFilterQuery() throws SolrException, SyntaxError { boolean isSubQuery = recurseCount != 0; if (!isFilter() && !isSubQuery) { String[] filterQueries = req.getParams().getParams(CommonParams.FQ); @@ -93,7 +93,7 @@ private Query getFilterQuery() throws SolrException { SolrIndexSearcher.ProcessedFilter processedFilter = req.getSearcher().getProcessedFilter(filters); return processedFilter.filter; - } catch (SyntaxError | IOException e) { + } catch (IOException e) { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e); } } From aa1b586d74c12a824cba3c966b5e4995a5c609ee Mon Sep 17 00:00:00 2001 From: Alessandro Benedetti Date: Fri, 23 Dec 2022 10:05:07 +0100 Subject: [PATCH 07/10] Knn in filters --- .../solr/search/neural/KnnQParserTest.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java b/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java index 8b94f4f5452..ed0b092cfb9 100644 --- a/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java +++ b/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java @@ -288,6 +288,7 @@ public void correctQuery_shouldRankBySimilarityFunction() { "//result/doc[10]/str[@name='id'][.='8']"); } + @Test public void knnQueryUsedInFilter_shouldFilterResultsBeforeTheQueryExecution() { String vectorToSearch = "[1.0, 2.0, 3.0, 4.0]"; assertQ( @@ -303,6 +304,23 @@ public void knnQueryUsedInFilter_shouldFilterResultsBeforeTheQueryExecution() { "//result/doc[2]/str[@name='id'][.='4']"); } + @Test + public void knnQueryUsedInFilters_shouldFilterResultsBeforeTheQueryExecution() { + String vectorToSearch = "[1.0, 2.0, 3.0, 4.0]"; + assertQ( + req( + CommonParams.Q, + "id:(3 4 9 2)", + "fq", + "{!knn f=vector topK=4}" + vectorToSearch, + "fq", + "id:(4 20)", + "fl", + "id"), + "//result[@numFound='1']", + "//result/doc[1]/str[@name='id'][.='4']"); + } + @Test public void knnQueryWithFilterQuery_shouldPerformKnnSearchInPreFilteredResults() { String vectorToSearch = "[1.0, 2.0, 3.0, 4.0]"; From 35dfd54e2db0b274fe6697c6aee890a4399de6d2 Mon Sep 17 00:00:00 2001 From: Alessandro Benedetti Date: Thu, 29 Dec 2022 16:29:54 +0100 Subject: [PATCH 08/10] method boolean param removal --- .../solr/handler/MoreLikeThisHandler.java | 2 +- .../handler/component/QueryComponent.java | 2 +- .../component/RealTimeGetComponent.java | 4 +++- .../org/apache/solr/search/QueryUtils.java | 8 +------ .../apache/solr/search/neural/KnnQParser.java | 2 +- .../solr/search/neural/KnnQParserTest.java | 22 +++++++++---------- .../apache/solr/handler/AnalyticsHandler.java | 2 +- 7 files changed, 19 insertions(+), 23 deletions(-) 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 84c30320425..08277ffb8a2 100644 --- a/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java @@ -119,7 +119,7 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw sortSpec = parser.getSortSpec(true); } - filters = QueryUtils.parseFilterQueries(req, false); + filters = QueryUtils.parseFilterQueries(req); } catch (SyntaxError e) { throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, e); } 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 d04a1c3c279..eba85c48a4c 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 @@ -204,7 +204,7 @@ public void prepare(ResponseBuilder rb) throws IOException { List filters = rb.getFilters(); // if filters already exists, make a copy instead of modifying the original filters = filters == null ? new ArrayList<>(fqs.length) : new ArrayList<>(filters); - filters.addAll(QueryUtils.parseFilterQueries(req, false)); + filters.addAll(QueryUtils.parseFilterQueries(req)); // only set the filters if they are not empty otherwise // fq=&someotherParam= will trigger all docs filter for every request diff --git a/solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java b/solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java index 7b93db06a22..01fdf736193 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java @@ -19,6 +19,7 @@ import static org.apache.solr.common.params.CommonParams.DISTRIB; import static org.apache.solr.common.params.CommonParams.ID; import static org.apache.solr.common.params.CommonParams.VERSION_FIELD; +import static org.apache.solr.search.QueryUtils.makeQueryable; import com.google.common.collect.Lists; import java.io.IOException; @@ -205,7 +206,7 @@ public void process(ResponseBuilder rb) throws IOException { List filters = rb.getFilters(); // if filters already exists, make a copy instead of modifying the original filters = filters == null ? new ArrayList<>(fqs.length) : new ArrayList<>(filters); - filters.addAll(QueryUtils.parseFilterQueries(req, true)); + filters.addAll(QueryUtils.parseFilterQueries(req)); if (!filters.isEmpty()) { rb.setFilters(filters); } @@ -326,6 +327,7 @@ public void process(ResponseBuilder rb) throws IOException { if (rb.getFilters() != null) { for (Query raw : rb.getFilters()) { + raw = makeQueryable(raw); Query q = raw.rewrite(searcherInfo.getSearcher().getIndexReader()); Scorer scorer = searcherInfo 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 d7a16b8446f..fa49ef91bc9 100644 --- a/solr/core/src/java/org/apache/solr/search/QueryUtils.java +++ b/solr/core/src/java/org/apache/solr/search/QueryUtils.java @@ -229,14 +229,11 @@ public static Query combineQueryAndFilter(Query scoreQuery, Query filterQuery) { * Parse the filter queries in Solr request * * @param req Solr request - * @param fixNegativeQueries if true, negative queries are rewritten by adding a MatchAllDocs - * query clause * @return and array of Query. If the request does not contain filter queries, returns an empty * list. * @throws SyntaxError if an error occurs during parsing */ - public static List parseFilterQueries(SolrQueryRequest req, boolean fixNegativeQueries) - throws SyntaxError { + public static List parseFilterQueries(SolrQueryRequest req) throws SyntaxError { String[] filterQueriesStr = req.getParams().getParams(CommonParams.FQ); @@ -247,9 +244,6 @@ public static List parseFilterQueries(SolrQueryRequest req, boolean fixNe QParser fqp = QParser.getParser(fq, req); fqp.setIsFilter(true); Query query = fqp.getQuery(); - if (fixNegativeQueries) { - query = makeQueryable(query); - } filters.add(query); } } diff --git a/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java b/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java index 410f341993d..5eb6fc1d0f4 100644 --- a/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java +++ b/solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java @@ -89,7 +89,7 @@ private Query getFilterQuery() throws SolrException, SyntaxError { String[] filterQueries = req.getParams().getParams(CommonParams.FQ); if (filterQueries != null && filterQueries.length != 0) { try { - List filters = QueryUtils.parseFilterQueries(req, true); + List filters = QueryUtils.parseFilterQueries(req); SolrIndexSearcher.ProcessedFilter processedFilter = req.getSearcher().getProcessedFilter(filters); return processedFilter.filter; diff --git a/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java b/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java index ed0b092cfb9..c209e8e4165 100644 --- a/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java +++ b/solr/core/src/test/org/apache/solr/search/neural/KnnQParserTest.java @@ -308,17 +308,17 @@ public void knnQueryUsedInFilter_shouldFilterResultsBeforeTheQueryExecution() { public void knnQueryUsedInFilters_shouldFilterResultsBeforeTheQueryExecution() { String vectorToSearch = "[1.0, 2.0, 3.0, 4.0]"; assertQ( - req( - CommonParams.Q, - "id:(3 4 9 2)", - "fq", - "{!knn f=vector topK=4}" + vectorToSearch, - "fq", - "id:(4 20)", - "fl", - "id"), - "//result[@numFound='1']", - "//result/doc[1]/str[@name='id'][.='4']"); + req( + CommonParams.Q, + "id:(3 4 9 2)", + "fq", + "{!knn f=vector topK=4}" + vectorToSearch, + "fq", + "id:(4 20)", + "fl", + "id"), + "//result[@numFound='1']", + "//result/doc[1]/str[@name='id'][.='4']"); } @Test diff --git a/solr/modules/analytics/src/java/org/apache/solr/handler/AnalyticsHandler.java b/solr/modules/analytics/src/java/org/apache/solr/handler/AnalyticsHandler.java index 2e17e373d3b..eb5fcf4a93c 100644 --- a/solr/modules/analytics/src/java/org/apache/solr/handler/AnalyticsHandler.java +++ b/solr/modules/analytics/src/java/org/apache/solr/handler/AnalyticsHandler.java @@ -131,7 +131,7 @@ private DocSet getDocuments(SolrQueryRequest req) throws SyntaxError, IOExceptio queries.add(query); // Filter Params - queries.addAll(QueryUtils.parseFilterQueries(req, false)); + queries.addAll(QueryUtils.parseFilterQueries(req)); return req.getSearcher().getDocSet(queries); } From 37a2b57dc3aadcebf9b1213c95a2feba66e578bd Mon Sep 17 00:00:00 2001 From: Alessandro Benedetti Date: Wed, 4 Jan 2023 19:27:27 +0100 Subject: [PATCH 09/10] changes added --- solr/CHANGES.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index fd104124821..b2141bb6a85 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -199,6 +199,8 @@ Bug Fixes * SOLR-16585: Fixed NPE when paginating MatchAllDocs with non-zero start offset, like q=*:*&start=10 (Michael Gibney) +* SOLR-16557: Fixed problem with filtering and KNN search, especially when using post-filters (Alessandro Benedetti) + ================== 9.1.0 ================== New Features From e92c4a0b90817cee5cf97184328b35084ba5fc86 Mon Sep 17 00:00:00 2001 From: Alessandro Benedetti Date: Wed, 4 Jan 2023 22:05:57 +0100 Subject: [PATCH 10/10] changes added --- solr/CHANGES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index b2141bb6a85..bc46adea467 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -199,7 +199,7 @@ Bug Fixes * SOLR-16585: Fixed NPE when paginating MatchAllDocs with non-zero start offset, like q=*:*&start=10 (Michael Gibney) -* SOLR-16557: Fixed problem with filtering and KNN search, especially when using post-filters (Alessandro Benedetti) +* SOLR-16567: Fixed problem with filtering and KNN search, especially when using post-filters (Alessandro Benedetti) ================== 9.1.0 ==================