[ 
https://issues.apache.org/jira/browse/SOLR-10155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15880988#comment-15880988
 ] 

Christine Poerschke commented on SOLR-10155:
--------------------------------------------

bq. ... whether there's a use case for passing blanks through ... supplying a 
blank is the means of "turning it off" without blowing up ...

That's a fair point, yes, the change in behaviour would have to be documented 
clearly in the CHANGES.txt e.g. something along the lines of _"facet.contains= 
is now rejected for numeric types"_.

So then, yes, would it make sense to apply the same change to facet.prefix with 
a joint _"facet.contains= and facet.prefix= are now rejected for numeric 
types"_ CHANGES.txt note?

[~jpountz] - would you have any thoughts on this, following on from the (long 
time ago) SOLR-3855 commit Gus mentioned above?

> Clarify logic for term filters on numeric types
> -----------------------------------------------
>
>                 Key: SOLR-10155
>                 URL: https://issues.apache.org/jira/browse/SOLR-10155
>             Project: Solr
>          Issue Type: Improvement
>      Security Level: Public(Default Security Level. Issues are Public) 
>          Components: faceting
>    Affects Versions: 6.4.1
>            Reporter: Gus Heck
>            Priority: Minor
>         Attachments: SOLR-10155.patch
>
>
> The following code has been found to be confusing to multiple folks working 
> in SimpleFacets.java (see SOLR-10132)
> {code}
> if (termFilter != null) {
>               // TODO: understand this logic... what is the case for 
> supporting an empty string
>               // for contains on numeric facets? What does that achieve?
>               // The exception message is misleading in the case of an 
> excludeTerms filter in any case...
>               // Also maybe vulnerable to NPE on isEmpty test?
>               final boolean supportedOperation = (termFilter instanceof 
> SubstringBytesRefFilter) && ((SubstringBytesRefFilter) 
> termFilter).substring().isEmpty();
>               if (!supportedOperation) {
>                 throw new SolrException(ErrorCode.BAD_REQUEST, 
> FacetParams.FACET_CONTAINS + " is not supported on numeric types");
>               }
>             }
> {code}
> This is found around line 482 or so. The comment in the code above is mine, 
> and won't be found in the codebase. This ticket can be resolved by 
> eliminating the complex check and just denying all termFilters with a better 
> exception message not specific to contains filters (and perhaps consolidated 
> with the proceeding check for about prefix filters?), or adding a comment to 
> the code base explaining why we need to allow a term filter with an empty, 
> non-null string to be processed, and why this isn't an NPE waiting to happen.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to