Re: [PR] chore: allow optional whitespace around parameters similar to other language implementations [iceberg-python]

["moomindani (via GitHub)" <gi...@apache.org>]

moomindani commented on PR #3530:
URL: https://github.com/apache/iceberg-python/pull/3530#issuecomment-5702025304

   @sungwy this was closed by the stale bot on 29 August rather than on merit — @rambleraptor had asked on 21 July to make the review changes so it could be merged. Would you reopen it? I dug into @geruh's request to align the other regexes and it turns out to be more than tidying, so I think this PR is worth more than it looked.
   
   On current `main` (`1312c550`), against Java's four patterns:
   
   | input | pyiceberg | Java |
   |---|---|---|
   | `decimal( 9 , 2 )`, `decimal(9 ,2)` | rejected | accepted |
   | `fixed[ 16 ]` | rejected | accepted |
   | `geometry(srid:4326)` | **rejected** | accepted |
   | `geography(srid:4326, spherical)` | **rejected** | accepted |
   | `decimal(9,2)foo`, `fixed[16]foo` | **accepted** | — |
   
   The geospatial rows are the ones I did not expect. `GEOMETRY_REGEX` and `GEOGRAPHY_REGEX` require the CRS to be quoted, so PyIceberg rejects the unquoted form that Appendix C gives as its example, and writes `"geometry('srid:4326')"` where the spec asks for `"geometry(srid:4326)"`. Reading Java's `geometry\s*(?:\(\s*([^)]*?)\s*\))?`, `[^)]*?` captures the quotes too, so Java does not reject our output — it takes the CRS as `'srid:4326'`, quotes included. So parameterized geospatial types are broken in both directions, and in Java's direction silently.
   
   A detail that makes this concrete: the geospatial fixtures in apache/iceberg-verification#9 already spell "unquoted" into their clauses (`the canonical serialized form is unquoted "geometry(<C>)"`), so the first time PyIceberg runs that surface these cases go red. Fixing it here means the fixture lands green rather than as a known failure.
   
   Happy to help either way — reopen and I can push the remaining pieces to your branch, or if you would rather not carry it, I can open a successor PR with your commit preserved and credited. Your call; I would rather not step on it.
   


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org