LUCENE-4893: facet counts were multiplied as many times as FacetsCollector.getFacetResults was called

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1463085 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Shai Erera 2013-04-01 04:49:36 +00:00
parent 48a28acbfd
commit 0f1738e265
5 changed files with 116 additions and 16 deletions

View File

@ -189,6 +189,9 @@ Bug Fixes
IndexDeletionPolicy and InfoStream in order to make an IndexWriterConfig and
its clone fully independent. (Adrien Grand)
* LUCENE-4893: Facet counts were multiplied as many times as
FacetsCollector.getFacetResults() is called. (Shai Erera)
Documentation
* LUCENE-4841: Added example SimpleSortedSetFacetsExample to show how

View File

@ -1,6 +1,5 @@
package org.apache.lucene.facet.search;
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with

View File

@ -81,6 +81,14 @@ public class FacetsAccumulator {
return new FacetsAccumulator(fsp, indexReader, taxoReader);
}
private static FacetResult emptyResult(int ordinal, FacetRequest fr) {
FacetResultNode root = new FacetResultNode();
root.ordinal = ordinal;
root.label = fr.categoryPath;
root.value = 0;
return new FacetResult(fr, root, 0);
}
/**
* Initializes the accumulator with the given parameters as well as
* {@link FacetArrays}. Note that the accumulator doesn't call
@ -173,12 +181,8 @@ public class FacetsAccumulator {
for (FacetRequest fr : searchParams.facetRequests) {
int rootOrd = taxonomyReader.getOrdinal(fr.categoryPath);
if (rootOrd == TaxonomyReader.INVALID_ORDINAL) { // category does not exist
// Add empty FacetResult:
FacetResultNode root = new FacetResultNode();
root.ordinal = TaxonomyReader.INVALID_ORDINAL;
root.label = fr.categoryPath;
root.value = 0;
res.add(new FacetResult(fr, root, 0));
// Add empty FacetResult
res.add(emptyResult(rootOrd, fr));
continue;
}
CategoryListParams clp = searchParams.indexingParams.getCategoryListParams(fr.categoryPath);

View File

@ -87,7 +87,7 @@ public abstract class FacetsCollector extends Collector {
}
@Override
public final void setNextReader(AtomicReaderContext context) throws IOException {
protected final void doSetNextReader(AtomicReaderContext context) throws IOException {
if (bits != null) {
matchingDocs.add(new MatchingDocs(this.context, bits, totalHits, scores));
}
@ -133,7 +133,7 @@ public abstract class FacetsCollector extends Collector {
public final void setScorer(Scorer scorer) throws IOException {}
@Override
public final void setNextReader(AtomicReaderContext context) throws IOException {
protected final void doSetNextReader(AtomicReaderContext context) throws IOException {
if (bits != null) {
matchingDocs.add(new MatchingDocs(this.context, bits, totalHits, null));
}
@ -183,6 +183,7 @@ public abstract class FacetsCollector extends Collector {
}
private final FacetsAccumulator accumulator;
private List<FacetResult> cachedResults;
protected final List<MatchingDocs> matchingDocs = new ArrayList<MatchingDocs>();
@ -196,15 +197,24 @@ public abstract class FacetsCollector extends Collector {
*/
protected abstract void finish();
/** Performs the actual work of {@link #setNextReader(AtomicReaderContext)}. */
protected abstract void doSetNextReader(AtomicReaderContext context) throws IOException;
/**
* Returns a {@link FacetResult} per {@link FacetRequest} set in
* {@link FacetSearchParams}. Note that if one of the {@link FacetRequest
* requests} is for a {@link CategoryPath} that does not exist in the taxonomy,
* no matching {@link FacetResult} will be returned.
* {@link FacetSearchParams}. Note that if a {@link FacetRequest} defines a
* {@link CategoryPath} which does not exist in the taxonomy, an empty
* {@link FacetResult} will be returned for it.
*/
public final List<FacetResult> getFacetResults() throws IOException {
finish();
return accumulator.accumulate(matchingDocs);
// LUCENE-4893: if results are not cached, counts are multiplied as many
// times as this method is called.
if (cachedResults == null) {
finish();
cachedResults = accumulator.accumulate(matchingDocs);
}
return cachedResults;
}
/**
@ -218,12 +228,22 @@ public abstract class FacetsCollector extends Collector {
/**
* Allows to reuse the collector between search requests. This method simply
* clears all collected documents (and scores) information, and does not
* attempt to reuse allocated memory spaces.
* clears all collected documents (and scores) information (as well as cached
* results), and does not attempt to reuse allocated memory spaces.
*/
public final void reset() {
finish();
matchingDocs.clear();
cachedResults = null;
}
@Override
public final void setNextReader(AtomicReaderContext context) throws IOException {
// clear cachedResults - needed in case someone called getFacetResults()
// before doing a search and didn't call reset(). Defensive code to prevent
// traps.
cachedResults = null;
doSetNextReader(context);
}
}

