[
https://issues.apache.org/jira/browse/LUCENE-10538?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17530905#comment-17530905
]
Greg Miller commented on LUCENE-10538:
--------------------------------------
We discussed this in the PR but I wanted to bring the conversation here for
visibility as well and to make sure this issue isn't just left hanging.
While it appears buggy that range facets don't use topN, I believe this is
intentional. Range facets are (somewhat confusingly) overloading the
{{getTopChildren}} faceting API with slightly different functionality that
returns counts for all requested ranges in the order the ranges were provided.
I think this existing functionality is important to retain, and I don't want to
lose it by truncating to topN.
I also think properly implementing {{getTopChildren}} for range faceting would
be useful for users. Meaning, a method that actually returns the top-n ranges
in decreasing count order, just like other faceting implementations.
What I'd actually suggest we do here is add a {{getAllChildren}} method to the
faceting API. Then we can migrate the existing {{getTopChildren}} functionality
implemented in range faceting to {{getAllChildren}}. Finally, we can replace
the existing {{getTopChildren}} range faceting implementation with a proper one.
{{LongValueFacetCounts}} is another faceting implementation where we've already
implemented "get all children" functionality, so I think there's value beyond
just range faceting here (i.e., we could migrate that implementation behind a
new method defined for all {{Facets}}).
> TopN is not being used in getTopChildren()
> ------------------------------------------
>
> Key: LUCENE-10538
> URL: https://issues.apache.org/jira/browse/LUCENE-10538
> Project: Lucene - Core
> Issue Type: Bug
> Reporter: Yuting Gan
> Priority: Minor
> Time Spent: 1.5h
> Remaining Estimate: 0h
>
> When looking at the overridden implementation getTopChildren(int topN, String
> dim, String... path) in RangeFacetCounts, I found that the topN parameter is
> not being used in the code, and the unit tests did not test this function
> properly. I will create a PR to fix this, and will look into other overridden
> implementations and see if they have the same issue. Please let me know if
> there is any question. Thanks!
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
