diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 26863cb1f06..b2b8dde7ffd 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -293,6 +293,8 @@ Bug fixes * LUCENE-9823: Prevent unsafe rewrites for SynonymQuery and CombinedFieldQuery. Before, rewriting could slightly change the scoring when weights were specified. (Naoto Minami via Julie Tibshirani) +* LUCENE-9988: Fix DrillSideways correctness bug introduced in LUCENE-9944 (Greg Miller) + Changes in Backwards Compatibility Policy * LUCENE-9904: regenerated UAX29URLEmailTokenizer and the corresponding analyzer with up-to-date top diff --git a/lucene/facet/src/java/org/apache/lucene/facet/DrillSidewaysQuery.java b/lucene/facet/src/java/org/apache/lucene/facet/DrillSidewaysQuery.java index ff58d8ea08f..799f1af660c 100644 --- a/lucene/facet/src/java/org/apache/lucene/facet/DrillSidewaysQuery.java +++ b/lucene/facet/src/java/org/apache/lucene/facet/DrillSidewaysQuery.java @@ -65,11 +65,34 @@ class DrillSidewaysQuery extends Query { FacetsCollectorManager[] drillSidewaysCollectorManagers, Query[] drillDownQueries, boolean scoreSubDocsAtOnce) { + this( + baseQuery, + drillDownCollectorManager, + drillSidewaysCollectorManagers, + new ArrayList<>(), + new ArrayList<>(), + drillDownQueries, + scoreSubDocsAtOnce); + } + + /** + * Needed for {@link #rewrite(IndexReader)}. Ensures the same "managed" lists get used since + * {@link DrillSideways} accesses references to these through the original {@code + * DrillSidewaysQuery}. + */ + private DrillSidewaysQuery( + Query baseQuery, + FacetsCollectorManager drillDownCollectorManager, + FacetsCollectorManager[] drillSidewaysCollectorManagers, + List managedDrillDownCollectors, + List managedDrillSidewaysCollectors, + Query[] drillDownQueries, + boolean scoreSubDocsAtOnce) { this.baseQuery = Objects.requireNonNull(baseQuery); this.drillDownCollectorManager = drillDownCollectorManager; this.drillSidewaysCollectorManagers = drillSidewaysCollectorManagers; - this.managedDrillDownCollectors = new ArrayList<>(); - this.managedDrillSidewaysCollectors = new ArrayList<>(); + this.managedDrillDownCollectors = managedDrillDownCollectors; + this.managedDrillSidewaysCollectors = managedDrillSidewaysCollectors; this.drillDownQueries = drillDownQueries; this.scoreSubDocsAtOnce = scoreSubDocsAtOnce; } @@ -96,6 +119,8 @@ class DrillSidewaysQuery extends Query { newQuery, drillDownCollectorManager, drillSidewaysCollectorManagers, + managedDrillDownCollectors, + managedDrillSidewaysCollectors, drillDownQueries, scoreSubDocsAtOnce); } diff --git a/lucene/facet/src/test/org/apache/lucene/facet/TestDrillSideways.java b/lucene/facet/src/test/org/apache/lucene/facet/TestDrillSideways.java index 9a4df52646e..b9e65f1f54f 100644 --- a/lucene/facet/src/test/org/apache/lucene/facet/TestDrillSideways.java +++ b/lucene/facet/src/test/org/apache/lucene/facet/TestDrillSideways.java @@ -358,26 +358,31 @@ public class TestDrillSideways extends FacetTestCase { RandomIndexWriter writer = new RandomIndexWriter(random(), dir); Document doc = new Document(); + doc.add(newStringField("content", "foo", Field.Store.NO)); doc.add(new FacetField("Author", "Bob")); doc.add(new FacetField("Publish Date", "2010", "10", "15")); writer.addDocument(config.build(taxoWriter, doc)); doc = new Document(); + doc.add(newStringField("content", "foo", Field.Store.NO)); doc.add(new FacetField("Author", "Lisa")); doc.add(new FacetField("Publish Date", "2010", "10", "20")); writer.addDocument(config.build(taxoWriter, doc)); doc = new Document(); + doc.add(newStringField("content", "foo", Field.Store.NO)); doc.add(new FacetField("Author", "Lisa")); doc.add(new FacetField("Publish Date", "2012", "1", "1")); writer.addDocument(config.build(taxoWriter, doc)); doc = new Document(); + doc.add(newStringField("content", "bar", Field.Store.NO)); doc.add(new FacetField("Author", "Susan")); doc.add(new FacetField("Publish Date", "2012", "1", "7")); writer.addDocument(config.build(taxoWriter, doc)); doc = new Document(); + doc.add(newStringField("content", "bar", Field.Store.NO)); doc.add(new FacetField("Author", "Frank")); doc.add(new FacetField("Publish Date", "1999", "5", "5")); writer.addDocument(config.build(taxoWriter, doc)); @@ -581,6 +586,27 @@ public class TestDrillSideways extends FacetTestCase { // Expect null facets since we provided a null FacetsCollectorManager assertNull(r.facets); + // Test the case where the base query rewrites itself. See LUCENE-9988: + Query baseQuery = + new TermQuery(new Term("content", "foo")) { + @Override + public Query rewrite(IndexReader reader) { + // return a new instance, forcing the DrillDownQuery to also rewrite itself, exposing + // the bug in LUCENE-9988: + return new TermQuery(getTerm()); + } + }; + ddq = new DrillDownQuery(config, baseQuery); + ddq.add("Author", "Lisa"); + r = ds.search(ddq, manager); + assertEquals(2, r.collectorResult.size()); + assertEquals( + "dim=Publish Date path=[] value=2 childCount=2\n 2010 (1)\n 2012 (1)\n", + r.facets.getTopChildren(10, "Publish Date").toString()); + assertEquals( + "dim=Author path=[] value=3 childCount=2\n Lisa (2)\n Bob (1)\n", + r.facets.getTopChildren(10, "Author").toString()); + writer.close(); IOUtils.close(searcher.getIndexReader(), taxoReader, taxoWriter, dir, taxoDir); }