gsmiller commented on a change in pull request #747:
URL: https://github.com/apache/lucene/pull/747#discussion_r831491304
##########
File path:
lucene/facet/src/java/org/apache/lucene/facet/sortedset/SortedSetDocValuesFacetCounts.java
##########
@@ -190,20 +254,33 @@ private FacetResult getPathResult(
String[] parts = FacetsConfig.stringToPath(term.utf8ToString());
labelValues[i] = new LabelAndValue(parts[parts.length - 1],
ordAndValue.value);
}
+ return labelValues;
+ }
- if (dimConfig.hierarchical == false) {
- // see if dimCount is actually reliable or needs to be reset
- if (dimConfig.multiValued) {
- if (dimConfig.requireDimCount) {
- dimCount = counts[pathOrd];
- } else {
- dimCount = -1; // dimCount is in accurate at this point, so set it
to -1
- }
- }
- return new FacetResult(dim, emptyPath, dimCount, labelValues,
childCount);
- } else {
- return new FacetResult(dim, path, counts[pathOrd], labelValues,
childCount);
+ /** Returns value/count of a dimension. */
+ private int getDimValue(
+ FacetsConfig.DimConfig dimConfig,
+ String dim,
+ int dimOrd,
+ PrimitiveIterator.OfInt childOrds,
+ int topN,
+ HashMap<String, ChildOrdsResult> dimToChildOrdsResult) {
+
+ // if dimConfig.hierarchical == true || dim is multiValued and dim count
has been aggregated at
+ // indexing time, return dimCount directly
+ if (dimConfig.hierarchical == true || (dimConfig.multiValued &&
dimConfig.requireDimCount)) {
+ return counts[dimOrd];
}
+
+ // if dimCount was not aggregated at indexing time, iterate over childOrds
to get dimCount
+ ChildOrdsResult childOrdsResult = getChildOrdsResult(childOrds, topN,
dimConfig, dimOrd);
+
+ // if no early termination, store dim and childOrdsResult into hashmap
+ if (dimToChildOrdsResult != null) {
Review comment:
minor: Can `dimToChildOrdsResult` ever be `null` here?
##########
File path:
lucene/facet/src/java/org/apache/lucene/facet/sortedset/SortedSetDocValuesFacetCounts.java
##########
@@ -178,10 +229,23 @@ private FacetResult getPathResult(
}
}
- if (q == null) {
- return null;
+ if (dimConfig.hierarchical == false) {
Review comment:
The dimCount might need to be reset if hierarchical too right? What if
the dim is configured as multi-valued and hierarchical? You kind of have code
for this already in `getDimValue`. Maybe this is something like:
```
if (dimConfig.hierarchical || (dimConfig.multiValued &&
dimConfig.requireDimCount)) {
dimCount = counts[pathOrd];
} else if (dimConfig.multiValued) {
dimCount = -1;
}
```
--
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
