Skip to content

fix: SRID extraction returns NULL for a valid authority-less CRS - #1108

Open
james-willis wants to merge 1 commit into
apache:mainfrom
james-willis:jw/srid-none-authorityless
Open

fix: SRID extraction returns NULL for a valid authority-less CRS#1108
james-willis wants to merge 1 commit into
apache:mainfrom
james-willis:jw/srid-none-authorityless

Conversation

@james-willis

Copy link
Copy Markdown
Contributor

A valid CRS with no top-level authority code (e.g. a custom polar-stereographic WKT from CF/rioxarray) is fully usable but has no EPSG/SRID. RS_SRID/ST_SRID on such a CRS now return NULL instead of erroring; a genuinely malformed CRS string still errors, and an authority-coded CRS still returns its integer SRID.

…y-less CRS

A valid CRS with no top-level authority code (e.g. a custom polar-stereographic
WKT from CF/rioxarray) is fully usable but has no EPSG/SRID. CachedCrsToSRIDMapping::
get_srid now returns Result<Option<u32>>, yielding None for such a CRS instead of
erroring; RS_SRID and ST_SRID surface that None as NULL. Only a genuinely malformed
CRS string still errors (propagated from deserialize_crs). Authority-coded CRSes
continue to return their integer SRID.
@james-willis
james-willis marked this pull request as ready for review August 3, 2026 18:51

@paleolimbot paleolimbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No notes on the implementation, but do we want this? We could also update the error to say "Use ST_Crs() instead of ST_SRID() to support CRSes without authority/code or with a non-EPSG authority"?

@github-actions
github-actions Bot requested a review from zhangfengcdt August 3, 2026 19:24
@james-willis

Copy link
Copy Markdown
Contributor Author

I think this is the right thing. 2 reasons:

  1. I believe null for a SRID is the correct value where the CRS does not have a SRID.
  2. Users cannot catch a per-row error so we should be extremely cautious about letting a single row fail a whole query.

@paleolimbot

Copy link
Copy Markdown
Member

I believe null for a SRID is the correct value where the CRS does not have a SRID.

It's true that there's no other way to check if an item has a CRS that is identifiable (we could add

Users cannot catch a per-row error so we should be extremely cautious about letting a single row fail a whole query.

Yes, but they may get bogus results. For example SELECT ST_Intersects(some_col, ST_SetSRID(other_col, ST_SRID(other_col)) FROM foofy will silently omit rows where some_col has an unidentifiable result instead of erroring and refusing to return a possibly meaningless result.

We could also put in the documentation of ST_SRID() to use ST_Crs() and ST_SetCrs() instead.

@james-willis

Copy link
Copy Markdown
Contributor Author

I think the round trip example is contrived and I'm not worried about it.

Maybe ST_SetSRID or ST_Intersects should be the one that rejects null CRS/SRID instead.

I'm imagining users who want to filter on the presence of a CRS with a SRID, so they can get rid of SRID-less rows (CRSes with no SRID)

@paleolimbot

Copy link
Copy Markdown
Member

I think the round trip example is contrived and I'm not worried about it.

How about this one: SELECT ST_Intersects(some_col, ST_Transform(other_col, ST_SRID(some_col)) FROM foofy. I think it's pretty reasonable somebody might want to align the sides of an ST_Intersects call (particularly when dealing with item-level CRSes).

Maybe ST_SetSRID or ST_Intersects should be the one that rejects null CRS/SRID instead.

Maybe, although there is precedent for our current behaviour (Sedona and PostGIS accept and propagate nulls with SQL semantics).

I'm imagining users who want to filter on the presence of a CRS with a SRID, so they can get rid of SRID-less rows (CRSes with no SRID)

On the day we're asked for that feature we can always add a function (maybe ST_TrySRID(), or TRY(ST_SRID()) if we support Try before that, or more directly ST_HasSRID()). I am not sure we'll be asked for that feature but I can see wanting to find problematic rows when dealing with systems that can't register CRS <-> SRID mappings. If we print out the start of the offending CRS in the error it could be filtered by value.

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