This is an automated email from the ASF dual-hosted git repository. hossman pushed a commit to branch jira/SOLR-16858 in repository https://gitbox.apache.org/repos/asf/solr.git
commit b22740f7797e067dadad406e38865c3e04ebfcbe Author: Chris Hostetter <[email protected]> AuthorDate: Mon Jan 29 15:24:34 2024 -0700 Switch localparam name: fq -> preFilter --- .../org/apache/solr/search/neural/KnnQParser.java | 29 ++++++------ .../org/apache/solr/search/QueryEqualityTest.java | 20 ++++---- .../apache/solr/search/neural/KnnQParserTest.java | 55 +++++++++++++--------- 3 files changed, 57 insertions(+), 47 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 5599fd8dbe2..252a4fcabc7 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 @@ -22,7 +22,6 @@ import java.util.Collections; import java.util.List; import org.apache.lucene.search.Query; import org.apache.solr.common.SolrException; -import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.StrUtils; import org.apache.solr.request.SolrQueryRequest; @@ -36,6 +35,7 @@ import org.apache.solr.search.SyntaxError; public class KnnQParser extends QParser { + static final String PRE_FILTER = "preFilter"; static final String EXCLUDE_TAGS = "excludeTags"; static final String INCLUDE_TAGS = "includeTags"; @@ -123,19 +123,20 @@ public class KnnQParser extends QParser { } } - // Explicit fq local params specifying the filter(s) to wrap - final String[] localFQs = getLocalParams().getParams(CommonParams.FQ); - if (null != localFQs) { + // Explicit local params specifying the filter(s) to wrap + final String[] preFilters = getLocalParams().getParams(PRE_FILTER); + if (null != preFilters) { - // We don't particularly care if localFQs is empty, the usage below will still work, + // We don't particularly care if preFilters is empty, the usage below will still work, // but SolrParams API says it should be null not empty... - assert 0 != localFQs.length : "SolrParams.getParams should return null, never zero len array"; + assert 0 != preFilters.length + : "SolrParams.getParams should return null, never zero len array"; if (haveGlobalFQTags) { throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "Knn Query Parser does not support combining " - + CommonParams.FQ + + PRE_FILTER + " localparam with either " + INCLUDE_TAGS + " or " @@ -143,25 +144,25 @@ public class KnnQParser extends QParser { + " localparams"); } - final List<Query> localParamFilters = new ArrayList<>(localFQs.length); - for (String fq : localFQs) { - final QParser parser = subQuery(fq, null); + final List<Query> preFilterQueries = new ArrayList<>(preFilters.length); + for (String f : preFilters) { + final QParser parser = subQuery(f, null); parser.setIsFilter(true); - // maybe null, ie: `fq=""` + // maybe null, ie: `preFilter=""` final Query filter = parser.getQuery(); if (null != filter) { - localParamFilters.add(filter); + preFilterQueries.add(filter); } } try { - return req.getSearcher().getProcessedFilter(localParamFilters).filter; + return req.getSearcher().getProcessedFilter(preFilterQueries).filter; } catch (IOException e) { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e); } } - // No explicit `fq` localparams specifying what we should filter on. + // No explicit `preFilter` localparams specifying what we should filter on. // // So now, if we're either a filter or a subquery, we have to default to // not wrapping anything... diff --git a/solr/core/src/test/org/apache/solr/search/QueryEqualityTest.java b/solr/core/src/test/org/apache/solr/search/QueryEqualityTest.java index a72de3efd72..034da7f92ec 100644 --- a/solr/core/src/test/org/apache/solr/search/QueryEqualityTest.java +++ b/solr/core/src/test/org/apache/solr/search/QueryEqualityTest.java @@ -1358,40 +1358,40 @@ public class QueryEqualityTest extends SolrTestCaseJ4 { "knn", req0, "{!knn f=vector}" + qvec, - "{!knn f=vector fq=''}" + qvec, + "{!knn f=vector preFilter=''}" + qvec, "{!knn f=vector v=" + qvec + "}"); try (SolrQueryRequest req1 = req("fq", "{!tag=t1}id:1", "xxx", "id:1")) { - // either global fq, or (same) fq as localparam + // either global fq, or (same) preFilter as localparam final Query fqOne = assertQueryEqualsAndReturn( "knn", req1, "{!knn f=vector}" + qvec, "{!knn f=vector includeTags=t1}" + qvec, - "{!knn f=vector fq='id:1'}" + qvec, - "{!knn f=vector fq=$xxx}" + qvec, + "{!knn f=vector preFilter='id:1'}" + qvec, + "{!knn f=vector preFilter=$xxx}" + qvec, "{!knn f=vector v=" + qvec + "}"); QueryUtils.checkUnequal(fqNull, fqOne); try (SolrQueryRequest req2 = req("fq", "{!tag=t2}id:2", "xxx", "id:1", "yyy", "")) { - // override global fq with local param to use different filter + // override global fq with local param to use different preFilter final Query fqOneOverride = assertQueryEqualsAndReturn( "knn", req2, - "{!knn f=vector fq='id:1'}" + qvec, - "{!knn f=vector fq=$xxx}" + qvec); + "{!knn f=vector preFilter='id:1'}" + qvec, + "{!knn f=vector preFilter=$xxx}" + qvec); QueryUtils.checkEqual(fqOne, fqOneOverride); - // override global fq with local param to use no filters + // override global fq with local param to use no preFilters final Query fqNullOverride = assertQueryEqualsAndReturn( "knn", req2, - "{!knn f=vector fq=''}" + qvec, + "{!knn f=vector preFilter=''}" + qvec, "{!knn f=vector excludeTags=t2}" + qvec, - "{!knn f=vector fq=$yyy}" + qvec); + "{!knn f=vector preFilter=$yyy}" + qvec); QueryUtils.checkEqual(fqNull, fqNullOverride); } } 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 197540b2c7b..12dd843b05d 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 @@ -470,13 +470,13 @@ public class KnnQParserTest extends SolrTestCaseJ4 { "//result[@numFound='1']", "//result/doc[1]/str[@name='id'][.='4']"); - // topK=4 w/localparam fq -> 1,4,7,9 + // topK=4 w/localparam preFilter -> 1,4,7,9 assertQ( req( CommonParams.Q, "id:(3 4 9 2)", "fq", - "{!knn f=vector topK=4 fq='id:(1 4 7 8 9)'}" + vectorToSearch, + "{!knn f=vector topK=4 preFilter='id:(1 4 7 8 9)'}" + vectorToSearch, "fq", "id:(4 20 9)", "fl", @@ -519,10 +519,10 @@ public class KnnQParserTest extends SolrTestCaseJ4 { "//result/doc[3]/str[@name='id'][.='3']", "//result/doc[4]/str[@name='id'][.='8']"); - // knn subquery should still accept `fq` local param + // knn subquery should still accept `preFilter` local param // filt -> topK -> 4,2,3,7,9 assertQ( - req(common, "q", "*:* AND {!knn f=vector topK=5 fq='" + filt + "' v=$vec}"), + req(common, "q", "*:* AND {!knn f=vector topK=5 preFilter='" + filt + "' v=$vec}"), "//result[@numFound='5']", "//result/doc[1]/str[@name='id'][.='4']", "//result/doc[2]/str[@name='id'][.='2']", @@ -530,7 +530,8 @@ public class KnnQParserTest extends SolrTestCaseJ4 { "//result/doc[4]/str[@name='id'][.='7']", "//result/doc[5]/str[@name='id'][.='9']"); - // knn subquery should still accept `fq` local param, and not pre-filter on any global fq params + // knn subquery should still accept `preFilter` local param, and not pre-filter on any global fq + // params // filt -> topK -> 4,2,3,7,9 -> fq -> 3,9 assertQ( req( @@ -538,7 +539,7 @@ public class KnnQParserTest extends SolrTestCaseJ4 { "fq", "id:(1 9 20 3 5 6 8)", "q", - "*:* AND {!knn f=vector topK=5 fq='" + filt + "' v=$vec}"), + "*:* AND {!knn f=vector topK=5 preFilter='" + filt + "' v=$vec}"), "//result[@numFound='2']", "//result/doc[1]/str[@name='id'][.='3']", "//result/doc[2]/str[@name='id'][.='9']"); @@ -549,7 +550,7 @@ public class KnnQParserTest extends SolrTestCaseJ4 { "fq", "id:(1 9 20 3 5 6 8)", "q", - "id:8^=100 OR {!knn f=vector topK=5 fq='" + filt + "' v=$vec}"), + "id:8^=100 OR {!knn f=vector topK=5 preFilter='" + filt + "' v=$vec}"), "//result[@numFound='3']", "//result/doc[1]/str[@name='id'][.='8']", "//result/doc[2]/str[@name='id'][.='3']", @@ -577,11 +578,14 @@ public class KnnQParserTest extends SolrTestCaseJ4 { for (SolrQueryRequest req : Arrays.asList( req(common, "q", "{!knn f=vector topK=10}" + vectorToSearch, "fq", filt), - req(common, "q", "{!knn f=vector fq=\"" + filt + "\" topK=10}" + vectorToSearch), req( common, "q", - "{!knn f=vector fq=$my_filt topK=10}" + vectorToSearch, + "{!knn f=vector preFilter=\"" + filt + "\" topK=10}" + vectorToSearch), + req( + common, + "q", + "{!knn f=vector preFilter=$my_filt topK=10}" + vectorToSearch, "my_filt", filt))) { assertQ( @@ -602,11 +606,16 @@ public class KnnQParserTest extends SolrTestCaseJ4 { req( common, "q", - "{!knn f=vector fq=\"" + fx + "\" fq=\"" + fy + "\" topK=4}" + vectorToSearch), + "{!knn f=vector preFilter=\"" + + fx + + "\" preFilter=\"" + + fy + + "\" topK=4}" + + vectorToSearch), req( common, "q", - "{!knn f=vector fq=$fx fq=$fy topK=4}" + vectorToSearch, + "{!knn f=vector preFilter=$fx preFilter=$fy topK=4}" + vectorToSearch, "fx", fx, "fy", @@ -614,7 +623,7 @@ public class KnnQParserTest extends SolrTestCaseJ4 { req( common, "q", - "{!knn f=vector fq=$multi_filt topK=4}" + vectorToSearch, + "{!knn f=vector preFilter=$multi_filt topK=4}" + vectorToSearch, "multi_filt", fx, "multi_filt", @@ -630,16 +639,16 @@ public class KnnQParserTest extends SolrTestCaseJ4 { } assertQEx( - "knn fq localparm incompatible with include/exclude localparams", - "does not support combining fq localparam with either", + "knn preFilter localparm incompatible with include/exclude localparams", + "does not support combining preFilter localparam with either", // shouldn't matter if global fq w/tag even exists, usage is an error - req("q", "{!knn f=vector fq='id:1' includeTags=xxx}" + vectorToSearch), + req("q", "{!knn f=vector preFilter='id:1' includeTags=xxx}" + vectorToSearch), SolrException.ErrorCode.BAD_REQUEST); assertQEx( - "knn fq localparm incompatible with include/exclude localparams", - "does not support combining fq localparam with either", + "knn preFilter localparm incompatible with include/exclude localparams", + "does not support combining preFilter localparam with either", // shouldn't matter if global fq w/tag even exists, usage is an error - req("q", "{!knn f=vector fq='id:1' excludeTags=xxx}" + vectorToSearch), + req("q", "{!knn f=vector preFilter='id:1' excludeTags=xxx}" + vectorToSearch), SolrException.ErrorCode.BAD_REQUEST); } @@ -647,10 +656,10 @@ public class KnnQParserTest extends SolrTestCaseJ4 { public void knnQueryWithFilterQuery_localParamOverridesGlobalFilters() { final String vectorToSearch = "[1.0, 2.0, 3.0, 4.0]"; - // trivial case: empty fq localparam means no pre-filtering + // trivial case: empty preFilter localparam means no pre-filtering assertQ( req( - "q", "{!knn f=vector fq='' topK=5}" + vectorToSearch, + "q", "{!knn f=vector preFilter='' topK=5}" + vectorToSearch, "fq", "-id:4", "fl", "id"), "//result[@numFound='4']", @@ -662,7 +671,7 @@ public class KnnQParserTest extends SolrTestCaseJ4 { // localparam prefiltering, global fqs applied independently assertQ( req( - "q", "{!knn f=vector fq='id:(3 4 9 2 7 8)' topK=5}" + vectorToSearch, + "q", "{!knn f=vector preFilter='id:(3 4 9 2 7 8)' topK=5}" + vectorToSearch, "fq", "-id:4", "fl", "id"), "//result[@numFound='4']", @@ -715,8 +724,8 @@ public class KnnQParserTest extends SolrTestCaseJ4 { // Only 7 matches both of the the regular fq params for (SolrQueryRequest req : Arrays.asList( - // explicit local empty fq - req(common, "q", "{!knn f=vector fq='' topK=6}" + vectorToSearch), + // explicit local empty preFilter + req(common, "q", "{!knn f=vector preFilter='' topK=6}" + vectorToSearch), // diff ways of explicitly including none of the global fq params req(common, "q", "{!knn f=vector includeTags='' topK=6}" + vectorToSearch), req(common, "q", "{!knn f=vector includeTags=bogus topK=6}" + vectorToSearch),
