Skip to content

TINKERPOP-2558: Adding Geometry value type - #3754

Open
GumpacG wants to merge 2 commits into
apache:masterfrom
GumpacG:geospatial-type
Open

GumpacG wants to merge 2 commits into
apache:masterfrom
GumpacG:geospatial-type

Conversation

@GumpacG

@GumpacG GumpacG commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

https://issues.apache.org/jira/browse/TINKERPOP-2558

First implementation pull request for the geospatial proposal (proposal-geospatial-10.asciidoc, #3743). It adds Geometry as a value type in gremlin-core.

Changes

  • structure.geo: the seven SQL/MM kinds under abstract Curve, Surface, MultiCurve and MultiSurface
    levels, and Model (CARTESIAN, GEOGRAPHIC, OTHER). Values are immutable.
  • WKT, EWKT and ISO WKB reading and writing. Malformed input is an IllegalArgumentException.
  • Exact equality, with -0.0 equal to 0.0.
  • A total order in GremlinValueComparator, between UUID and Vertex, consistent with equals.
  • GType entries for GEOMETRY, each kind, and CARTESIAN and GEOGRAPHIC. CoreImports entries.
  • gremlin-semantics.asciidoc: the type, its equality and order, and the Hilbert key. One changelog line.

Testing

  • New unit tests: GeometryTest, WktTest, WkbTest, GeometryOrderTest and HilbertKeyTest, plus new rows in
    OrderTest, CompareTest and PTest.

First implementation pull request for the geospatial proposal (`proposal-geospatial-10.asciidoc`). It adds `Geometry` as a value type in `gremlin-core`.

Changes:
- `structure.geo`: the seven SQL/MM kinds under abstract `Curve`, `Surface`, `MultiCurve` and `MultiSurface` levels, and `Model` (`CARTESIAN`, `GEOGRAPHIC`, `OTHER`). Values are immutable.
- WKT, EWKT and ISO WKB reading and writing. Malformed input is an `IllegalArgumentException`.
- Exact equality, with `-0.0` equal to `0.0`.
- A total order in `GremlinValueComparator`, between `UUID` and `Vertex`, consistent with `equals`.
- `GType` entries for `GEOMETRY`, each kind, and `CARTESIAN` and `GEOGRAPHIC`. `CoreImports` entries.
- `gremlin-semantics.asciidoc`: the type, its equality and order, and the Hilbert key.

Assisted-by: Kiro:claude-opus-5.5
@codecov-commenter

codecov-commenter commented Oct 31, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.89591% with 109 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.83%. Comparing base (a28cd1f) to head (1100d7c).
⚠️ Report is 977 commits behind head on master.

Files with missing lines Patch % Lines
...che/tinkerpop/gremlin/structure/geo/WkbReader.java 82.98% 13 Missing and 20 partials ⚠️
...che/tinkerpop/gremlin/structure/geo/WktReader.java 93.89% 3 Missing and 16 partials ⚠️
.../tinkerpop/gremlin/structure/geo/MultiPolygon.java 81.13% 4 Missing and 6 partials ⚠️
...nkerpop/gremlin/structure/geo/MultiLineString.java 83.01% 3 Missing and 6 partials ⚠️
...pache/tinkerpop/gremlin/structure/geo/Polygon.java 91.30% 1 Missing and 5 partials ⚠️
...che/tinkerpop/gremlin/structure/geo/WkbWriter.java 90.16% 6 Missing ⚠️
...che/tinkerpop/gremlin/structure/geo/WktWriter.java 93.40% 3 Missing and 3 partials ⚠️
...rpop/gremlin/structure/geo/GeometryCollection.java 91.07% 1 Missing and 4 partials ⚠️
...he/tinkerpop/gremlin/structure/geo/MultiPoint.java 90.56% 1 Missing and 4 partials ⚠️
...he/tinkerpop/gremlin/structure/geo/LineString.java 92.30% 2 Missing and 2 partials ⚠️
... and 4 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3754      +/-   ##
============================================
+ Coverage     76.35%   77.83%   +1.47%     
- Complexity    13424    15526    +2102     
============================================
  Files          1012     1071      +59     
  Lines         60341    68350    +8009     
  Branches       7075     8266    +1191     
============================================
+ Hits          46076    53198    +7122     
- Misses        11548    11878     +330     
- Partials       2717     3274     +557     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

* The key is a 32-bit value. Compare it as unsigned: keys above 2^31 are negative as an int.
* The grid and curve are specified in gremlin-semantics.asciidoc.
*/
final class HilbertKey {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some implementations in this PR like this class that should be available in the third party library we use to implement the predicates and steps. I'm assuming this is just a temporary class to ensure that the tests work?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JTS will be added in the PR that implements the predicates. Only encode() will be removed, the rest will remain as it implements the grid specs that we need.

Assisted-by: Kiro:claude-opus-5.5
DURATION(Duration.class),
EDGE(Edge.class),
FLOAT(Float.class),
GEOGRAPHIC(Geometry.class, Model.GEOGRAPHIC),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CARTESIAN and GEOGRAPHIC exist in both Model and GType. CoreImports statically imports every GType value, so in the console a bare GEOGRAPHIC means GType.GEOGRAPHIC. The proposal's examples use a bare GEOGRAPHIC to mean the model. How is this meant to be resolved in the console?


// A further ordinate without a Z tag is malformed
if (startsNumber(peek())) {
throw new IllegalArgumentException("Coordinate has 3 ordinates but no Z tag");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SRID=4326;POINT(1 2 3) fails with "3 ordinates but no Z tag". The proposal says EWKT support exists so PostGIS output can be pasted in, but PostGIS writes 3D EWKT without a Z tag. Is this intentional?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants