gsmiller commented on a change in pull request #718:
URL: https://github.com/apache/lucene/pull/718#discussion_r826085655
##########
File path:
lucene/facet/src/java/org/apache/lucene/facet/taxonomy/IntTaxonomyFacets.java
##########
@@ -173,17 +185,17 @@ public FacetResult getTopChildren(int topN, String dim,
String... path) throws I
if (sparseValues != null) {
for (IntIntCursor c : sparseValues) {
- int count = c.value;
+ int value = c.value;
int ord = c.key;
- if (parents[ord] == dimOrd && count > 0) {
- totValue += count;
+ if (parents[ord] == dimOrd && value > 0) {
+ aggregatedValue = aggregationFunction.aggregate(aggregatedValue,
value);
childCount++;
- if (count > bottomValue) {
+ if (value > bottomValue) {
Review comment:
That's right. There are a number of things actually preventing us from
cleanly adding something like `min`. I had it originally but as I started
looking at all the changes it would require, I backed off for the time being
(especially since I don't have a concrete use-case in mind). One interesting
challenge is that these facets implementations all assume the weights are
positive values. There are a lot of `> 0` checks floating around the various
implementations to check whether-or-not a value had any "weight" associated
with it. This makes sense when using counts, but it's weird when generally
associated weights with the values. So `min` started to feel a little weird and
I just left it out for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
