diff --git a/lucene/spatial/src/java/org/apache/lucene/spatial/prefix/AbstractVisitingPrefixTreeFilter.java b/lucene/spatial/src/java/org/apache/lucene/spatial/prefix/AbstractVisitingPrefixTreeFilter.java index 22f69b9f8e6..6763c998670 100644 --- a/lucene/spatial/src/java/org/apache/lucene/spatial/prefix/AbstractVisitingPrefixTreeFilter.java +++ b/lucene/spatial/src/java/org/apache/lucene/spatial/prefix/AbstractVisitingPrefixTreeFilter.java @@ -52,7 +52,7 @@ public abstract class AbstractVisitingPrefixTreeFilter extends AbstractPrefixTre public AbstractVisitingPrefixTreeFilter(Shape queryShape, String fieldName, SpatialPrefixTree grid, int detailLevel, int prefixGridScanLevel) { super(queryShape, fieldName, grid, detailLevel); - this.prefixGridScanLevel = Math.max(1, Math.min(prefixGridScanLevel, grid.getMaxLevels() - 1)); + this.prefixGridScanLevel = Math.max(0, Math.min(prefixGridScanLevel, grid.getMaxLevels() - 1)); assert detailLevel <= grid.getMaxLevels(); } diff --git a/lucene/spatial/src/java/org/apache/lucene/spatial/prefix/WithinPrefixTreeFilter.java b/lucene/spatial/src/java/org/apache/lucene/spatial/prefix/WithinPrefixTreeFilter.java index 831b9199548..e21c93e40b3 100644 --- a/lucene/spatial/src/java/org/apache/lucene/spatial/prefix/WithinPrefixTreeFilter.java +++ b/lucene/spatial/src/java/org/apache/lucene/spatial/prefix/WithinPrefixTreeFilter.java @@ -32,6 +32,7 @@ import org.apache.lucene.util.Bits; import org.apache.lucene.util.FixedBitSet; import java.io.IOException; +import java.util.Collection; import java.util.Iterator; /** @@ -160,18 +161,41 @@ public class WithinPrefixTreeFilter extends AbstractVisitingPrefixTreeFilter { @Override protected void visitLeaf(Cell cell) throws IOException { - SpatialRelation relation = visitRelation; + //visitRelation is declared as a field, populated by visit() so we don't recompute it + assert detailLevel != cell.getLevel(); assert visitRelation == cell.getShape().relate(queryShape); - if (relation.intersects()) { + if (allCellsIntersectQuery(cell, visitRelation)) collectDocs(inside); - } else { + else collectDocs(outside); + } + + /** Returns true if the provided cell, and all its sub-cells down to + * detailLevel all intersect the queryShape. + */ + private boolean allCellsIntersectQuery(Cell cell, SpatialRelation relate/*cell to query*/) { + if (relate == null) + relate = cell.getShape().relate(queryShape); + if (cell.getLevel() == detailLevel) + return relate.intersects(); + if (relate == SpatialRelation.WITHIN) + return true; + if (relate == SpatialRelation.DISJOINT) + return false; + // Note: Generating all these cells just to determine intersection is not ideal. + // It was easy to implement but could be optimized. For example if the docs + // in question are already marked in the 'outside' bitset then it can be avoided. + Collection subCells = cell.getSubCells(null); + for (Cell subCell : subCells) { + if (!allCellsIntersectQuery(subCell, null))//recursion + return false; } + return true; } @Override protected void visitScanned(Cell cell) throws IOException { - if (queryShape.relate(cell.getShape()).intersects()) { + if (allCellsIntersectQuery(cell, null)) { collectDocs(inside); } else { collectDocs(outside); diff --git a/lucene/spatial/src/test/org/apache/lucene/spatial/prefix/SpatialOpRecursivePrefixTreeTest.java b/lucene/spatial/src/test/org/apache/lucene/spatial/prefix/SpatialOpRecursivePrefixTreeTest.java index d33d81cb6d5..dc85e50a716 100644 --- a/lucene/spatial/src/test/org/apache/lucene/spatial/prefix/SpatialOpRecursivePrefixTreeTest.java +++ b/lucene/spatial/src/test/org/apache/lucene/spatial/prefix/SpatialOpRecursivePrefixTreeTest.java @@ -55,12 +55,14 @@ public class SpatialOpRecursivePrefixTreeTest extends StrategyTestCase { deleteAll(); } - public void mySetup() throws IOException { + public void mySetup(int maxLevels) throws IOException { //non-geospatial makes this test a little easier (in gridSnap), and using boundary values 2^X raises // the prospect of edge conditions we want to test, plus makes for simpler numbers (no decimals). this.ctx = new SpatialContext(false, null, new RectangleImpl(0, 256, -128, 128, null)); //A fairly shallow grid, and default 2.5% distErrPct - this.grid = new QuadPrefixTree(ctx, randomIntBetween(1, 8)); + if (maxLevels == -1) + maxLevels = randomIntBetween(1, 8); + this.grid = new QuadPrefixTree(ctx, maxLevels); this.strategy = new RecursivePrefixTreeStrategy(grid, getClass().getSimpleName()); //((PrefixTreeStrategy) strategy).setDistErrPct(0);//fully precise to grid @@ -70,30 +72,27 @@ public class SpatialOpRecursivePrefixTreeTest extends StrategyTestCase { @Test @Repeat(iterations = 10) public void testIntersects() throws IOException { - mySetup(); + mySetup(-1); doTest(SpatialOperation.Intersects); } @Test @Repeat(iterations = 10) public void testWithin() throws IOException { - mySetup(); + mySetup(-1); doTest(SpatialOperation.IsWithin); } @Test @Repeat(iterations = 10) public void testContains() throws IOException { - mySetup(); + mySetup(-1); doTest(SpatialOperation.Contains); } @Test public void testWithinDisjointParts() throws IOException { - this.ctx = new SpatialContext(false, null, new RectangleImpl(0, 256, -128, 128, null)); - //A fairly shallow grid, and default 2.5% distErrPct - this.grid = new QuadPrefixTree(ctx, 7); - this.strategy = new RecursivePrefixTreeStrategy(grid, getClass().getSimpleName()); + mySetup(7); //one shape comprised of two parts, quite separated apart adoc("0", new ShapePair(ctx.makeRectangle(0, 10, -120, -100), ctx.makeRectangle(220, 240, 110, 125))); @@ -102,7 +101,30 @@ public class SpatialOpRecursivePrefixTreeTest extends StrategyTestCase { Query query = strategy.makeQuery(new SpatialArgs(SpatialOperation.IsWithin, ctx.makeRectangle(210, 245, 105, 128))); SearchResults searchResults = executeQuery(query, 1); //we shouldn't find it because it's not completely within - assertTrue(searchResults.numFound==0); + assertTrue(searchResults.numFound == 0); + } + + @Test /** LUCENE-4916 */ + public void testWithinLeafApproxRule() throws IOException { + mySetup(2);//4x4 grid + //indexed shape will simplify to entire right half (2 top cells) + adoc("0", ctx.makeRectangle(192, 204, -128, 128)); + commit(); + + ((RecursivePrefixTreeStrategy) strategy).setPrefixGridScanLevel(randomInt(2)); + + //query does NOT contain it; both indexed cells are leaves to the query, and + // when expanded to the full grid cells, the top one's top row is disjoint + // from the query and thus not a match. + assertTrue(executeQuery(strategy.makeQuery( + new SpatialArgs(SpatialOperation.IsWithin, ctx.makeRectangle(38, 192, -72, 56)) + ), 1).numFound==0);//no-match + + //this time the rect is a little bigger and is considered a match. It's a + // an acceptable false-positive because of the grid approximation. + assertTrue(executeQuery(strategy.makeQuery( + new SpatialArgs(SpatialOperation.IsWithin, ctx.makeRectangle(38, 192, -72, 80)) + ), 1).numFound==1);//match } private void doTest(final SpatialOperation operation) throws IOException {