rseitz commented on code in PR #1154:
URL: https://github.com/apache/solr/pull/1154#discussion_r1014542480
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -590,52 +592,79 @@ private void setFilters(ResponseBuilder rb, Elevation
elevation) {
return;
}
- Map<?, ?> tagMap = (Map<?, ?>) rb.req.getContext().get("tags");
- if (tagMap == null) {
+ Set<Query> excludeSet = getTaggedQueries(rb, excludeTags);
+ if (excludeSet.isEmpty()) {
// no filters were tagged
return;
}
- // TODO: this code is copied from FacetProcessor#handleFilterExclusions()
- // duplication could be avoided by placing this code in a common utility
method, perhaps in
- // QueryUtils
- IdentityHashMap<Query, Boolean> excludeSet = new IdentityHashMap<>();
- for (String excludeTag : excludeTags) {
- Object olst = tagMap.get(excludeTag);
- // tagMap has entries of List<String,List<QParser>>, but subject to
change in the future
- if (!(olst instanceof Collection)) continue;
- for (Object o : (Collection<?>) olst) {
- if (!(o instanceof QParser)) continue;
- QParser qp = (QParser) o;
- try {
- excludeSet.put(qp.getQuery(), Boolean.TRUE);
- } catch (SyntaxError syntaxError) {
- // This should not happen since we should only be retrieving a
previously parsed query
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
syntaxError);
- }
- }
- }
-
List<Query> updatedFilters = new ArrayList<Query>();
- for (Query q : filters) {
- if (!excludeSet.containsKey(q) || q instanceof
CollapsingQParserPlugin.CollapsingPostFilter) {
- updatedFilters.add(q);
+ for (Query filter : filters) {
+ if (!excludeSet.contains(filter) || filter instanceof PostFilter) {
Review Comment:
I put that there because otherwise... if a collapse filter is tagged for
exclusion, as in:
`fq={!collapse tag=test1 field=str_s sort='score
desc'}&elevate.excludeTags=test1`
...we would create a BooleanQuery with the CollapsingPostFilter as one of
the clauses. Ultimately, createWeight() would be called on the BooleanQuery,
and then its components, leading to an UnsupportedOperationException from the
CollapsingPostFilter.
The original version of this PR did not allow filters to be excluded
selectively, so I thought it would be best to ignore the CollapsingPostFilter
so a user could invoke the "exclude all filters" functionality and still
collapse. Now that the PR supports selective exclusion, I agree, we should give
the user an error if they specifically tag the collapsing filter for exclusion.
Simplest would be to consider any PostFilter tagged for exclusion as an error
-- what do you think of that approach? Other PostFilters to consider are
AnalyticsQuery and FunctionRangeQuery...
--
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]
