[jira] [Commented] (SOLR-10155) Clarify logic for term filters on numeric types
[
https://issues.apache.org/jira/browse/SOLR-10155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15894363#comment-15894363
]
ASF subversion and git services commented on SOLR-10155:
Commit 54731019085cef2fb9499d4c872bd7aa29456ff3 in lucene-solr's branch
refs/heads/branch_6x from [~cpoerschke]
[ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=5473101 ]
SOLR-10155: For numeric types facet.contains= and facet.prefix= are now
rejected.
(Gus Heck, Christine Poerschke)
> 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
>Assignee: Christine Poerschke
>Priority: Minor
> Attachments: SOLR-10155.patch, 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
[jira] [Commented] (SOLR-10155) Clarify logic for term filters on numeric types
[
https://issues.apache.org/jira/browse/SOLR-10155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15894323#comment-15894323
]
ASF subversion and git services commented on SOLR-10155:
Commit 43474312eb2b66df4102bd653b9546e7eff47363 in lucene-solr's branch
refs/heads/master from [~cpoerschke]
[ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4347431 ]
SOLR-10155: For numeric types facet.contains= and facet.prefix= are now
rejected.
(Gus Heck, Christine Poerschke)
> 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
>Assignee: Christine Poerschke
>Priority: Minor
> Attachments: SOLR-10155.patch, 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
[jira] [Commented] (SOLR-10155) Clarify logic for term filters on numeric types
[
https://issues.apache.org/jira/browse/SOLR-10155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15880996#comment-15880996
]
Adrien Grand commented on SOLR-10155:
-
+1 to explicitly reject facet.contains and facet.prefix on numerics with a
clear error message
> 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
>Assignee: Christine Poerschke
>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
[jira] [Commented] (SOLR-10155) Clarify logic for term filters on numeric types
[
https://issues.apache.org/jira/browse/SOLR-10155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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
[jira] [Commented] (SOLR-10155) Clarify logic for term filters on numeric types
[
https://issues.apache.org/jira/browse/SOLR-10155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15877413#comment-15877413
]
Gus Heck commented on SOLR-10155:
-
I think the pattern actually started with facet.prefix at the time DocValues
was added by [~jpountz] in Solr-3855 in 2013...
https://github.com/apache/lucene-solr/commit/e61398084d3f1ca0f28c5c35d3318645d7a401ec#diff-5ac9dc7b128b4dd99b764060759222b2R381
The only question I have is whether there's a use case for passing blanks
through... perhaps some situation in which facet.prefix or facet.contains would
be robotically added and supplying a blank is the means of "turning it off"
without blowing up? Maybe some component might do such a thing?
> 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
[jira] [Commented] (SOLR-10155) Clarify logic for term filters on numeric types
[
https://issues.apache.org/jira/browse/SOLR-10155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15875687#comment-15875687
]
Alan Woodward commented on SOLR-10155:
--
+1, regex matching on numeric facets doesn't really make sense
> 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
