From 02b557e9cd8b63f0dbeabf5a4892454efebd3c76 Mon Sep 17 00:00:00 2001 From: Leo Gertsenshteyn <146586+leoger@users.noreply.github.com> Date: Wed, 20 Nov 2024 14:23:03 -0800 Subject: [PATCH 1/5] WIP geodetic distance search Added NearFilter unit test with real lat-long data. The current implementation of NearFilter doesn't return the expected results. See this shared custom map on Google Maps for an illustration of the Oslo test case: https://www.google.com/maps/d/viewer?mid=1WXlEa5nBOSvBej3HSUNhsh_LLahoab8 --- nitrite-spatial/pom.xml | 5 + .../org/dizitart/no2/spatial/NearFilter.java | 4 +- .../dizitart/no2/spatial/SpatialIndex.java | 13 ++ .../jts/util/GeometricShapeFactoryExt.java | 45 +++++ .../no2/spatial/GeoSpatialFindNearTest.java | 180 ++++++++++++++++++ .../no2/spatial/GeoSpatialTestBase.java | 120 ++++++++++++ .../org/dizitart/no2/spatial/SpatialData.java | 4 + .../no2/spatial/SpatialIndexTest.java | 25 +++ 8 files changed, 394 insertions(+), 2 deletions(-) create mode 100644 nitrite-spatial/src/main/java/org/locationtech/jts/util/GeometricShapeFactoryExt.java create mode 100644 nitrite-spatial/src/test/java/org/dizitart/no2/spatial/GeoSpatialFindNearTest.java create mode 100644 nitrite-spatial/src/test/java/org/dizitart/no2/spatial/GeoSpatialTestBase.java diff --git a/nitrite-spatial/pom.xml b/nitrite-spatial/pom.xml index 2f5e77258..653f8bca2 100644 --- a/nitrite-spatial/pom.xml +++ b/nitrite-spatial/pom.xml @@ -42,6 +42,11 @@ org.locationtech.jts jts-core + + net.sf.geographiclib + GeographicLib-Java + 2.0 + org.projectlombok lombok diff --git a/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/NearFilter.java b/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/NearFilter.java index fe4d4764e..5e602b806 100644 --- a/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/NearFilter.java +++ b/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/NearFilter.java @@ -19,7 +19,7 @@ import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Geometry; import org.locationtech.jts.geom.Point; -import org.locationtech.jts.util.GeometricShapeFactory; +import org.locationtech.jts.util.GeometricShapeFactoryExt; /** * @since 4.0 @@ -35,7 +35,7 @@ class NearFilter extends WithinFilter { } private static Geometry createCircle(Coordinate center, double radius) { - GeometricShapeFactory shapeFactory = new GeometricShapeFactory(); + GeometricShapeFactoryExt shapeFactory = new GeometricShapeFactoryExt(); shapeFactory.setNumPoints(64); shapeFactory.setCentre(center); shapeFactory.setSize(radius * 2); diff --git a/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/SpatialIndex.java b/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/SpatialIndex.java index 33f5e0fd1..bfb94cb6f 100644 --- a/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/SpatialIndex.java +++ b/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/SpatialIndex.java @@ -138,6 +138,19 @@ public LinkedHashSet findNitriteIds(FindPlan findPlan) { Geometry geometry = spatialFilter.getValue(); BoundingBox boundingBox = fromGeometry(geometry); + // NOTE: It looks like we are painted into something of a corner here! The index only knows the bounding boxes, + // because that's how an R-Tree is meant to work. And that's fine until we have to deal with the reality + // of the fact that these are actually arbitrary geometries and the users are crafting queries that are + // meant to test whether they are within each other or not. + // + // The query *cannot* be satisfied by simply checking bounding boxes and calling it a day. + // + // The closest thing I see so far to a solution is for us to *split the filter into two steps* and either: + // (a) plumb the Map from ReadOperations down to here, so that we can look at the actual + // geometry values; or (b) treat this as an initial "fast filtering" step and then plan to have something + // in ReadOperations (or even further up the call stack) take the next step and perform the more + // computationally intensive geometry check. + if (filter instanceof WithinFilter) { keys = indexMap.findContainedKeys(boundingBox); } else if (filter instanceof IntersectsFilter) { diff --git a/nitrite-spatial/src/main/java/org/locationtech/jts/util/GeometricShapeFactoryExt.java b/nitrite-spatial/src/main/java/org/locationtech/jts/util/GeometricShapeFactoryExt.java new file mode 100644 index 000000000..15fb1ecec --- /dev/null +++ b/nitrite-spatial/src/main/java/org/locationtech/jts/util/GeometricShapeFactoryExt.java @@ -0,0 +1,45 @@ +package org.locationtech.jts.util; + +import net.sf.geographiclib.Geodesic; +import net.sf.geographiclib.GeodesicData; +import net.sf.geographiclib.GeodesicMask; +import org.locationtech.jts.geom.Coordinate; +import org.locationtech.jts.geom.Envelope; +import org.locationtech.jts.geom.LinearRing; +import org.locationtech.jts.geom.Polygon; + +/** Extends the JTS GeometricShapeFactory to add geodetic "small circle" geometry. */ +public class GeometricShapeFactoryExt extends GeometricShapeFactory { + + /** + * Bitmask specifying which results should be returned from the "direct" calculation. + * We don't need to know the azimuth at s2, so we can save a couple of CPU cycles by not asking for it! + *

