Proposed work break-down for GeoSpatial enhancements #1126
Description
I asked ChatGPT to read through our discussion on PR #1086 and break it down into tasks because it had gotten too big for me to digest. It was far from perfect but surprisingly helpful and I appreciate that it "thinks" to mention specific unit tests to include as well as where documentation updates are needed.
I have edited what it produced, and this is the result. I have marked points I'd particularly like confirmation on with '❔'
1 Introduce Geo primitives
Title: Add GeoPoint, GeoLineString, GeoPolygon, … plus lat/long helpers
Why: Give us types that are explicitly geodetic and avoid the x/y confusion. These will back all remaining work.
Scope / Acceptance
-
New classes that extend/compose JTS Geometry but expose latitude / longitude.
-
Codec/serialiser implementations for MVStore & RocksDB.
- ❔ I have not explored or understood the persistence layers at all. Is ChatGPT right that these will be needed separately? And if so, is it just a copy-paste of what exists for current Geometry classes but with a substitution of x/y for long/lat?
-
Unit tests proving round‑trip and equals/hashCode.
❔ : It seems like it should be OK to start with only GeoPoint and grow the rest later as needed.
2 Split “geo‑spatial” from “planar spatial” filters
Title: Create GeoNearFilter, GeoWithinFilter, GeoIntersectsFilter
Why: Users working with game‑world or CAD coordinates shouldn’t pay the geodesic cost, and the semantics differ.
Scope
-
Copy/rename existing spatial filters, adjusting docs & validation to demand Geo* types.
-
Registration in the query DSL and Nitrite’s Filters helper class.
-
Tests showing existing planar filters still work unchanged.
❔ Decision needed: Do we actually need anything other than GeoNearFilter? For the others, if there are arbitrary polygons, they've already been giving the right answers when treated as cartesian coordinates and should continue to do so.
3 Define authoritative semantics for Near
Title: Document and test Near‑filter behaviour for non‑point geometries
Why: Current usage of “near a polygon” is ambiguous (contains vs. intersects). We should nail that down.
Scope
-
Spec section / Javadoc that states: “Near = circle intersects geometry”.
-
Add unit tests for Point, LineString, Polygon cases at edge distances.
4 Two‑pass spatial query execution
Title: Implement bounding‑box fast path + geometry refinement in SpatialIndex.findNitriteIds
Why: Solves the “bbox superset” accuracy problem without duplicating geometry in the index.
Scope
-
First stage: current R‑tree bbox search.
-
Second stage: fetch geometry field for candidate ids, filter with JTS intersects/within.
-
Benchmarks comparing before/after latency.
5 Support multiple spatial filters in an AND
Title: Fix SpatialIndexTest.testAndSpatialQuery and general AND‑chaining
Why: Current engine can only honour one spatial filter; others degrade to collection scan.
Scope
-
Extend SpatialIndex.findNitriteIds (or helper) to iterate over all spatial filters, intersecting their candidate id sets.
-
Test demonstrating two Within filters on different fields.
❔ I think this can just be a completely separate feature request, as it doesn't directly impact
6 Align spatial index scanning with standard index plan
⚠ NOTE: I reread this one a couple of times and I'm not really sure what ChatGPT means, but I'm going to leave it here for now and re-read it with fresh eyes in the morning. 🙃
_Title: Redesign SpatialIndex.findNitriteIds to return { ids, plan }
Why: Brings spatial scanning in line with SingleFieldIndex & CompoundIndex so the optimiser can compose streams uniformly. pr-1086-graphql-resppr-1086-graphql-respScope
Introduce lightweight result record (IdSetAndPlan) so ReadOperations needn’t mutate FindPlan.
Refactor callers; keep behaviour identical for non‑spatial indexes.
➡ Open question: happy with a tiny new record, or would you rather touch ReadOperations now to accept multiple index streams?_