diff --git a/packages/nitrite/lib/src/collection/operations/find_optimizer.dart b/packages/nitrite/lib/src/collection/operations/find_optimizer.dart index aba91e4..880595c 100644 --- a/packages/nitrite/lib/src/collection/operations/find_optimizer.dart +++ b/packages/nitrite/lib/src/collection/operations/find_optimizer.dart @@ -17,8 +17,8 @@ class FindOptimizer { FindPlan _createFilterPlan( Iterable indexDescriptors, Filter filter) { - if (filter is AndFilter) { - var filters = _flattenAndFilter(filter); + if (filter is FlattenableFilter) { + var filters = _flattenFilter(filter as FlattenableFilter); return _createAndPlan(indexDescriptors, filters); } else if (filter is OrFilter) { return _createOrPlan(indexDescriptors, filter.filters); @@ -28,11 +28,12 @@ class FindOptimizer { } } - List _flattenAndFilter(AndFilter andFilter) { + List _flattenFilter(FlattenableFilter flattenableFilter) { var flattenedFilters = []; - for (var filter in andFilter.filters) { - if (filter is AndFilter) { - flattenedFilters.addAll(_flattenAndFilter(filter)); + for (var filter in flattenableFilter.getFilters()) { + if (filter is FlattenableFilter) { + // Type is narrowed to FlattenableFilter here + flattenedFilters.addAll(_flattenFilter(filter as FlattenableFilter)); } else { flattenedFilters.add(filter); } diff --git a/packages/nitrite/lib/src/filters/filter.dart b/packages/nitrite/lib/src/filters/filter.dart index c81a0e5..936bc88 100644 --- a/packages/nitrite/lib/src/filters/filter.dart +++ b/packages/nitrite/lib/src/filters/filter.dart @@ -175,6 +175,16 @@ abstract class Filter { } } +/// Represents a filter which can be flattened or consists of multiple constituent filters. +/// +/// This interface allows filters to be decomposed into multiple sub-filters during query +/// optimization. For example, spatial filters can be split into an index scan filter +/// (for bounding box checks) and a validation filter (for actual geometry checks). +abstract class FlattenableFilter { + /// Returns the list of constituent filters that make up this filter. + List getFilters(); +} + /// An abstract class representing a filter for Nitrite database. abstract class NitriteFilter extends Filter { /// Gets the [NitriteConfig] instance. diff --git a/packages/nitrite/lib/src/filters/filter_impl.dart b/packages/nitrite/lib/src/filters/filter_impl.dart index ba74123..642c9f2 100644 --- a/packages/nitrite/lib/src/filters/filter_impl.dart +++ b/packages/nitrite/lib/src/filters/filter_impl.dart @@ -70,7 +70,7 @@ class OrFilter extends LogicalFilter { } /// @nodoc -class AndFilter extends LogicalFilter { +class AndFilter extends LogicalFilter implements FlattenableFilter { AndFilter(List filters) : super(filters) { for (int i = 1; i < filters.length; i++) { if (filters[i] is TextFilter) { @@ -91,6 +91,9 @@ class AndFilter extends LogicalFilter { return true; } + @override + List getFilters() => filters; + @override String toString() { StringBuffer buffer = StringBuffer(); diff --git a/packages/nitrite_spatial/lib/src/filter.dart b/packages/nitrite_spatial/lib/src/filter.dart index af25cc6..3b2ae5d 100644 --- a/packages/nitrite_spatial/lib/src/filter.dart +++ b/packages/nitrite_spatial/lib/src/filter.dart @@ -1,7 +1,17 @@ import 'package:dart_jts/dart_jts.dart'; import 'package:nitrite/nitrite.dart'; +import 'package:nitrite_spatial/src/geom_utils.dart'; import 'package:nitrite_spatial/src/indexer.dart'; +/// Copies NitriteFilter context (nitriteConfig, collectionName, objectFilter) from source to target +void _copyFilterContext(NitriteFilter source, NitriteFilter target) { + if (source.nitriteConfig != null) { + target.nitriteConfig = source.nitriteConfig; + target.collectionName = source.collectionName; + target.objectFilter = source.objectFilter; + } +} + /// The abstract base class for all spatial filters in Nitrite. /// /// A spatial filter is used to query Nitrite database for @@ -31,9 +41,61 @@ abstract class SpatialFilter extends IndexOnlyFilter { } } -///@nodoc -class WithinFilter extends SpatialFilter { - WithinFilter(super.field, super.value); +/// A non-index filter that validates actual geometry relationships. +/// This filter is used as the second stage of spatial filtering after +/// the R-Tree index has filtered candidates based on bounding boxes. +class _GeometryValidationFilter extends FieldBasedFilter { + final bool Function(Geometry, Geometry) _validator; + + _GeometryValidationFilter(super.field, super.value, this._validator); + + @override + bool apply(Document doc) { + var fieldValue = doc.get(field); + if (fieldValue == null) { + return false; + } + + Geometry? documentGeometry; + if (fieldValue is Geometry) { + documentGeometry = fieldValue; + } else if (fieldValue is String) { + // Try to parse WKT string + try { + var reader = WKTReader(); + documentGeometry = reader.read(fieldValue); + } catch (e) { + return false; + } + } else if (fieldValue is Document) { + // For entity repositories, geometry is stored as a Document with serialized string + try { + var geometryString = fieldValue['geometry'] as String?; + if (geometryString != null) { + var deserialized = GeometrySerializer.deserialize(geometryString); + if (deserialized != null) { + documentGeometry = deserialized; + } + } + } catch (e) { + return false; + } + } else { + return false; + } + + if (documentGeometry == null) { + return false; + } + + return _validator(documentGeometry, value as Geometry); + } +} + +/// Internal implementation of WithinFilter for index scanning only. +/// Does not implement FlattenableFilter to avoid infinite recursion. +class WithinIndexFilter extends SpatialFilter { + WithinIndexFilter(super.field, super.value); @override Stream applyOnIndex(IndexMap indexMap) { @@ -47,9 +109,10 @@ class WithinFilter extends SpatialFilter { } } -///@nodoc -class IntersectsFilter extends SpatialFilter { - IntersectsFilter(super.field, super.value); +/// Internal implementation of IntersectsFilter for index scanning only. +/// Does not implement FlattenableFilter to avoid infinite recursion. +class IntersectsIndexFilter extends SpatialFilter { + IntersectsIndexFilter(super.field, super.value); @override Stream applyOnIndex(IndexMap indexMap) { @@ -64,17 +127,183 @@ class IntersectsFilter extends SpatialFilter { } ///@nodoc -class NearFilter extends WithinFilter { +class WithinFilter extends NitriteFilter implements FlattenableFilter { + final String field; + final Geometry geometry; + + WithinFilter(this.field, this.geometry); + + @override + bool apply(Document doc) { + // For non-indexed queries, apply the validation filter directly + var validationFilter = _GeometryValidationFilter( + field, + geometry, + (docGeom, filterGeom) => docGeom.within(filterGeom), + ); + _copyFilterContext(this, validationFilter); + return validationFilter.apply(doc); + } + + @override + List getFilters() { + // Return two filters: one for index scan, one for validation + return [ + WithinIndexFilter(field, geometry), + _GeometryValidationFilter( + field, + geometry, + (docGeom, filterGeom) => docGeom.within(filterGeom), + ), + ]; + } + + @override + String toString() { + return '($field within $geometry)'; + } +} + +///@nodoc +class IntersectsFilter extends NitriteFilter implements FlattenableFilter { + final String field; + final Geometry geometry; + + IntersectsFilter(this.field, this.geometry); + + @override + bool apply(Document doc) { + // For non-indexed queries, apply the validation filter directly + var validationFilter = _GeometryValidationFilter( + field, + geometry, + (docGeom, filterGeom) => docGeom.intersects(filterGeom), + ); + _copyFilterContext(this, validationFilter); + return validationFilter.apply(doc); + } + + @override + List getFilters() { + // Return two filters: one for index scan, one for validation + return [ + IntersectsIndexFilter(field, geometry), + _GeometryValidationFilter( + field, + geometry, + (docGeom, filterGeom) => docGeom.intersects(filterGeom), + ), + ]; + } + + @override + String toString() { + return '($field intersects $geometry)'; + } +} + +///@nodoc +class NearFilter extends NitriteFilter implements FlattenableFilter { + final String field; + final Geometry circle; + final Coordinate center; + final double radius; + factory NearFilter(String field, Coordinate center, double radius) { - var geometry = _createCircle(center, radius); - return NearFilter._(field, geometry); + var circle = _createCircle(center, radius); + return NearFilter._(field, circle, center, radius); } factory NearFilter.fromPoint(String field, Point point, double radius) { - return NearFilter._(field, _createCircle(point.getCoordinate(), radius)); + var center = point.getCoordinate(); + var circle = _createCircle(center, radius); + return NearFilter._(field, circle, center!, radius); + } + + NearFilter._(this.field, this.circle, this.center, this.radius); + + @override + bool apply(Document doc) { + // For non-indexed queries, apply the validation filter directly + var validationFilter = _NearValidationFilter(field, center, radius); + _copyFilterContext(this, validationFilter); + return validationFilter.apply(doc); } - NearFilter._(super.field, super.geometry); + @override + List getFilters() { + // Return two filters: one for index scan (using within), one for distance validation + return [ + WithinIndexFilter(field, circle), + _NearValidationFilter(field, center, radius), + ]; + } + + @override + String toString() { + return '($field near $center within $radius)'; + } +} + +/// Validation filter for near queries that checks actual distance. +class _NearValidationFilter extends NitriteFilter { + final String field; + final Coordinate center; + final double radius; + + _NearValidationFilter(this.field, this.center, this.radius); + + @override + bool apply(Document doc) { + var fieldValue = doc.get(field); + if (fieldValue == null) { + return false; + } + + Geometry? documentGeometry; + if (fieldValue is Geometry) { + documentGeometry = fieldValue; + } else if (fieldValue is String) { + try { + var reader = WKTReader(); + documentGeometry = reader.read(fieldValue); + } catch (e) { + return false; + } + } else if (fieldValue is Document) { + // For entity repositories, geometry is stored as a Document with serialized string + try { + var geometryString = fieldValue['geometry'] as String?; + if (geometryString != null) { + var deserialized = GeometrySerializer.deserialize(geometryString); + if (deserialized != null) { + documentGeometry = deserialized; + } + } + } catch (e) { + return false; + } + } else { + return false; + } + + if (documentGeometry == null) { + return false; + } + + // For near queries, check if the geometry is within the distance + // For points, check direct distance. For other geometries, check if they intersect the circle. + if (documentGeometry is Point) { + var coord = documentGeometry.getCoordinate(); + if (coord == null) return false; + var distance = center.distance(coord); + return distance <= radius; + } else { + // For non-point geometries, check if they intersect the circle + var circle = _createCircle(center, radius); + return documentGeometry.intersects(circle); + } + } } Geometry _createCircle(Coordinate? center, double radius) { diff --git a/packages/nitrite_spatial/lib/src/spatial_index.dart b/packages/nitrite_spatial/lib/src/spatial_index.dart index 64dd1c4..8bf9455 100644 --- a/packages/nitrite_spatial/lib/src/spatial_index.dart +++ b/packages/nitrite_spatial/lib/src/spatial_index.dart @@ -37,9 +37,9 @@ class SpatialIndex extends NitriteIndex { var boundingBox = _fromGeometry(geometry); Stream keys; - if (filter is WithinFilter) { + if (filter is WithinIndexFilter) { keys = indexMap.findContainedKeys(boundingBox); - } else if (filter is IntersectsFilter) { + } else if (filter is IntersectsIndexFilter) { keys = indexMap.findIntersectingKeys(boundingBox); } else { throw FilterException('Unsupported spatial filter: $filter'); diff --git a/packages/nitrite_spatial/test/index_test.dart b/packages/nitrite_spatial/test/index_test.dart index 2d88815..026b818 100644 --- a/packages/nitrite_spatial/test/index_test.dart +++ b/packages/nitrite_spatial/test/index_test.dart @@ -1,7 +1,6 @@ import 'package:dart_jts/dart_jts.dart'; import 'package:nitrite/nitrite.dart' as no2; import 'package:nitrite/nitrite.dart' hide where; -import 'package:nitrite/src/filters/filter.dart' as filter; import 'package:nitrite_spatial/nitrite_spatial.dart'; import 'package:nitrite_spatial/src/filter.dart'; import 'package:test/test.dart'; @@ -169,8 +168,8 @@ void main() { var findPlan = await cursor.findPlan; expect(findPlan, isNotNull); expect(findPlan.indexScanFilter?.filters.length, 1); - expect(findPlan.indexScanFilter?.filters.first, isA()); - expect(findPlan.collectionScanFilter, isA()); + expect(findPlan.indexScanFilter?.filters.first, isA()); + expect(findPlan.collectionScanFilter, isNotNull); // Now has validation filter too var result = await cursor.map((doc) => doc['key']).toList(); expect(result.length, 1); diff --git a/packages/nitrite_spatial/test/intersects_false_positive_test.dart b/packages/nitrite_spatial/test/intersects_false_positive_test.dart new file mode 100644 index 0000000..d24e438 --- /dev/null +++ b/packages/nitrite_spatial/test/intersects_false_positive_test.dart @@ -0,0 +1,111 @@ +import 'package:dart_jts/dart_jts.dart'; +import 'package:nitrite/nitrite.dart' hide where; +import 'package:nitrite_spatial/nitrite_spatial.dart'; +import 'package:test/test.dart'; + +import 'base_test_loader.dart'; +import 'test_utils.dart'; + +void main() { + group(retry: 3, 'Spatial Intersects False Positive Test Suite', () { + var reader = WKTReader(); + + setUp(() async { + setUpLog(); + await setUpNitriteTest(); + }); + + tearDown(() async { + await tearDownNitriteTest(); + }); + + test('Test Intersects - Polygon and MultiPoint with overlapping bounding boxes but no intersection', () async { + // This is the test case from the issue report + // The polygon and multipoint have overlapping bounding boxes + // but the geometries themselves do not intersect + var polygon = reader.read('POLYGON ((40486.563 45036.319, 40084.108 44545.927, 39496.171 44938.774, 39889.018 45526.712, 40486.563 45036.319))') as Polygon; + var multipoint = reader.read('MULTIPOINT ((40933.744 45423.275), (40395.332 45612.623), (40574.536 45576.665))') as MultiPoint; + + // Insert the multipoint into the collection + final doc = createDocument("geometry", multipoint); + await collection.insert(doc); + + // Create spatial index + await collection.createIndex(["geometry"], indexOptions(spatialIndex)); + + // Query for geometries that intersect the polygon + final result = await collection + .find(filter: where('geometry').intersects(polygon)) + .toList(); + + // The multipoint does not intersect the polygon, so result should be empty + expect(result.length, 0, reason: 'MultiPoint does not intersect Polygon, should return no results'); + }); + + test('Test Intersects - Polygon and Point inside bounding box but outside geometry', () async { + // Create a non-convex L-shaped polygon to test bbox vs geometry + var lShapedPolygon = reader.read('POLYGON ((0 0, 10 0, 10 5, 5 5, 5 10, 0 10, 0 0))') as Polygon; + var pointInBBoxButOutsidePolygon = reader.read('POINT (7 7)') as Point; // In bbox but outside L-shape + + // Insert the point + final doc = createDocument("geometry", pointInBBoxButOutsidePolygon); + await collection.insert(doc); + + // Create spatial index + await collection.createIndex(["geometry"], indexOptions(spatialIndex)); + + // Query for geometries that intersect the L-shaped polygon + final result = await collection + .find(filter: where('geometry').intersects(lShapedPolygon)) + .toList(); + + // The point is inside the bounding box but outside the polygon + expect(result.length, 0, reason: 'Point is in bounding box but outside polygon geometry'); + }); + + test('Test Intersects - Actual intersection should return results', () async { + // Create geometries that actually intersect + var polygon = reader.read('POLYGON ((0 0, 10 0, 10 10, 0 10, 0 0))') as Polygon; + var pointInside = reader.read('POINT (5 5)') as Point; + var lineIntersecting = reader.read('LINESTRING (-5 5, 15 5)') as LineString; + + // Insert the geometries + await collection.insertMany([ + createDocument("id", 1).put("geometry", pointInside), + createDocument("id", 2).put("geometry", lineIntersecting), + ]); + + // Create spatial index + await collection.createIndex(["geometry"], indexOptions(spatialIndex)); + + // Query for geometries that intersect the polygon + final result = await collection + .find(filter: where('geometry').intersects(polygon)) + .toList(); + + // Both geometries intersect the polygon + expect(result.length, 2, reason: 'Point and LineString both intersect the polygon'); + }); + + test('Test Within - Point in bounding box but outside geometry', () async { + // L-shaped polygon + var lShapedPolygon = reader.read('POLYGON ((0 0, 10 0, 10 5, 5 5, 5 10, 0 10, 0 0))') as Polygon; + var pointInBBoxButOutsidePolygon = reader.read('POINT (7 7)') as Point; + + // Insert the point + final doc = createDocument("geometry", pointInBBoxButOutsidePolygon); + await collection.insert(doc); + + // Create spatial index + await collection.createIndex(["geometry"], indexOptions(spatialIndex)); + + // Query for geometries within the L-shaped polygon + final result = await collection + .find(filter: where('geometry').within(lShapedPolygon)) + .toList(); + + // The point is not within the polygon + expect(result.length, 0, reason: 'Point is in bounding box but not within polygon geometry'); + }); + }); +}