[jira] [Updated] (LUCENE-6900) Grouping sortWithinGroup should use Sort.RELEVANCE to indicate that, not null

["David Smiley (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/LUCENE-6900?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

David Smiley updated LUCENE-6900:
---------------------------------
    Attachment: LUCENE_6900.patch

Here is a patch.  There are some edits I did just for code clarity.

I expanded the scope to include having the AbstractSecondPassGroupingCollector.needScores be determined based on the constructor args instead of always returning true (this is an optimization and addresses a TODO comment).  I did not do this likewise for other collectors.  Do you think this is okay or would you prefer a separate issue and increasing the scope there?

The Solr side was tricky to debug & fix.  I ended up doing a refactoring in TopGroupsResultTransformer to extract out near duplicated code.  I strengthened the test in TestDistributedGrouping so it actually tests score ordered groups, which it didn't before because the commenter (you?) thought distributed IDF was necessary when in fact just returning some field value as a score works fine.  I ran into SOLR-6612 but avoided it by adding "maxScore" to the "handle" map as a "SKIP".  I spent a little time trying to fix it but I stopped myself as it was becoming a distraction.  Some little improvements in the issue might reflect that.

> Grouping sortWithinGroup should use Sort.RELEVANCE to indicate that, not null
> -----------------------------------------------------------------------------
>
>                 Key: LUCENE-6900
>                 URL: https://issues.apache.org/jira/browse/LUCENE-6900
>             Project: Lucene - Core
>          Issue Type: Improvement
>          Components: modules/grouping
>            Reporter: David Smiley
>            Priority: Minor
>         Attachments: LUCENE_6900.patch
>
>
> In AbstractSecondPassGroupingCollector, {{withinGroupSort}} uses a value of null to indicate a relevance sort.  I think it's nicer to use Sort.RELEVANCE for this -- after all it's how the {{groupSort}} variable is handled.  This choice is also seen in GroupingSearch; likely some other collaborators too.
> [~martijn.v.groningen] is there some wisdom in the current choice that escapes me?  If not I'll post a patch.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

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