You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by sh...@apache.org on 2013/04/01 06:54:40 UTC
svn commit: r1463086 - in /lucene/dev/branches/branch_4x: ./ lucene/
lucene/facet/ lucene/facet/src/java/org/apache/lucene/facet/search/
lucene/facet/src/test/org/apache/lucene/facet/search/
Author: shaie
Date: Mon Apr 1 04:54:39 2013
New Revision: 1463086
URL: http://svn.apache.org/r1463086
Log:
LUCENE-4893: facet counts were multiplied as many times as FacetsCollector.getFacetResults was called
Modified:
lucene/dev/branches/branch_4x/ (props changed)
lucene/dev/branches/branch_4x/lucene/ (props changed)
lucene/dev/branches/branch_4x/lucene/CHANGES.txt (contents, props changed)
lucene/dev/branches/branch_4x/lucene/facet/ (props changed)
lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/search/FacetResult.java
lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/search/FacetsAccumulator.java
lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/search/FacetsCollector.java
lucene/dev/branches/branch_4x/lucene/facet/src/test/org/apache/lucene/facet/search/TestFacetsCollector.java
Modified: lucene/dev/branches/branch_4x/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/CHANGES.txt?rev=1463086&r1=1463085&r2=1463086&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/CHANGES.txt (original)
+++ lucene/dev/branches/branch_4x/lucene/CHANGES.txt Mon Apr 1 04:54:39 2013
@@ -162,6 +162,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
Modified: lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/search/FacetResult.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/search/FacetResult.java?rev=1463086&r1=1463085&r2=1463086&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/search/FacetResult.java (original)
+++ lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/search/FacetResult.java Mon Apr 1 04:54:39 2013
@@ -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
Modified: lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/search/FacetsAccumulator.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/search/FacetsAccumulator.java?rev=1463086&r1=1463085&r2=1463086&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/search/FacetsAccumulator.java (original)
+++ lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/search/FacetsAccumulator.java Mon Apr 1 04:54:39 2013
@@ -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);
Modified: lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/search/FacetsCollector.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/search/FacetsCollector.java?rev=1463086&r1=1463085&r2=1463086&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/search/FacetsCollector.java (original)
+++ lucene/dev/branches/branch_4x/lucene/facet/src/java/org/apache/lucene/facet/search/FacetsCollector.java Mon Apr 1 04:54:39 2013
@@ -87,7 +87,7 @@ public abstract class FacetsCollector ex
}
@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 ex
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 ex
}
private final FacetsAccumulator accumulator;
+ private List<FacetResult> cachedResults;
protected final List<MatchingDocs> matchingDocs = new ArrayList<MatchingDocs>();
@@ -196,15 +197,24 @@ public abstract class FacetsCollector ex
*/
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 ex
/**
* 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);
+ }
+
}
Modified: lucene/dev/branches/branch_4x/lucene/facet/src/test/org/apache/lucene/facet/search/TestFacetsCollector.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/facet/src/test/org/apache/lucene/facet/search/TestFacetsCollector.java?rev=1463086&r1=1463085&r2=1463086&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/facet/src/test/org/apache/lucene/facet/search/TestFacetsCollector.java (original)
+++ lucene/dev/branches/branch_4x/lucene/facet/src/test/org/apache/lucene/facet/search/TestFacetsCollector.java Mon Apr 1 04:54:39 2013
@@ -230,4 +230,78 @@ public class TestFacetsCollector extends
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);
+ }
+
}