cpoerschke commented on a change in pull request #1381: SOLR-14364: LTR
SolrFeature fq improvements
URL: https://github.com/apache/lucene-solr/pull/1381#discussion_r402340272
##########
File path:
solr/core/src/java/org/apache/solr/search/grouping/CommandHandler.java
##########
@@ -228,12 +226,8 @@ private void searchWithTimeLimiter(Query query,
collector = MultiCollector.wrap(collector, hitCountCollector);
}
- if (filter.filter != null) {
- query = new BooleanQuery.Builder()
- .add(query, Occur.MUST)
- .add(filter.filter, Occur.FILTER)
- .build();
- }
+ query = QueryUtils.combineQueryAndFilter(query, filter.filter);
Review comment:
Okay, returning to this I think I now understand the
`QueryUtils.combineQueryAndFilter` scenarios better:
* There are two inputs and zero, two or one of them could be null.
* scoreQuery non-null, filterQuery null:
* (both old and new code) just use `scoreQuery`.
* scoreQuery non-null, filterQuery non-null:
* `BooleanQuery.Builder` is used to combine the two (in both old and new
code).
* scoreQuery null, filterQuery null:
* In the old code in
CommandHandler/SolrIndexSearcher/Grouping/ExpandComponent a value of null would
have been used for `scoreQuery`, searching on that isn't gonna end well, is it.
* In the new code `MatchAllDocsQuery` will be used to return everything.
* scoreQuery null, filterQuery non-null:
* In the old code `BooleanQuery.Builder` is used to combine the two,
regardless of `scoreQuery` being null and so that search is not going to end
well too.
* In the new code the `filterQuery` wrapped up into a
`ConstantScoreQuery` is used to search. In terms of search meaning this is
equivalent to the `scoreQuery` being a `MatchAllDocsQuery` i.e. with a score
query matching everything only the filter query needs to be searched with.
So in the "scoreQuery null" scenario the new code
* no longer results in a failed search,
* but instead returns something,
* and what is returned is consistent w.r.t. any filter query,
* and/or consistent within itself i.e. a null (score or filter) query is
taken to mean 'everything' i.e. `MatchAllDocsQuery`.
So yes, I agree, `MatchAllDocsQuery` is the logical choice.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