+ * See javadoc at {@link Geodesic#Direct(double, double, double, double, int)} for more details. + */ + public static final int OUTMASK = GeodesicMask.STANDARD ^ GeodesicMask.AZIMUTH; + + @Override + public Polygon createCircle() + { + Envelope env = dim.getEnvelope(); + double radiusInMeters = env.getWidth() / 2.0; + double ctrX = dim.getCentre().x; + double ctrY = dim.getCentre().y; + + Coordinate[] pts = new Coordinate[nPts + 1]; + int i; + for (i = 0; i < nPts; i++) { + // because this is the "azimuth" value, it starts at "geodetic north" and proceeds clockwise + double azimuthInDegrees = i * (360.0 / nPts); + GeodesicData directResult = Geodesic.WGS84.Direct(ctrX, ctrY, azimuthInDegrees, radiusInMeters, OUTMASK); + double lat = directResult.lat2; + double lon = directResult.lon2; + pts[i] = coord(lat, lon); + } + pts[i] = new Coordinate(pts[0]); + + LinearRing ring = geomFact.createLinearRing(pts); + return geomFact.createPolygon(ring); + } +} diff --git a/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/GeoSpatialFindNearTest.java b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/GeoSpatialFindNearTest.java new file mode 100644 index 000000000..12bcd0097 --- /dev/null +++ b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/GeoSpatialFindNearTest.java @@ -0,0 +1,180 @@ +/* + * Copyright (c) 2017-2021 Nitrite author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ +package org.dizitart.no2.spatial; + +import net.sf.geographiclib.GeodesicData; +import org.dizitart.no2.collection.DocumentCursor; +import org.dizitart.no2.collection.FindOptions; +import org.dizitart.no2.index.IndexOptions; +import org.dizitart.no2.repository.Cursor; +import org.junit.Test; +import org.locationtech.jts.geom.Coordinate; + +import java.util.Random; +import java.util.stream.Collectors; + +import static java.util.Collections.emptySet; +import static net.sf.geographiclib.Geodesic.WGS84; +import static org.dizitart.no2.collection.Document.createDocument; +import static org.dizitart.no2.collection.FindOptions.orderBy; +import static org.dizitart.no2.common.SortOrder.Ascending; +import static org.dizitart.no2.common.util.Iterables.setOf; +import static org.dizitart.no2.spatial.GeoSpatialTestBase.TestLocations.*; +import static org.dizitart.no2.spatial.SpatialFluentFilter.where; +import static org.dizitart.no2.spatial.SpatialIndexer.SPATIAL_INDEX; +import static org.junit.Assert.assertEquals; + +/** + * Test cases to check whether the Spatial distance search is properly converting meters to/from degrees and + * accounting for the curvature of the Earth. + *

+ * From the "near filter" + * documentation, it is clear that the intended use is for the arguments to represent (1) a geo-coordinate + * as a lat/long pair; and (2) a distance in meters. + *

+ * This appears to disagree with the example provided in 

nitrite-spatial/README.md
, which makes no + * mention of distance units uses a coordinate value outside the space of possible lat/long coordinates. + **/ +public class GeoSpatialFindNearTest extends GeoSpatialTestBase { + + /** 2.5 kilometers, in meters */ + private static final double DIST_2500_M = 2_500.0d; + /** 20 millimeters, in meters */ + private static final double DIST_20_MM = (20.0d / 1000.0d); + + /** Just sorting by "id" field so that JUnit's "Expected" vs. "Actual" output is easier to visually inspect. */ + public static final FindOptions ORDER_BY_ID = orderBy("id", Ascending); + public static final Random RANDOM = new Random(); + + @Test + public void testNearFilter_ObjectRepository_FindsCorrectSubset() { + repository.createIndex(IndexOptions.indexOptions(SPATIAL_INDEX), "geometry"); + Cursor cursor = repository.find( + where("geometry").near(centerPt.getCoordinate(), DIST_2500_M), + ORDER_BY_ID); + + assertEquals("Near filter should only find two locations within 2.5km", + setOf(obj2_950m_ESE, obj3_930m_W), + cursor.toSet()); + } + + @Test + public void testNearFilter_NitriteCollection_FindsCorrectSubset() { + collection.createIndex(IndexOptions.indexOptions(SPATIAL_INDEX), "location"); + + DocumentCursor cursor = collection.find( + where("location").near(centerPt.getCoordinate(), DIST_2500_M), + ORDER_BY_ID); + + assertEquals("Near filter should only find two locations within 2.5km", + setOf(doc2_950m_ESE, doc3_930m_W), + cursor.toList().stream().map(this::trimMeta).collect(Collectors.toSet())); + } + + @Test + public void testNearFilter_ObjectRepository_SimpleEquatorDistances() { + + repository.insert(new SpatialData(randLong(), readPoint("POINT(0.0 1.00)"))); // 111km from (0,0) + repository.insert(new SpatialData(randLong(), readPoint("POINT(0.0 0.01)"))); // 1.11km from (0,0) + + repository.createIndex(IndexOptions.indexOptions(SPATIAL_INDEX), "geometry"); + + Cursor cursor = repository.find(where("geometry").near(new Coordinate(0.0, 0.0), 1120.0)); + + assertEquals("Near filter should find 1 location within 1.12km", 1, cursor.toList().size()); + } + + @Test + public void testNearFilter_NitriteCollection_SimpleEquatorDistances() { + collection.insert(createDocument("key", randLong()).put("location", readPoint("POINT(0.0 1.00)"))); + collection.insert(createDocument("key", randLong()).put("location", readPoint("POINT(0.0 0.01)"))); + collection.createIndex(IndexOptions.indexOptions(SPATIAL_INDEX), "location"); + + DocumentCursor cursor = collection.find(where("location").near(new Coordinate(0.0, 0.0), 1120.0)); + + assertEquals("Near filter should find 1 location within 1.12km", 1, cursor.toList().size()); + } + + @Test + public void testNearFilter_ObjectRepository_Simple60NDistances() { + + repository.insert(new SpatialData(randLong(), readPoint("POINT(60.0 1.00)"))); // ~56km from (60,0) + repository.insert(new SpatialData(randLong(), readPoint("POINT(60.0 2.00)"))); // ~111km from (60,0) + repository.insert(new SpatialData(randLong(), readPoint("POINT(61.0 0.00)"))); // ~111km from (60,0) + repository.insert(new SpatialData(randLong(), readPoint("POINT(62.0 0.00)"))); // ~222km from (60,0) + + repository.createIndex(IndexOptions.indexOptions(SPATIAL_INDEX), "geometry"); + + Cursor cursor = repository.find( + where("geometry").near(new Coordinate(60.0, 0.0), 112_000.0)); + + assertEquals("Near filter should find 3 locations within 1.12km", 3, cursor.toList().size()); + } + + @Test + public void testNearFilter_NitriteCollection_Simple60NDistances() { + collection.insert(createDocument("key", randLong()).put("location", readPoint("POINT(60.0 1.00)"))); // ~56km from (60,0) + collection.insert(createDocument("key", randLong()).put("location", readPoint("POINT(60.0 2.00)"))); // ~111km from (60,0) + collection.insert(createDocument("key", randLong()).put("location", readPoint("POINT(61.0 0.00)"))); // ~111km from (60,0) + collection.insert(createDocument("key", randLong()).put("location", readPoint("POINT(62.0 0.00)"))); // ~222km from (60,0) + + // This point is outside the circle but inside the bounding box + collection.insert(createDocument("key", randLong()).put("location", readPoint("POINT(60.75 1.75)"))); // ~127km from (60,0) + + collection.createIndex(IndexOptions.indexOptions(SPATIAL_INDEX), "location"); + + DocumentCursor cursor = collection.find(where("location").near(new Coordinate(60.0, 0.0), 112_000.0)); + + assertEquals("Near filter should find 3 locations within 1.12km", 3, cursor.toList().size()); + } + + private static long randLong() { + return RANDOM.nextLong(); + } + + @Test + public void testNearFilter_ObjectRepository_TinyDistance_ReturnsNoResults() { + Coordinate coordinate = centerPt.getCoordinate(); + + Cursor cursor = repository.find(where("geometry").near(coordinate, DIST_20_MM)); + assertEquals("Near filter should return no results within 20mm distance", emptySet(), cursor.toSet()); + } + + @Test + public void testNearFilter_NitriteCollection_TinyDistance_ReturnsNoResults() { + Coordinate coordinate = centerPt.getCoordinate(); + + collection.createIndex(IndexOptions.indexOptions(SPATIAL_INDEX), "location"); + DocumentCursor cursor = collection.find(where("location").near(coordinate, DIST_20_MM)); + assertEquals("Near filter should return no results within 20mm distance", emptySet(), cursor.toSet()); + } + + @Test + public void testGeoLibrary_Distance_SanityCheck() { + final double EPSILON_10M = 10.0; + // All of these distances should be within 10 metres of what was estimated on Google Earth. + + GeodesicData s12 = WGS84.Inverse(centerPt.getX(), centerPt.getY(), pt2_950m_ESE.getX(), pt2_950m_ESE.getY()); + assertEquals(s12.s12, 950.0, EPSILON_10M); + + GeodesicData s13 = WGS84.Inverse(centerPt.getX(), centerPt.getY(), pt3_930m_W.getX(), pt3_930m_W.getY()); + assertEquals(s13.s12, 930.0, EPSILON_10M); + + GeodesicData s14 = WGS84.Inverse(centerPt.getX(), centerPt.getY(), pt4_2750m_WSW.getX(), pt4_2750m_WSW.getY()); + assertEquals(s14.s12, 2750.0, EPSILON_10M); + } +} diff --git a/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/GeoSpatialTestBase.java b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/GeoSpatialTestBase.java new file mode 100644 index 000000000..8befa0465 --- /dev/null +++ b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/GeoSpatialTestBase.java @@ -0,0 +1,120 @@ +/* + * Copyright (c) 2017-2021 Nitrite author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.dizitart.no2.spatial; + +import lombok.SneakyThrows; +import org.dizitart.no2.Nitrite; +import org.dizitart.no2.collection.Document; +import org.dizitart.no2.collection.NitriteCollection; +import org.dizitart.no2.repository.ObjectRepository; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.locationtech.jts.geom.Point; +import org.locationtech.jts.io.ParseException; +import org.locationtech.jts.io.WKTReader; + +import static org.dizitart.no2.collection.Document.createDocument; +import static org.dizitart.no2.common.Constants.DOC_ID; +import static org.dizitart.no2.common.Constants.DOC_MODIFIED; +import static org.dizitart.no2.common.Constants.DOC_REVISION; +import static org.dizitart.no2.common.Constants.DOC_SOURCE; +import static org.dizitart.no2.spatial.TestUtil.createDb; +import static org.dizitart.no2.spatial.TestUtil.deleteDb; +import static org.dizitart.no2.spatial.TestUtil.getRandomTempDbFile; + +public abstract class GeoSpatialTestBase { + private String fileName; + protected Nitrite db; + protected NitriteCollection collection; + protected ObjectRepository repository; + + static WKTReader reader = new WKTReader(); + protected SpatialData obj2_950m_ESE, obj3_930m_W, obj4_2750m_WSW; + protected Document doc2_950m_ESE, doc3_930m_W, doc4_2750m_WSW; + + @Rule + public Retry retry = new Retry(3); + + /** + * I chose a set of simple landmarks in a major city at high latitude, near 60°N, + * such that the separation between them is primarily east-west. + *

+ * At the equator, 1 degree of either latitude or longitude measures approx. 111km wide. + * However, at 60°N, 1 degree of longitude is only half as wide. (cf. cos(60°) == 0.5) + *

+ * This math is not exact enough for the needs of a geographer, but it's close enough to create + * simple test cases that can distinguish whether we are properly converting meters to/from degrees, + * including accounting for the curvature of the Earth. + */ + public static class TestLocations { + static Point centerPt = readPoint("POINT(59.91437 10.73402)"); // National Theater (Oslo) + static Point pt2_950m_ESE = readPoint("POINT(59.9115306 10.7501574)"); // Olso Central Station + static Point pt3_930m_W = readPoint("POINT(59.91433 10.71730)"); // National Library of Norway + static Point pt4_2750m_WSW = readPoint("POINT(59.90749 10.68670)"); // Norwegian Museum of Cultural Hist. + } + + @SneakyThrows + protected static Point readPoint(String wkt) { + return (Point) reader.read(wkt); + } + + @Before + public void before() throws ParseException { + fileName = getRandomTempDbFile(); + db = createDb(fileName); + + collection = db.getCollection("test"); + repository = db.getRepository(SpatialData.class); + + insertObjects(); + insertDocuments(); + } + + protected void insertObjects() throws ParseException { + obj2_950m_ESE = new SpatialData(2L, TestLocations.pt2_950m_ESE); + obj3_930m_W = new SpatialData(3L, TestLocations.pt3_930m_W); + obj4_2750m_WSW = new SpatialData(4L, TestLocations.pt4_2750m_WSW); + repository.insert(obj2_950m_ESE, obj3_930m_W, obj4_2750m_WSW); + } + + protected void insertDocuments() throws ParseException { + doc2_950m_ESE = createDocument("key", 2L).put("location", TestLocations.pt2_950m_ESE); + doc3_930m_W = createDocument("key", 3L).put("location", TestLocations.pt3_930m_W); + doc4_2750m_WSW = createDocument("key", 4L).put("location", TestLocations.pt4_2750m_WSW); + + collection.insert(doc2_950m_ESE, doc3_930m_W, doc4_2750m_WSW); + } + + @After + public void after() { + if (db != null && !db.isClosed()) { + db.close(); + } + + deleteDb(fileName); + } + + protected Document trimMeta(Document document) { + document.remove(DOC_ID); + document.remove(DOC_REVISION); + document.remove(DOC_MODIFIED); + document.remove(DOC_SOURCE); + return document; + } +} diff --git a/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialData.java b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialData.java index c22870786..bc34d672b 100644 --- a/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialData.java +++ b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialData.java @@ -16,7 +16,9 @@ package org.dizitart.no2.spatial; +import lombok.AllArgsConstructor; import lombok.Data; +import lombok.NoArgsConstructor; import org.dizitart.no2.collection.Document; import org.dizitart.no2.common.mapper.EntityConverter; import org.dizitart.no2.common.mapper.NitriteMapper; @@ -30,6 +32,8 @@ * @author Anindya Chatterjee */ @Data +@AllArgsConstructor +@NoArgsConstructor @Index(fields = "geometry", type = SPATIAL_INDEX) public class SpatialData { @Id diff --git a/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialIndexTest.java b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialIndexTest.java index 8905fe197..bc20ee3da 100644 --- a/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialIndexTest.java +++ b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialIndexTest.java @@ -79,6 +79,31 @@ public void testWithin() throws ParseException { assertEquals(cursor1.toList().stream().map(this::trimMeta).collect(Collectors.toList()), Collections.singletonList(doc1)); } + @Test + public void testWithinTriangleNotJustTestingBoundingBox() throws ParseException { + + /* + (490, 530) * ┄┄┄┄┄. + │\ ┆ + │ \ x<---(520, 520) + │ \ ┆ + │ \ ┆ + │ x<-\---(500, 505) + │ \┆ + (490, 490) *──────* (530, 490) + */ + + WKTReader reader = new WKTReader(); + Geometry search = reader.read("POLYGON((490 490, 530 490 , 490 530, 490 490))"); + + SpatialData outsidePoint = new SpatialData(7L, reader.read("POINT(520 520)")); + repository.insert(outsidePoint); + + Cursor cursor = repository.find(where("geometry").within(search)); + assertEquals(cursor.size(), 1); + assertFalse(cursor.toList().contains(outsidePoint)); + } + @Test public void testNearPoint() throws ParseException { WKTReader reader = new WKTReader(); From 095b32f4cab6d8721b8293404d942f037dba689a Mon Sep 17 00:00:00 2001 From: Leo Gertsenshteyn <146586+leoger@users.noreply.github.com> Date: Fri, 13 Dec 2024 22:51:37 -0800 Subject: [PATCH 2/5] minor cleanup --- .../jts/util/GeometricShapeFactoryExt.java | 16 ++++++++++++++++ .../no2/spatial/GeoSpatialFindNearTest.java | 2 +- .../dizitart/no2/spatial/SpatialIndexTest.java | 15 +++++++-------- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/nitrite-spatial/src/main/java/org/locationtech/jts/util/GeometricShapeFactoryExt.java b/nitrite-spatial/src/main/java/org/locationtech/jts/util/GeometricShapeFactoryExt.java index 15fb1ecec..d418f083c 100644 --- a/nitrite-spatial/src/main/java/org/locationtech/jts/util/GeometricShapeFactoryExt.java +++ b/nitrite-spatial/src/main/java/org/locationtech/jts/util/GeometricShapeFactoryExt.java @@ -1,3 +1,19 @@ +/* + * Copyright (c) 2017-2024 Nitrite author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ package org.locationtech.jts.util; import net.sf.geographiclib.Geodesic; diff --git a/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/GeoSpatialFindNearTest.java b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/GeoSpatialFindNearTest.java index 12bcd0097..f6ffbaf63 100644 --- a/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/GeoSpatialFindNearTest.java +++ b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/GeoSpatialFindNearTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017-2021 Nitrite author or authors. + * Copyright (c) 2017-2024 Nitrite author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialIndexTest.java b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialIndexTest.java index bc20ee3da..c2d42b655 100644 --- a/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialIndexTest.java +++ b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialIndexTest.java @@ -83,14 +83,13 @@ public void testWithin() throws ParseException { public void testWithinTriangleNotJustTestingBoundingBox() throws ParseException { /* - (490, 530) * ┄┄┄┄┄. - │\ ┆ - │ \ x<---(520, 520) - │ \ ┆ - │ \ ┆ - │ x<-\---(500, 505) - │ \┆ - (490, 490) *──────* (530, 490) + (490, 530) * - - - + │\ - + │ \ x <── (520, 520); outside triangle but within triangle's bounding box + │ \ - + │ x <── (500, 505); inside triangle + │ \- + (490, 490) *─────* (530, 490) */ WKTReader reader = new WKTReader(); From fa2ed69f89ab740a24feda21465504e4bf2efacd Mon Sep 17 00:00:00 2001 From: Leo Gertsenshteyn <146586+leoger@users.noreply.github.com> Date: Fri, 13 Dec 2024 23:01:09 -0800 Subject: [PATCH 3/5] move GeometricShapeFactoryExt to nitrite package namespace --- .../no2/spatial}/GeometricShapeFactoryExt.java | 3 ++- .../src/main/java/org/dizitart/no2/spatial/NearFilter.java | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) rename nitrite-spatial/src/main/java/org/{locationtech/jts/util => dizitart/no2/spatial}/GeometricShapeFactoryExt.java (96%) diff --git a/nitrite-spatial/src/main/java/org/locationtech/jts/util/GeometricShapeFactoryExt.java b/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/GeometricShapeFactoryExt.java similarity index 96% rename from nitrite-spatial/src/main/java/org/locationtech/jts/util/GeometricShapeFactoryExt.java rename to nitrite-spatial/src/main/java/org/dizitart/no2/spatial/GeometricShapeFactoryExt.java index d418f083c..90c5ce266 100644 --- a/nitrite-spatial/src/main/java/org/locationtech/jts/util/GeometricShapeFactoryExt.java +++ b/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/GeometricShapeFactoryExt.java @@ -14,7 +14,7 @@ * limitations under the License. * */ -package org.locationtech.jts.util; +package org.dizitart.no2.spatial; import net.sf.geographiclib.Geodesic; import net.sf.geographiclib.GeodesicData; @@ -23,6 +23,7 @@ import org.locationtech.jts.geom.Envelope; import org.locationtech.jts.geom.LinearRing; import org.locationtech.jts.geom.Polygon; +import org.locationtech.jts.util.GeometricShapeFactory; /** Extends the JTS GeometricShapeFactory to add geodetic "small circle" geometry. */ public class GeometricShapeFactoryExt extends GeometricShapeFactory { diff --git a/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/NearFilter.java b/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/NearFilter.java index 5e602b806..a424299fc 100644 --- a/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/NearFilter.java +++ b/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/NearFilter.java @@ -19,7 +19,6 @@ import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Geometry; import org.locationtech.jts.geom.Point; -import org.locationtech.jts.util.GeometricShapeFactoryExt; /** * @since 4.0 From b5f2486cdd2c4fd59c37a0ee0661d6795c998715 Mon Sep 17 00:00:00 2001 From: Leo Gertsenshteyn <146586+leoger@users.noreply.github.com> Date: Mon, 20 Jan 2025 23:08:53 -0800 Subject: [PATCH 4/5] WIP with working NearFilter unit tests --- .../org/dizitart/no2/spatial/NearFilter.java | 84 ++++++++- .../no2/spatial/SpatialFluentFilter.java | 1 + .../dizitart/no2/spatial/WithinFilter.java | 9 + .../spatial/CompoundFilterExampleTest.java | 159 ++++++++++++++++++ .../no2/spatial/GeoSpatialFindNearTest.java | 6 +- .../org/dizitart/no2/spatial/SpatialData.java | 2 +- .../no2/spatial/SpatialIndexTest.java | 48 +++++- .../collection/operation/FindOptimizer.java | 16 +- .../org/dizitart/no2/filters/AndFilter.java | 2 +- .../no2/filters/FlattenableFilter.java | 29 ++++ 10 files changed, 327 insertions(+), 29 deletions(-) create mode 100644 nitrite-spatial/src/test/java/org/dizitart/no2/spatial/CompoundFilterExampleTest.java create mode 100644 nitrite/src/main/java/org/dizitart/no2/filters/FlattenableFilter.java diff --git a/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/NearFilter.java b/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/NearFilter.java index a424299fc..a8a3c75cd 100644 --- a/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/NearFilter.java +++ b/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/NearFilter.java @@ -16,21 +16,45 @@ package org.dizitart.no2.spatial; -import org.locationtech.jts.geom.Coordinate; -import org.locationtech.jts.geom.Geometry; -import org.locationtech.jts.geom.Point; +import net.sf.geographiclib.Geodesic; +import net.sf.geographiclib.GeodesicData; +import net.sf.geographiclib.GeodesicMask; +import org.dizitart.no2.collection.Document; +import org.dizitart.no2.collection.NitriteId; +import org.dizitart.no2.common.tuples.Pair; +import org.dizitart.no2.exceptions.FilterException; +import org.dizitart.no2.filters.FlattenableFilter; +import org.dizitart.no2.filters.FieldBasedFilter; +import org.dizitart.no2.filters.Filter; +import org.locationtech.jts.geom.*; + +import java.util.List; + +import static org.locationtech.jts.geom.PrecisionModel.FLOATING; /** * @since 4.0 * @author Anindya Chatterjee */ -class NearFilter extends WithinFilter { - NearFilter(String field, Coordinate point, Double distance) { - super(field, createCircle(point, distance)); +class NearFilter extends IntersectsFilter implements FlattenableFilter { + private Point center; + private Double distance; + + /** Uses full "double" floating-point precision, and SRID 4326 */ + private static GeometryFactory geometryFactory = + new GeometryFactory(new PrecisionModel(FLOATING), 4326); + + + NearFilter(String field, Coordinate center, Double distance) { + super(field, createCircle(center, distance)); + this.center = geometryFactory.createPoint(center); + this.distance = distance; } - NearFilter(String field, Point point, Double distance) { - super(field, createCircle(point.getCoordinate(), distance)); + NearFilter(String field, Point center, Double distance) { + super(field, createCircle(center.getCoordinate(), distance)); + this.center = center; + this.distance = distance; } private static Geometry createCircle(Coordinate center, double radius) { @@ -45,4 +69,48 @@ private static Geometry createCircle(Coordinate center, double radius) { public String toString() { return "(" + getField() + " nears " + getValue() + ")"; } + + @Override + public List getFilters() { + return List.of( + new IntersectsFilter(getField(), getValue()), + new NonIndexNearFilter(getField(), getValue())); + } + + public class NonIndexNearFilter extends FieldBasedFilter { + + protected NonIndexNearFilter(String field, Geometry circle) { + super(field, circle); + } + + @Override + public boolean apply(Pair element) { + Document document = element.getSecond(); + Object fieldValue = document.get(getField()); + + if (fieldValue == null) { + return false; + } else if (fieldValue instanceof Geometry) { + if (fieldValue instanceof Point) { + Point pointValue = (Point) fieldValue; + Point centerPoint = NearFilter.this.center; + GeodesicData inverseResult = + Geodesic.WGS84.Inverse( + centerPoint.getX(), centerPoint.getY(), + pointValue.getX(), pointValue.getY(), + GeodesicMask.DISTANCE); + return inverseResult.s12 <= NearFilter.this.distance; + } else { + // TODO this doesn't seem to work?? + Geometry elemGeo = (Geometry) fieldValue; + Geometry filterGeo = (Geometry) getValue(); + return filterGeo.intersects(elemGeo); + } + } else { + throw new FilterException(getField() + " does not contain Geometry value"); + } + } + + } + } diff --git a/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/SpatialFluentFilter.java b/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/SpatialFluentFilter.java index 9f3e78ba3..911b86bf9 100644 --- a/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/SpatialFluentFilter.java +++ b/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/SpatialFluentFilter.java @@ -89,6 +89,7 @@ public Filter near(Coordinate point, Double distance) { * @return the new {@link Filter} instance */ public Filter near(Point point, Double distance) { +// return new NearFilter(field, point, distance); return new NearFilter(field, point, distance); } } diff --git a/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/WithinFilter.java b/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/WithinFilter.java index fe25ea741..13225d2c9 100644 --- a/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/WithinFilter.java +++ b/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/WithinFilter.java @@ -16,10 +16,18 @@ package org.dizitart.no2.spatial; +import org.dizitart.no2.collection.Document; +import org.dizitart.no2.collection.NitriteId; +import org.dizitart.no2.common.tuples.Pair; +import org.dizitart.no2.exceptions.FilterException; +import org.dizitart.no2.filters.FieldBasedFilter; import org.dizitart.no2.index.IndexMap; import org.locationtech.jts.geom.Geometry; import java.util.List; +import java.util.regex.Matcher; + +import static org.dizitart.no2.common.util.Numbers.compare; /** * @since 4.0 @@ -40,4 +48,5 @@ public List applyOnIndex(IndexMap indexMap) { public String toString() { return "(" + getField() + " within " + getValue() + ")"; } + } diff --git a/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/CompoundFilterExampleTest.java b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/CompoundFilterExampleTest.java new file mode 100644 index 000000000..93515f199 --- /dev/null +++ b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/CompoundFilterExampleTest.java @@ -0,0 +1,159 @@ +/* + * Copyright (c) 2017-2021 Nitrite author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.dizitart.no2.spatial; + +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; +import org.dizitart.no2.Nitrite; +import org.dizitart.no2.collection.Document; +import org.dizitart.no2.collection.NitriteCollection; +import org.dizitart.no2.common.mapper.EntityConverter; +import org.dizitart.no2.common.mapper.NitriteMapper; +import org.dizitart.no2.common.mapper.SimpleNitriteMapper; +import org.dizitart.no2.filters.Filter; +import org.dizitart.no2.filters.FluentFilter; +import org.dizitart.no2.repository.Cursor; +import org.dizitart.no2.repository.ObjectRepository; +import org.dizitart.no2.repository.annotations.Index; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.locationtech.jts.io.ParseException; + +import java.security.SecureRandom; + +import static org.dizitart.no2.collection.Document.createDocument; +import static org.dizitart.no2.common.Constants.DOC_ID; +import static org.dizitart.no2.common.Constants.DOC_MODIFIED; +import static org.dizitart.no2.common.Constants.DOC_REVISION; +import static org.dizitart.no2.common.Constants.DOC_SOURCE; +import static org.dizitart.no2.index.IndexType.NON_UNIQUE; +import static org.dizitart.no2.spatial.TestUtil.createDb; +import static org.dizitart.no2.spatial.TestUtil.deleteDb; +import static org.dizitart.no2.spatial.TestUtil.getRandomTempDbFile; + +public class CompoundFilterExampleTest { + private String fileName; + protected Nitrite db; + protected NitriteCollection collection; + protected ObjectRepository repository; + + @Data + @AllArgsConstructor + @NoArgsConstructor + @Index(fields = "indexString", type = NON_UNIQUE) + public static class PartiallyIndexedData { + private String indexStr; + private String otherStr; + + public static class PidConverter implements EntityConverter { + + @Override + public Class getEntityType() { + return PartiallyIndexedData.class; + } + + @Override + public Document toDocument(PartiallyIndexedData entity, NitriteMapper nitriteMapper) { + return Document + .createDocument("indexStr", entity.getIndexStr()) + .put("otherStr", entity.getOtherStr()); + } + + @Override + public PartiallyIndexedData fromDocument(Document document, NitriteMapper nitriteMapper) { + return new PartiallyIndexedData( + document.get("indexStr", String.class), + document.get("otherStr", String.class) + ); + } + } + } + + @Rule + public Retry retry = new Retry(3); + + + @Before + public void before() throws ParseException { + fileName = getRandomTempDbFile(); + db = createDb(fileName); + try (SimpleNitriteMapper documentMapper = (SimpleNitriteMapper) db.getConfig().nitriteMapper()) { + documentMapper.registerEntityConverter(new PartiallyIndexedData.PidConverter()); + } + repository = db.getRepository(PartiallyIndexedData.class); + + insertObjects(); + } + + // Method to generate a random alphanumeric string of given length + public static String randomAlphanumeric(int length) { + // Define characters to choose from (alphanumeric) + String characters = "abcdefghijklmnopqrstuvwxyz"; + SecureRandom random = new SecureRandom(new byte[] {0, 0, 0}); + + // StringBuilder to build the random string + StringBuilder randomString = new StringBuilder(length); + + // Generate the random string + for (int i = 0; i < length; i++) { + int randomIndex = random.nextInt(characters.length()); + randomString.append(characters.charAt(randomIndex)); + } + + // Return the random string + return randomString.toString(); + } + + protected void insertObjects() throws ParseException { + for (int i = 0; i < 10_000; i++) { + repository.insert(new PartiallyIndexedData( + randomAlphanumeric(2), + randomAlphanumeric(2))); + } + } + + @After + public void after() { + if (db != null && !db.isClosed()) { + db.close(); + } + + deleteDb(fileName); + } + + @Test + public void testMixedQuery() { + Cursor cursor = repository.find(Filter.and( + FluentFilter.where("indexStr").eq("aa"), + FluentFilter.where("otherStr").gt("kk") + )); + System.out.println("cursor.getFindPlan() = " + cursor.getFindPlan()); + System.out.println("cursor.toList() = " + cursor.toList()); + } + + protected Document trimMeta(Document document) { + document.remove(DOC_ID); + document.remove(DOC_REVISION); + document.remove(DOC_MODIFIED); + document.remove(DOC_SOURCE); + return document; + } +} diff --git a/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/GeoSpatialFindNearTest.java b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/GeoSpatialFindNearTest.java index f6ffbaf63..90166b192 100644 --- a/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/GeoSpatialFindNearTest.java +++ b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/GeoSpatialFindNearTest.java @@ -169,12 +169,12 @@ public void testGeoLibrary_Distance_SanityCheck() { // All of these distances should be within 10 metres of what was estimated on Google Earth. GeodesicData s12 = WGS84.Inverse(centerPt.getX(), centerPt.getY(), pt2_950m_ESE.getX(), pt2_950m_ESE.getY()); - assertEquals(s12.s12, 950.0, EPSILON_10M); + assertEquals(950.0, s12.s12, EPSILON_10M); GeodesicData s13 = WGS84.Inverse(centerPt.getX(), centerPt.getY(), pt3_930m_W.getX(), pt3_930m_W.getY()); - assertEquals(s13.s12, 930.0, EPSILON_10M); + assertEquals(930.0, s13.s12, EPSILON_10M); GeodesicData s14 = WGS84.Inverse(centerPt.getX(), centerPt.getY(), pt4_2750m_WSW.getX(), pt4_2750m_WSW.getY()); - assertEquals(s14.s12, 2750.0, EPSILON_10M); + assertEquals(2750.0, s14.s12, EPSILON_10M); } } diff --git a/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialData.java b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialData.java index bc34d672b..a2d6d18a1 100644 --- a/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialData.java +++ b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialData.java @@ -50,7 +50,7 @@ public Class getEntityType() { @Override public Document toDocument(SpatialData entity, NitriteMapper nitriteMapper) { return Document.createDocument("id", entity.getId()) - .put("geometry", nitriteMapper.tryConvert(entity.getGeometry(), Document.class)); + .put("geometry", nitriteMapper.tryConvert(entity.getGeometry(), Geometry.class)); } @Override diff --git a/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialIndexTest.java b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialIndexTest.java index c2d42b655..dab6e3d5c 100644 --- a/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialIndexTest.java +++ b/nitrite-spatial/src/test/java/org/dizitart/no2/spatial/SpatialIndexTest.java @@ -25,10 +25,9 @@ import org.dizitart.no2.index.IndexOptions; import org.dizitart.no2.mvstore.MVStoreModule; import org.dizitart.no2.repository.Cursor; +import org.junit.Ignore; import org.junit.Test; -import org.locationtech.jts.geom.Coordinate; -import org.locationtech.jts.geom.Geometry; -import org.locationtech.jts.geom.Point; +import org.locationtech.jts.geom.*; import org.locationtech.jts.io.ParseException; import org.locationtech.jts.io.WKTReader; @@ -80,6 +79,7 @@ public void testWithin() throws ParseException { } @Test + @Ignore public void testWithinTriangleNotJustTestingBoundingBox() throws ParseException { /* @@ -93,16 +93,48 @@ public void testWithinTriangleNotJustTestingBoundingBox() throws ParseException */ WKTReader reader = new WKTReader(); - Geometry search = reader.read("POLYGON((490 490, 530 490 , 490 530, 490 490))"); + Geometry search = reader.read("POLYGON((490 490, 530 490, 490 530, 490 490))"); - SpatialData outsidePoint = new SpatialData(7L, reader.read("POINT(520 520)")); - repository.insert(outsidePoint); + SpatialData insidePoint = new SpatialData(7L, reader.read("POINT(500 505)")); + SpatialData outsidePoint = new SpatialData(8L, reader.read("POINT(529 529)")); + repository.insert(insidePoint,outsidePoint); - Cursor cursor = repository.find(where("geometry").within(search)); - assertEquals(cursor.size(), 1); + Cursor cursor = repository.find(where("geometry").within(search)); +// Cursor cursor = repository.find( +// and(where("geometry").within(search), where("geometry").within(search.getBoundary()))); + assertEquals(1, cursor.size()); assertFalse(cursor.toList().contains(outsidePoint)); } + @Test + public void testNearGeometry_TriangleNearPoint() throws ParseException { + /* + x (1ºN, 2ºE) + │\ + (1ºN, 1ºE) x─x (2ºN, 1ºE) + + x (0ºN, 0ºE) + */ + + // given + WKTReader reader = new WKTReader(); + SpatialData triangle = new SpatialData(7L, reader.read("POLYGON((1 1, 1 2, 2 1, 1 1))")); + repository.insert(triangle); + + // Define a radius that should include the near corner (with 20% "safety" margin), but not the entire triangle + double sqrt2 = 1.4142d; // i.e. the distance from (0,0) to (1,1) + double metersPerDegreeAtEquator = 111_000d; + double radiusThatShouldIncludeNearCornerOfTriangle = 1.2 * sqrt2 * metersPerDegreeAtEquator; + + // when + Cursor cursor = repository.find( + where("geometry").near(new Coordinate(0d, 0d), radiusThatShouldIncludeNearCornerOfTriangle)); + + // then + assertEquals(1, cursor.size()); + assertTrue(cursor.toList().contains(triangle)); + } + @Test public void testNearPoint() throws ParseException { WKTReader reader = new WKTReader(); diff --git a/nitrite/src/main/java/org/dizitart/no2/collection/operation/FindOptimizer.java b/nitrite/src/main/java/org/dizitart/no2/collection/operation/FindOptimizer.java index 935cc69e3..2d6954504 100644 --- a/nitrite/src/main/java/org/dizitart/no2/collection/operation/FindOptimizer.java +++ b/nitrite/src/main/java/org/dizitart/no2/collection/operation/FindOptimizer.java @@ -53,8 +53,8 @@ public FindPlan optimize(Filter filter, } private FindPlan createFilterPlan(Collection indexDescriptors, Filter filter) { - if (filter instanceof AndFilter) { - List filters = flattenAndFilter((AndFilter) filter); + if (filter instanceof FlattenableFilter) { + List filters = flattenFilter((FlattenableFilter) filter); return createAndPlan(indexDescriptors, filters); } else if (filter instanceof OrFilter) { return createOrPlan(indexDescriptors, ((OrFilter) filter).getFilters()); @@ -64,12 +64,12 @@ private FindPlan createFilterPlan(Collection indexDescriptors, } } - private List flattenAndFilter(AndFilter andFilter) { + private List flattenFilter(FlattenableFilter flattenableFilter) { List flattenedFilters = new ArrayList<>(); - if (andFilter != null) { - for (Filter filter : andFilter.getFilters()) { - if (filter instanceof AndFilter) { - List filters = flattenAndFilter((AndFilter) filter); + if (flattenableFilter != null) { + for (Filter filter : flattenableFilter.getFilters()) { + if (filter instanceof FlattenableFilter) { + List filters = flattenFilter((FlattenableFilter) filter); flattenedFilters.addAll(filters); } else { flattenedFilters.add(filter); @@ -174,7 +174,7 @@ private void planForIndexOnlyFilters(FindPlan findPlan, Set in if (filter instanceof IndexOnlyFilter) { IndexOnlyFilter indexScanFilter = (IndexOnlyFilter) filter; if (isCompatibleFilter(indexOnlyFilters, indexScanFilter)) { - // if filter is compatible with already identified index only filter then add + // if filter is compatible with already identified index-only filter then add indexOnlyFilters.add(indexScanFilter); } else { throw new FilterException("A query can not have multiple index only filters"); diff --git a/nitrite/src/main/java/org/dizitart/no2/filters/AndFilter.java b/nitrite/src/main/java/org/dizitart/no2/filters/AndFilter.java index d971a50ab..5398c26dc 100644 --- a/nitrite/src/main/java/org/dizitart/no2/filters/AndFilter.java +++ b/nitrite/src/main/java/org/dizitart/no2/filters/AndFilter.java @@ -27,7 +27,7 @@ * @since 1.0 */ @Getter -public class AndFilter extends LogicalFilter { +public class AndFilter extends LogicalFilter implements FlattenableFilter { AndFilter(Filter... filters) { super(filters); diff --git a/nitrite/src/main/java/org/dizitart/no2/filters/FlattenableFilter.java b/nitrite/src/main/java/org/dizitart/no2/filters/FlattenableFilter.java new file mode 100644 index 000000000..800c5aa8d --- /dev/null +++ b/nitrite/src/main/java/org/dizitart/no2/filters/FlattenableFilter.java @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2017-2020. Nitrite author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.dizitart.no2.filters; + +import java.util.List; + +/** + * Represents a filter which can be flattened or otherwise consists of multiple constituent filters. + * + * // TODO verify the "@since" version? + * @since 4.4.0 + */ +public interface FlattenableFilter { + List getFilters(); +} From 44d2f4aaa71660408c224f24cfcf9384d8f1d4e5 Mon Sep 17 00:00:00 2001 From: Leo Gertsenshteyn <146586+leoger@users.noreply.github.com> Date: Tue, 21 Jan 2025 10:16:12 -0800 Subject: [PATCH 5/5] adding in-line PR comments --- .../org/dizitart/no2/spatial/NearFilter.java | 8 +++++++ .../no2/filters/FlattenableFilter.java | 24 +++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/NearFilter.java b/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/NearFilter.java index a8a3c75cd..d8fc2ca75 100644 --- a/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/NearFilter.java +++ b/nitrite-spatial/src/main/java/org/dizitart/no2/spatial/NearFilter.java @@ -73,10 +73,18 @@ public String toString() { @Override public List getFilters() { return List.of( + // [PR note] Use of "IntersectsFilter" was an arbitrary choice. Any of the misbehaving filters that + // are really just doing a bounding box test within the spatial index would have worked. + // The important thing for now was to not accidentally produce *recursive* flattening, and at the + // time I wrote this line, I was still planning to edit WithinFilter to have it also implement + // the FlattenableFilter interface new IntersectsFilter(getField(), getValue()), new NonIndexNearFilter(getField(), getValue())); } + // [PR note] This is probably the first time in years I've used a non-static inner class. I think we + // should prefer to avoid the pattern in the final code, but it saved some boiler-plate here for the + // proof-of-concept. public class NonIndexNearFilter extends FieldBasedFilter { protected NonIndexNearFilter(String field, Geometry circle) { diff --git a/nitrite/src/main/java/org/dizitart/no2/filters/FlattenableFilter.java b/nitrite/src/main/java/org/dizitart/no2/filters/FlattenableFilter.java index 800c5aa8d..a4f2d8618 100644 --- a/nitrite/src/main/java/org/dizitart/no2/filters/FlattenableFilter.java +++ b/nitrite/src/main/java/org/dizitart/no2/filters/FlattenableFilter.java @@ -21,9 +21,29 @@ /** * Represents a filter which can be flattened or otherwise consists of multiple constituent filters. * - * // TODO verify the "@since" version? - * @since 4.4.0 + *

[PR notes] + *

    + *
  • We can't use {@code instanceof SomeClass} to trigger the application of "and"-like flattening, + * because any classes defined in submodules aren't available here in the `nitrite` module at compile-time. + * that interface of course needs to be here in the `nitrite` module, just like IndexOnlyFilter is.
    • + * + *
  • There are plenty of things we could call this. "Andlike" was the first thing I came up with but + * that sounded terrible. Flattenable is at least a "functional" naming, in that it says what it is, but this + * also feels like it's asking for a {@code flatten} method to be added to the interface. But that then implies + * that some of the logic in FindOptimizer would end up distributed across multiple classes. In its current state, + * it's very helpful that all the related logic is right there in one file.
    • + * + *
  • If we want to keep {@code getFilters} as the interface, then names like CompoundFilter and CompositeFilter + * come to mind. However, that would leave FindOptimizer with a strange asymmetry where `and` is just a special + * case of this interface but `or` is still its own thing with separate handling.
    • + *
+ * + * // TODO add a "@since" tag */ public interface FlattenableFilter { + /** + * [PR note] Clearly this is not the ideal contract for this interface, but it allowed existing code to + * be leveraged to get the proof-of-concept working. + */ List getFilters(); }