[
https://issues.apache.org/jira/browse/SOLR-13970?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16993194#comment-16993194
]
Munendra S N edited comment on SOLR-13970 at 12/11/19 5:08 AM:
---------------------------------------------------------------
Initially added the check in QueryComponent to handle both expand and collapse
params but +1 to delegating the check to specific components.
{code:java}
request.getParams().get("group")
{code}
instead of get, using {{getBool}} would be cleaner, avoids unnecessary checks.
The latest patch covers only collapse filter. I think we should also cover the
case where expand is specified w/o collapse(as it can cause NPE with grouping).
Since, issue is related to grouping behavior with collapse/expand, I think we
should handle both cases.
The test added in SOLR-5230 was to check delegatingCollector's finish call with
grouping. Since, we are throwing error for collapse, We should use some other
component which uses delegatingCollector for testing. Maybe significantTerms or
some mock component. This is to ensure the {{finish}} call case is covered in
tests.
I think we should also update the documentation for the same.
Some of the above things are covered in previous patch. If needed, we can
incorporate them into latest patch.
[~jbernste] If you are okay with above things, I will attach new patch
incorporating those changes
was (Author: munendrasn):
Initially added the check in QueryComponent to handle both expand and collapse
params but +1 to delegating the check to specific components.
{code:java}
request.getParams().get("group")
{code}
instead of get, using {{getBool}} would be cleaner, avoids unnecessary checks.
The latest patch covers only collapse filter. I think we should also cover the
case where expand is specified w/o collapse(as it can cause NPE with grouping).
Since, issue is related to grouping behavior with collapse/expand, I think we
should handle both cases.
The test added in SOLR-5230 was to check delegatingCollector's finish call with
grouping. Since, we are throwing error for collapse, We should use some other
component which uses delegatingCollector for testing. Maybe significantTerms or
some mock component. This is to ensure the {{finish}} call case is covered in
tests.
I think we should also update the documentation for the same.
Some of the above things are covered in previous patch. If needed, we can
incorporate them into latest patch.
[~jbernste] If you are okay with above things, I will attach new patch with
above things
> Collapse/Expand and Grouping are not designed to work together, we should
> fail requests that specify both gracefully
> --------------------------------------------------------------------------------------------------------------------
>
> Key: SOLR-13970
> URL: https://issues.apache.org/jira/browse/SOLR-13970
> Project: Solr
> Issue Type: Improvement
> Security Level: Public(Default Security Level. Issues are Public)
> Reporter: Erick Erickson
> Assignee: Munendra S N
> Priority: Major
> Attachments: SOLR-13970.patch, SOLR-13970.patch, SOLR-13970.patch
>
>
> Expand/Collapse was conceived as orthogonal to grouping, and there are odd
> interactions when both are specified. If these two options are specified, we
> should reject the query with an informative message.
> Shorter term, making this explicit in the documents would be helpful.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
