gsmiller commented on a change in pull request #159:
URL: https://github.com/apache/lucene/pull/159#discussion_r680552232
##########
File path: lucene/facet/src/test/org/apache/lucene/facet/TestDrillSideways.java
##########
@@ -1798,4 +1798,81 @@ public void testScorer() throws Exception {
writer.close();
IOUtils.close(searcher.getIndexReader(), taxoReader, taxoWriter, dir,
taxoDir);
}
+
+ public void testExtendedDrillSidewaysResult() throws Exception {
+ // LUCENE-9945: Extend DrillSideways to support exposing FacetCollectors
directly
+ Directory dir = newDirectory();
+ Directory taxoDir = newDirectory();
+
+ RandomIndexWriter writer = new RandomIndexWriter(random(), dir);
+
+ DirectoryTaxonomyWriter taxoWriter =
+ new DirectoryTaxonomyWriter(taxoDir,
IndexWriterConfig.OpenMode.CREATE);
+
+ FacetsConfig config = new FacetsConfig();
+ config.setHierarchical("dim", true);
+
+ Document doc = new Document();
+ doc.add(new FacetField("dim", "a", "b"));
+ writer.addDocument(config.build(taxoWriter, doc));
+
+ Document doc2 = new Document();
+ doc.add(new FacetField("dim", "x", "y"));
+ writer.addDocument(config.build(taxoWriter, doc2));
+
+ //open NRT
+ IndexSearcher searcher = getNewSearcher(writer.getReader());
+ TaxonomyReader taxoReader = new DirectoryTaxonomyReader(taxoWriter);
+
+ DrillDownQuery ddq = new DrillDownQuery(config);
+ ddq.add("dim", "x");
+
+ DrillSideways ds = getNewDrillSidewaysBuildFacetsResult(searcher, config,
taxoReader);
+
+ SimpleCollectorManager manager = new SimpleCollectorManager(10,
+ (a, b) -> Float.compare(b.docAndScore.score,
a.docAndScore.score));
+ SimpleCollector collector = manager.newCollector();
+
+ // Sometimes pass in a Collector and sometimes CollectorManager
+ // so that we can test both DrillSidewaysResult and
ConcurrentDrillSidewaysResult
+ DrillSidewaysResult r;
+ if (random().nextBoolean()) {
+ r = ds.search(ddq, collector);
+ } else {
+ r = ds.search(ddq, manager);
+ }
+
+ // compute Facets using exposed FacetCollectors from DrillSidewaysResult
+ Map<String, Facets> drillSidewaysFacets = new HashMap<>();
+ Facets drillDownFacets = getTaxonomyFacetCounts(taxoReader, config,
r.drillDownFacetsCollector);
+ if (r.drillSidewaysFacetsCollector != null) {
+ for (int i = 0; i < r.drillSidewaysFacetsCollector.length; i++) {
+ drillSidewaysFacets.put(
+ r.drillSidewaysDims[i],
+ getTaxonomyFacetCounts(taxoReader, config,
r.drillSidewaysFacetsCollector[i]));
+ }
+ }
+
+ Facets facets;
+ if (drillSidewaysFacets.isEmpty()) {
+ facets = drillDownFacets;
+ } else {
+ facets = new MultiFacets(drillSidewaysFacets, drillDownFacets);
+ }
+
+ // Facets computed using FacetsCollecter exposed in DrillSidewaysResult
+ // should match the Facets computed by {@link
DrillSideways#buildFacetsResult}
+ FacetResult facetResultExpected = facets.getTopChildren(2, "dim");
Review comment:
It seems like you've got your "expected" and "actual" instances reversed
here. It would probably make more sense to use the results from `r` as
"expected" I would think?
##########
File path: lucene/facet/src/java/org/apache/lucene/facet/DrillSideways.java
##########
@@ -390,18 +400,53 @@ protected boolean scoreSubDocsAtOnce() {
return false;
}
- /** Result of a drill sideways search, including the {@link Facets} and
{@link TopDocs}. */
+ /**
+ * Result of a drill sideways search, including the {@link Facets} and
{@link TopDocs}.
+ * The {@link FacetsCollector}s for the drill down and drill sideways
dimensions are also exposed
+ * for advanced use-cases that need access to them as an alternative to
accessing the
+ * {@code Facets}.
+ */
public static class DrillSidewaysResult {
/** Combined drill down and sideways results. */
public final Facets facets;
/** Hits. */
public final TopDocs hits;
+ /**
+ * FacetsCollector populated based on hits that match the full
DrillDownQuery,
+ * treating all drill down dimensions as required clauses. Useful for
advanced
+ * use-cases that want to compute Facets results separate from the provided
+ * Facets in this result.
+ */
+ public final FacetsCollector drillDownFacetsCollector;
+
+ /**
+ * FacetsCollectors populated for each drill sideways dimension. Each
collector exposes
+ * the hits that match on all DrillDownQuery dimensions, but treating
their corresponding
+ * sideways dimension as optional. This array provides a FacetsCollector
for each drill
+ * down dimension present in the original DrillDownQuery, and the
associated dimension
+ * for each FacetsCollector can be determined using the parallel
`drillSidewaysDims`
Review comment:
Looks like you're using markdown syntax with the backticks (`) here, but
javadoc doesn't know what that means. Try {@code drillSidewaysDims} instead.
##########
File path: lucene/facet/src/test/org/apache/lucene/facet/TestDrillSideways.java
##########
@@ -1798,4 +1798,81 @@ public void testScorer() throws Exception {
writer.close();
IOUtils.close(searcher.getIndexReader(), taxoReader, taxoWriter, dir,
taxoDir);
}
+
+ public void testExtendedDrillSidewaysResult() throws Exception {
+ // LUCENE-9945: Extend DrillSideways to support exposing FacetCollectors
directly
+ Directory dir = newDirectory();
+ Directory taxoDir = newDirectory();
+
+ RandomIndexWriter writer = new RandomIndexWriter(random(), dir);
+
+ DirectoryTaxonomyWriter taxoWriter =
+ new DirectoryTaxonomyWriter(taxoDir,
IndexWriterConfig.OpenMode.CREATE);
+
+ FacetsConfig config = new FacetsConfig();
+ config.setHierarchical("dim", true);
+
+ Document doc = new Document();
+ doc.add(new FacetField("dim", "a", "b"));
Review comment:
nit: you're creating this two-level hierarchical value but your test
only relies on the top-level values. You should be able to remove the "b" (and
the "y" below) to avoid any future confusion over what the test is testing.
##########
File path: lucene/facet/src/java/org/apache/lucene/facet/DrillSideways.java
##########
@@ -390,18 +400,53 @@ protected boolean scoreSubDocsAtOnce() {
return false;
}
- /** Result of a drill sideways search, including the {@link Facets} and
{@link TopDocs}. */
+ /**
+ * Result of a drill sideways search, including the {@link Facets} and
{@link TopDocs}.
+ * The {@link FacetsCollector}s for the drill down and drill sideways
dimensions are also exposed
+ * for advanced use-cases that need access to them as an alternative to
accessing the
+ * {@code Facets}.
+ */
public static class DrillSidewaysResult {
/** Combined drill down and sideways results. */
public final Facets facets;
/** Hits. */
public final TopDocs hits;
+ /**
+ * FacetsCollector populated based on hits that match the full
DrillDownQuery,
+ * treating all drill down dimensions as required clauses. Useful for
advanced
+ * use-cases that want to compute Facets results separate from the provided
+ * Facets in this result.
+ */
+ public final FacetsCollector drillDownFacetsCollector;
+
+ /**
+ * FacetsCollectors populated for each drill sideways dimension. Each
collector exposes
+ * the hits that match on all DrillDownQuery dimensions, but treating
their corresponding
+ * sideways dimension as optional. This array provides a FacetsCollector
for each drill
+ * down dimension present in the original DrillDownQuery, and the
associated dimension
+ * for each FacetsCollector can be determined using the parallel
`drillSidewaysDims`
+ * array. Useful for advanced use-cases that want to compute Facets
results separate
+ * from the provided Facets in this result.
+ */
+ public final FacetsCollector[] drillSidewaysFacetsCollector;
+
+ /** Dimensions that correspond to to the `drillSideways` FacetsCollectors
*/
Review comment:
Same as above: please use {@code ...} instead of backticks.
--
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
