Merge pull request #40 from nitrite/copilot/fix-false-positives-spatial-filter

Fix spatial filter false positives using FlattenableFilter pattern

Author: anidotnet

packages/nitrite/lib/src/collection/operations/find_optimizer.dart

@@ -17,8 +17,8 @@ class FindOptimizer {
 
   FindPlan _createFilterPlan(
       Iterable<IndexDescriptor> 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<Filter> _flattenAndFilter(AndFilter andFilter) {
+  List<Filter> _flattenFilter(FlattenableFilter flattenableFilter) {
     var flattenedFilters = <Filter>[];
-    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);
       }

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<Filter> getFilters();
+}
+
 /// An abstract class representing a filter for Nitrite database.
 abstract class NitriteFilter extends Filter {
   /// Gets the [NitriteConfig] instance.

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<Filter> 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<Filter> getFilters() => filters;
+
   @override
   String toString() {
     StringBuffer buffer = StringBuffer();

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<dynamic> 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<dynamic> 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<Filter> 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<Filter> 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<Filter> 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) {

packages/nitrite_spatial/lib/src/spatial_index.dart

@@ -37,9 +37,9 @@ class SpatialIndex extends NitriteIndex {
     var boundingBox = _fromGeometry(geometry);
 
     Stream<NitriteId> 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');

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<IntersectsFilter>());
-      expect(findPlan.collectionScanFilter, isA<filter.EqualsFilter>());
+      expect(findPlan.indexScanFilter?.filters.first, isA<IntersectsIndexFilter>());
+      expect(findPlan.collectionScanFilter, isNotNull);  // Now has validation filter too
 
       var result = await cursor.map((doc) => doc['key']).toList();
       expect(result.length, 1);