Skip to content

#6567 add support for writing GeoJSON with Z values - #6602

Open
xfischer wants to merge 1 commit into
npgsql:mainfrom
xfischer:feature/6567-geojson-writez
Open

#6567 add support for writing GeoJSON with Z values#6602
xfischer wants to merge 1 commit into
npgsql:mainfrom
xfischer:feature/6567-geojson-writez

Conversation

@xfischer

@xfischer xfischer commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

As discussed in #6567
Original typed Write methods wrote the Z double when a Position had an altitude, but never set the HasZ flag on the EWKB type header.
So 3D geometries were serialized with a 2D type tag. This change sets type |= EwkbGeometryType.HasZ.
It's a "first coordinate decides" assumption, to avoid going over the entire geojson tree for exhaustive check.

@xfischer
xfischer requested review from roji and vonzshik as code owners June 9, 2026 16:35
@xfischer

xfischer commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

@hiSandog thanks for the review! Both scenarios are now covered, and digging into them turned up a related bug beyond just missing test coverage:

  • Mixed 2D/3D within a MultiPoint/MultiLineString/MultiPolygon: FirstCoordinateHasZ decides the outer EWKB HasZ flag from the first member only, but nothing checked that the other members agreed. If they didn't, the writer emitted variably-sized member data while the reader kept assuming uniform sizing from the outer flag — silently corrupting the stream on read, exactly the failure mode you called out. Fixed by extending the existing "all-or-none" consistency check (already used for positions within one LineString/Polygon) up to these composite types, so a mismatch now throws a clear ArgumentException instead of producing bad EWKB.
  • GeometryCollection: I initially assumed heterogeneous 2D/3D children already round-tripped fine, since the reader re-parses each child's own header. Turns out PostGIS itself rejects it server-side (Dimensions mismatch in lwcollection), confirmed directly against a live instance with plain WKT — independent of anything in this library. So GeometryCollection now gets the same client-side guard, and the new test asserts it throws rather than round-trips.
  • Position with Z but no M: added Roundtrip_Z_without_M, confirming the altitude survives round-tripping on its own without implying/attaching M.

Pushed as a new commit on top of the branch.

@xfischer

Copy link
Copy Markdown
Contributor Author

Rebased to match main new file organisation

Typed Write methods wrote the Z double when a Position had an
altitude, but never set the HasZ flag on the EWKB type header, so 3D
geometries were serialized with a 2D type tag. Set type |=
EwkbGeometryType.HasZ using a "first coordinate decides" assumption to
avoid walking the entire geojson tree for an exhaustive check.

Extend the existing all-or-none Z consistency validation to
MultiPoint/MultiLineString/MultiPolygon/GeometryCollection members,
matching what PostGIS enforces server-side (it rejects mixed 2D/3D
collections with "Dimensions mismatch in lwcollection"). Point,
LineString, and Polygon already guarded intra-geometry consistency;
composite types had no equivalent check across members, which could
corrupt the EWKB since the reader assumes uniform dimensionality based
on the outer flag.

Adds tests covering the new guards and a Z-without-M round-trip case.
@xfischer
xfischer force-pushed the feature/6567-geojson-writez branch from 6b1dcd3 to 42a4f93 Compare July 20, 2026 12:21
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.

1 participant