dsmiley commented on code in PR #1245:
URL: https://github.com/apache/solr/pull/1245#discussion_r1053656524
##########
solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java:
##########
@@ -80,44 +82,68 @@ 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 {
Review Comment:
IMO its name was just fine. "pre-filter" is not something I've ever heard
someone say in Solr.
##########
solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java:
##########
@@ -80,44 +82,60 @@ 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<Query> filters;
+ List<Query> 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);
+ 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();
}
-
- BooleanQuery.Builder builder = new BooleanQuery.Builder();
- for (Query query : filters) {
- builder.add(query, BooleanClause.Occur.FILTER);
- }
-
- return builder.build();
}
}
-
return null;
}
+ private List<Query> acceptPreFiltersOnly(List<Query> filters) {
Review Comment:
Can we name this method `excludePostFilters`? I've never heard of
"pre-filter" before. Any part of a query that isn't a PostFilter is not
necessarily executed in a defined order (is not "pre"); it's a combined query
tree with interesting leap-frogging behavior based on various costs.
The logic appears correct but I'd like to recommend a simplification to make
it even easier to read. I suggest adding a static utility method on PostFilter
called `boolean runAsPostFilter(Query q)`. You could then simply make this a
method reference. Perhaps use this elsewhere (e.g. in
SolrIndexSearcher.getProcessedFilter).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