View File

@ -230,4 +230,78 @@ public class TestFacetsCollector extends FacetTestCase {
IOUtils.close(taxo, taxoDir, r, indexDir);
}
@Test
public void testGetFacetResultsTwice() throws Exception {
// LUCENE-4893: counts were multiplied as many times as getFacetResults was called.
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);
Document doc = new Document();
facetFields.addFields(doc, Arrays.asList(new CategoryPath("a/1", '/'), new CategoryPath("b/1", '/')));
iw.addDocument(doc);
taxonomyWriter.close();
iw.close();
DirectoryReader r = DirectoryReader.open(indexDir);
DirectoryTaxonomyReader taxo = new DirectoryTaxonomyReader(taxoDir);
FacetSearchParams fsp = new FacetSearchParams(
new CountFacetRequest(new CategoryPath("a"), 10),
new CountFacetRequest(new CategoryPath("b"), 10));
final FacetsAccumulator fa = random().nextBoolean() ? new FacetsAccumulator(fsp, r, taxo) : new StandardFacetsAccumulator(fsp, r, taxo);
final FacetsCollector fc = FacetsCollector.create(fa);
new IndexSearcher(r).search(new MatchAllDocsQuery(), fc);
List<FacetResult> res1 = fc.getFacetResults();
List<FacetResult> res2 = fc.getFacetResults();
assertSame("calling getFacetResults twice should return the exact same result", res1, res2);
IOUtils.close(taxo, taxoDir, r, indexDir);
}
@Test
public void testReset() throws Exception {
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);
Document doc = new Document();
facetFields.addFields(doc, Arrays.asList(new CategoryPath("a/1", '/'), new CategoryPath("b/1", '/')));
iw.addDocument(doc);
taxonomyWriter.close();
iw.close();
DirectoryReader r = DirectoryReader.open(indexDir);
DirectoryTaxonomyReader taxo = new DirectoryTaxonomyReader(taxoDir);
FacetSearchParams fsp = new FacetSearchParams(
new CountFacetRequest(new CategoryPath("a"), 10),
new CountFacetRequest(new CategoryPath("b"), 10));
final FacetsAccumulator fa = random().nextBoolean() ? new FacetsAccumulator(fsp, r, taxo) : new StandardFacetsAccumulator(fsp, r, taxo);
final FacetsCollector fc = FacetsCollector.create(fa);
// this should populate the cached results, but doing search should clear the cache
fc.getFacetResults();
new IndexSearcher(r).search(new MatchAllDocsQuery(), fc);
List<FacetResult> res1 = fc.getFacetResults();
// verify that we didn't get the cached result
assertEquals(2, res1.size());
for (FacetResult res : res1) {
assertEquals(1, res.getFacetResultNode().subResults.size());
assertEquals(1, (int) res.getFacetResultNode().subResults.get(0).value);
}
fc.reset();
List<FacetResult> res2 = fc.getFacetResults();
assertNotSame("reset() should clear the cached results", res1, res2);
IOUtils.close(taxo, taxoDir, r, indexDir);
}
}