Revanth14 commented on PR #1239:
URL: https://github.com/apache/iceberg-go/pull/1239#issuecomment-4760245858
> Nice hardening — the per-member JSON id errors are a real readability win,
and the projjson-string rejection is good to have.
>
> One thing I want to make sure we've thought through: the new
authority:code regex at arrow_utils.go:1939 is now the only accepted shape for
a string CRS on read, but the write side doesn't enforce it.
icebergCRSToGeoArrowMetadata (arrow_utils.go:2050) serializes any string, and
GeometryTypeOf/GeographyTypeOf (types.go:847, :907) accept any non-empty CRS
with no shape check. So something like geometry("my-custom-crs") writes fine
today but fails to read back after this change — which the old `len(crs) <= 32`
branch used to allow. That makes it a forward-compat break for tables already
written with a free-form CRS string. Is the plan to also tighten the
constructor/write path so those values can't be produced, or should read stay
lenient for already-written data? Right now read and write disagree on what's
valid.
@tanmayrauth thanks for the review.
Good catch. This does create a round-trip break: `geometry("my-custom-crs")`
can be written today, but the new read path rejects it.
I think read should stay lenient for non-empty string CRS values, since
tightening write would not help existing data and would still leave
constructors more permissive than the writer. We can still reject `projjson`
string CRS and WKT2, since those are unsupported.
Will wait for @laskoviymishka thoughts before updating. If that direction
sounds right, I’ll add a round-trip regression test for
`GeometryTypeOf("my-custom-crs")`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
