diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index d2329950a4e..95a7ea9b527 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -163,6 +163,9 @@ Bug Fixes * LUCENE-4868: SumScoreFacetsAggregator used an incorrect index into the scores array. (Shai Erera) +* LUCENE-4882: FacetsAccumulator did not allow to count ROOT category (i.e. + count dimensions). (Shai Erera) + Documentation * LUCENE-4841: Added example SimpleSortedSetFacetsExample to show how diff --git a/lucene/facet/src/java/org/apache/lucene/facet/index/CountingListBuilder.java b/lucene/facet/src/java/org/apache/lucene/facet/index/CountingListBuilder.java index c7a3cf2722a..cf05a2250c9 100644 --- a/lucene/facet/src/java/org/apache/lucene/facet/index/CountingListBuilder.java +++ b/lucene/facet/src/java/org/apache/lucene/facet/index/CountingListBuilder.java @@ -152,12 +152,15 @@ public class CountingListBuilder implements CategoryListBuilder { if (op != OrdinalPolicy.NO_PARENTS) { // need to add parents too int parent = taxoWriter.getParent(ordinal); - while (parent > 0) { - ordinals.ints[ordinals.length++] = parent; - parent = taxoWriter.getParent(parent); - } - if (op == OrdinalPolicy.ALL_BUT_DIMENSION) { // discard the last added parent, which is the dimension - ordinals.length--; + if (parent > 0) { + // only do this if the category is not a dimension itself, otherwise, it was just discarded by the 'if' below + while (parent > 0) { + ordinals.ints[ordinals.length++] = parent; + parent = taxoWriter.getParent(parent); + } + if (op == OrdinalPolicy.ALL_BUT_DIMENSION) { // discard the last added parent, which is the dimension + ordinals.length--; + } } } } diff --git a/lucene/facet/src/java/org/apache/lucene/facet/search/FacetsAccumulator.java b/lucene/facet/src/java/org/apache/lucene/facet/search/FacetsAccumulator.java index f0a25ae288e..0bc39a3d009 100644 --- a/lucene/facet/src/java/org/apache/lucene/facet/search/FacetsAccumulator.java +++ b/lucene/facet/src/java/org/apache/lucene/facet/search/FacetsAccumulator.java @@ -182,10 +182,12 @@ public class FacetsAccumulator { continue; } CategoryListParams clp = searchParams.indexingParams.getCategoryListParams(fr.categoryPath); - OrdinalPolicy ordinalPolicy = clp .getOrdinalPolicy(fr.categoryPath.components[0]); - if (ordinalPolicy == OrdinalPolicy.NO_PARENTS) { - // rollup values - aggregator.rollupValues(fr, rootOrd, children, siblings, facetArrays); + if (fr.categoryPath.length > 0) { // someone might ask to aggregate the ROOT category + OrdinalPolicy ordinalPolicy = clp.getOrdinalPolicy(fr.categoryPath.components[0]); + if (ordinalPolicy == OrdinalPolicy.NO_PARENTS) { + // rollup values + aggregator.rollupValues(fr, rootOrd, children, siblings, facetArrays); + } } FacetResultsHandler frh = createFacetResultsHandler(fr); diff --git a/lucene/facet/src/test/org/apache/lucene/facet/search/TestFacetsCollector.java b/lucene/facet/src/test/org/apache/lucene/facet/search/TestFacetsCollector.java index dad729bc547..a324d56d5e1 100644 --- a/lucene/facet/src/test/org/apache/lucene/facet/search/TestFacetsCollector.java +++ b/lucene/facet/src/test/org/apache/lucene/facet/search/TestFacetsCollector.java @@ -1,6 +1,7 @@ package org.apache.lucene.facet.search; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -190,5 +191,41 @@ public class TestFacetsCollector extends FacetTestCase { IOUtils.close(taxo, taxoDir, r, indexDir); } + + @Test + public void testCountRoot() throws Exception { + // LUCENE-4882: FacetsAccumulator threw NPE if a FacetRequest was defined on CP.EMPTY + Directory indexDir = newDirectory(); + Directory taxoDir = newDirectory(); + + TaxonomyWriter taxonomyWriter = new DirectoryTaxonomyWriter(taxoDir); + IndexWriter iw = new IndexWriter(indexDir, newIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(random()))); + + FacetFields facetFields = new FacetFields(taxonomyWriter); + for(int i = atLeast(30); i > 0; --i) { + Document doc = new Document(); + facetFields.addFields(doc, Arrays.asList(new CategoryPath("a"), new CategoryPath("b"))); + iw.addDocument(doc); + } + + taxonomyWriter.close(); + iw.close(); + + DirectoryReader r = DirectoryReader.open(indexDir); + DirectoryTaxonomyReader taxo = new DirectoryTaxonomyReader(taxoDir); + + FacetSearchParams fsp = new FacetSearchParams(new CountFacetRequest(CategoryPath.EMPTY, 10)); + + final FacetsAccumulator fa = random().nextBoolean() ? new FacetsAccumulator(fsp, r, taxo) : new StandardFacetsAccumulator(fsp, r, taxo); + FacetsCollector fc = FacetsCollector.create(fa); + new IndexSearcher(r).search(new MatchAllDocsQuery(), fc); + + FacetResult res = fc.getFacetResults().get(0); + for (FacetResultNode node : res.getFacetResultNode().subResults) { + assertEquals(r.numDocs(), (int) node.value); + } + + IOUtils.close(taxo, taxoDir, r, indexDir); + } }