Skip to content

Conversation

umartin
Copy link
Contributor

@umartin umartin commented Aug 27, 2025

Description

Don't encode crs in GeoJSON. The crs attribute is no longer allowed in GeoJSON: https://datatracker.ietf.org/doc/html/rfc7946#section-4. It is also pointless in trino since geometries are stripped of srid. It will always be 0 (unknown).

> select to_geojson_geometry(to_spherical_geography(st_geometryFromText('POINT (1 1)')));
> {"type":"Point","coordinates":[1,1],"crs":{"type":"name","properties":{"name":"EPSG:0"}}}

Additional context and related issues

Split from #26451

Release notes

I don't think a release note is required. Compliant GeoJSON consumers should ignore the crs attribute. For completeness a note could be added.

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

# General
* Don't encode crs in GeoJSON. ({issue}`26499`)

@wendigo
Copy link
Contributor

wendigo commented Aug 27, 2025

Please shorten commit message. You can just move the second sentence into next line.

@umartin umartin force-pushed the remove_crs_from_geojson branch from acad5c9 to 8ab5c10 Compare August 27, 2025 13:42
@umartin
Copy link
Contributor Author

umartin commented Aug 27, 2025

Please shorten commit message. You can just move the second sentence into next line.

Done

@umartin umartin closed this Aug 27, 2025
@umartin
Copy link
Contributor Author

umartin commented Aug 27, 2025

Sorry, I accidentally closed the PR. Opening again

@umartin umartin reopened this Aug 27, 2025
@ebyhr
Copy link
Member

ebyhr commented Aug 27, 2025

Please remove the following period from the commit title. https://trino.io/development/process.html#pull-request-and-commit-guidelines-

@ebyhr ebyhr changed the title Don't encode crs in GeoJSON. The crs attribute is no longer allowed i… Don't encode crs in GeoJSON Aug 27, 2025
@umartin umartin force-pushed the remove_crs_from_geojson branch from 8ab5c10 to cf87e8c Compare August 27, 2025 14:37
@umartin umartin force-pushed the remove_crs_from_geojson branch from cf87e8c to 01c8b2c Compare August 28, 2025 07:25
@wendigo wendigo force-pushed the remove_crs_from_geojson branch from 01c8b2c to 20f4d8a Compare September 2, 2025 09:08
@wendigo
Copy link
Contributor

wendigo commented Sep 2, 2025

Applied small nits and going to merge soon

@umartin
Copy link
Contributor Author

umartin commented Sep 2, 2025

Applied small nits and going to merge soon

Great, thanks!

@wendigo wendigo merged commit bc52393 into trinodb:master Sep 2, 2025
95 checks passed
@github-actions github-actions bot added this to the 477 milestone Sep 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants