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

Joel Bernstein commented on SOLR-13970:
---------------------------------------

All good points. Let's go through them:
{code:java}
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.{code}
Does SOLR-7798 fix the NPE with expand? Does grouping a collapse have a 
specific conflict? If it does then let's add the exception as you suggested.
{code:java}
 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.{code}
I think the big problem now is not if finish() is being called, but whether we 
can call the same PostFilter twice without resetting its internal state. This 
is a bigger review problem which involves understanding the inner working of 
each postfilter. So my suggestion is rather then putting in another token check 
that, we address the whole problem of postfilters with grouping in a different 
ticket (SOLR-14045) I planned addressing this ticket immediately after the 8.4 
release is cut if not sooner.


{code:java}
I think we should also update the documentation for the same. {code}
+1, sorry if I missed this in the previous patch.
{code:java}
Some of the above things are covered in previous patch. If needed, we can 
incorporate them into latest patch.
Joel Bernstein If you are okay with above things, I will attach new patch 
incorporating those changes {code}
Sounds good! I'll check back later today and see how things are going.

 

 

 

. 

 

> 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

Reply via email to