mikemccand commented on code in PR #12862:
URL: https://github.com/apache/lucene/pull/12862#discussion_r1418982289
##########
lucene/CHANGES.txt:
##########
@@ -67,6 +67,8 @@ API Changes
* GITHUB#11023: Adding -level param to CheckIndex, making the old -fast param
the default behaviour. (Jakub Slowinski)
+* GITHUB#12180: Add Facets#getBulkSpecificValues method to get specific values
for multiple FacetLabel at once. (Egor Potemkin)
Review Comment:
`FacetLabel` -> `FacetLabels`?
##########
lucene/facet/src/java/org/apache/lucene/facet/LongValueFacetCounts.java:
##########
@@ -568,6 +568,12 @@ public Number getSpecificValue(String dim, String... path)
{
throw new UnsupportedOperationException();
}
+ @Override
+ public Number[] getBulkSpecificValues(FacetLabel[] facetLabels) {
+ // TODO: should we impl this?
Review Comment:
Couldn't we emulate it by calling `getSpecificValue` N times and collating?
##########
lucene/facet/src/java/org/apache/lucene/facet/Facets.java:
##########
@@ -44,11 +50,17 @@ public abstract FacetResult getTopChildren(int topN, String
dim, String... path)
throws IOException;
/**
- * Return the count or value for a specific path. Returns -1 if this path
doesn't exist, else the
- * count.
+ * Return the count or value for a specific path. Returns -1 {@link
#MISSING_SPECIFIC_VALUE} if
Review Comment:
Hmm remove the `-1`? We want users to use the constant not -1 right?
##########
lucene/facet/src/java/org/apache/lucene/facet/MultiFacets.java:
##########
@@ -77,6 +80,39 @@ public Number getSpecificValue(String dim, String... path)
throws IOException {
return facets.getSpecificValue(dim, path);
}
+ @Override
+ public Number[] getBulkSpecificValues(FacetLabel[] facetLabels) throws
IOException {
+ if (facetLabels.length == 0) {
+ return new Number[0];
+ }
+ // Split facetLabels into chunks by the Facets they belog to
+ Map<Facets, IntArrayList> labelsPerFacet = new HashMap<>();
+ for (int i = 0; i < facetLabels.length; i++) {
+ String dim = facetLabels[i].components[0];
+ Facets facets = dimToFacets.get(dim);
Review Comment:
Hmm does the `Facets` class have a reasonable `hashCode/equals`?
##########
lucene/facet/src/java/org/apache/lucene/facet/MultiFacets.java:
##########
@@ -77,6 +80,39 @@ public Number getSpecificValue(String dim, String... path)
throws IOException {
return facets.getSpecificValue(dim, path);
}
+ @Override
+ public Number[] getBulkSpecificValues(FacetLabel[] facetLabels) throws
IOException {
+ if (facetLabels.length == 0) {
+ return new Number[0];
+ }
+ // Split facetLabels into chunks by the Facets they belog to
+ Map<Facets, IntArrayList> labelsPerFacet = new HashMap<>();
+ for (int i = 0; i < facetLabels.length; i++) {
+ String dim = facetLabels[i].components[0];
+ Facets facets = dimToFacets.get(dim);
+ if (facets == null) {
+ if (defaultFacets == null) {
+ throw new IllegalArgumentException("invalid dim \"" + dim + "\"");
+ }
+ facets = defaultFacets;
+ }
+ labelsPerFacet.computeIfAbsent(facets, f -> new IntArrayList()).add(i);
+ }
+ // Call getBulkSpecificValues for each chunk and reconcile results
+ Number[] result = new Number[facetLabels.length];
+ for (Map.Entry<Facets, IntArrayList> fl : labelsPerFacet.entrySet()) {
+ FacetLabel[] labelsForFacet = new FacetLabel[fl.getValue().size()];
+ for (IntCursor ordI : fl.getValue()) {
+ labelsForFacet[ordI.index] = facetLabels[ordI.value];
+ }
+ Number[] ords = fl.getKey().getBulkSpecificValues(labelsForFacet);
Review Comment:
Remind me why these APIs return `Number` not `long`? If we represent
missing as `-1` it seems like we don't need `null` that `Number` would give us?
--
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
