Skip to content

Conversation

@yutannihilation
Copy link
Contributor

As Proj::transform() already transforms 4D, the implementation is fairly simple.

I guess the hard part is to find proper test cases...

> SELECT ST_Transform(ST_GeomFromWKT('POINT Z (-124 45 100)'), 4979, 4978);
┌─────────────────────────────────────────────────────────────────────────────────────┐
│ st_transform(st_geomfromwkt(Utf8("POINT Z (-124 45 100)")),Int64(4979),Int64(4978)) │
│                                       geometry                                      │
╞═════════════════════════════════════════════════════════════════════════════════════╡
│ POINT Z(-2526244.3011458996 -3745311.197806657 4487419.119544039)                   │
└─────────────────────────────────────────────────────────────────────────────────────┘

@yutannihilation yutannihilation marked this pull request as ready for review January 23, 2026 12:44
Copy link
Member

@paleolimbot paleolimbot left a comment

Choose a reason for hiding this comment

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

Thank you!

This is a great addition as is...I suggested a few test cases but I'm not sure all of my suggestions work and we can do those later. In particular our CrsTransform isn't really set up to handle changes in dimensions. We could implement that change when we implement ST_Force2D/3DZM(), too so that we can implement those functions using CrsTransform.

Also linking #47 here (this PR will close that).

Comment on lines 36 to 41
eng = eng.create_or_skip()
eng.assert_query_result(
"SELECT ST_Transform(ST_GeomFromText('POINT Z (1 1 1)'), 'EPSG:4326', 'EPSG:4978')",
"POINT Z (6376201.805927448 111297.016517882 110568.792276973)",
wkt_precision=9,
)
Copy link
Member

Choose a reason for hiding this comment

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

This particular transformation is a great test case and an interesting one because it is 2D -> 3D. Does 2D input (e.g., POINT (-64 45)) work? I believe you can query the number of axes of a CRS to handle this:

https://github.com/apache/sedona-db/blob/main/c/sedona-proj/src/proj_dyn_bindgen.rs#L127-L128

...although we can do that later because I think 3D->3D is what the current code is set up to handle.

Another good test would be 3D -> 2D (the inverse of this transform would be good) and 3D -> 3D like EPSG:4326+EPSG:5701 (lon/lat with Z in meters) to EPSG:4326+EPSG:8050 (lon/lat with Z in feet). You might have to get pyproj to render those to PROJJSON to get them to work in ST_Transform because our internal deserialize_crs() doesn't know how to parse compound CRSes yet.

@yutannihilation
Copy link
Contributor Author

handle changes in dimensions

Do you mean conversions between 2D CRS and 3D CRS should change the dimension of the geometry? In PostGIS, I don't see such changes.

postgres=# SELECT ST_AsText(ST_Transform(ST_GeomFromText('POINT (1 1 1)'), 'EPSG:4979', 'EPSG:4326'));
    st_astext
-----------------
 POINT Z (1 1 1)
(1 row)

@paleolimbot
Copy link
Member

Do you mean conversions between 2D CRS and 3D CRS should change the dimension of the geometry? In PostGIS, I don't see such changes.

I think that it should although you certainly don't have to implement that here and it is probably worth digging into why PostGIS doesn't do this before deciding that's what should happen. There is significantly less precedent around 3D/4D/spatiotemporal CRS handling and I don't think this is one of the things we should perfectly match PostGIS if we can do a better job. (Or maybe our alternative behaviour should have a function with a different name while we sort out what that behaviour should be).

@yutannihilation
Copy link
Contributor Author

Thanks for clarifying. I'm not immediately sure what's the right behavior, but at least I agree with this part:

There is significantly less precedent around 3D/4D/spatiotemporal CRS handling and I don't think this is one of the things we should perfectly match PostGIS if we can do a better job.

Copy link
Member

@paleolimbot paleolimbot left a comment

Choose a reason for hiding this comment

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

Great! Let's merge this and deal with changes in dimensions elsewhere ( #554 ).

Thank you!

@paleolimbot paleolimbot merged commit 47f7bee into apache:main Jan 26, 2026
15 checks passed
@paleolimbot paleolimbot added this to the 0.3.0 milestone Jan 26, 2026
@yutannihilation yutannihilation deleted the feat/st_transform_3d branch January 26, 2026 22:51
@yutannihilation
Copy link
Contributor Author

Thanks for merging! I'll comment there once I've formed an opinion on this.

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.

2 participants