moomindani commented on PR #9:
URL:
https://github.com/apache/iceberg-verification/pull/9#issuecomment-5702040896
Reviewed from the consumer side: I ran the cases in this PR against
PyIceberg on current `main` to see what the surface would actually catch. Two
things came out of it.
**The geospatial cases work exactly as intended, and they catch a live
defect.** Spelling "unquoted" into the clauses was the right call. PyIceberg's
`GEOMETRY_REGEX` requires the CRS to be quoted, so it rejects
`geometry(srid:4326)` — the Appendix C example — and writes
`"geometry('srid:4326')"` instead. Java's pattern captures the quotes rather
than rejecting them, so it reads that CRS as `'srid:4326'` with the quotes
included. So this fixture would have surfaced a silent bidirectional
divergence, which is the case for the corpus about as well as it can be made. I
have raised it on the PyIceberg side (apache/iceberg-python#3530).
**The decimal whitespace case stops one step short of the interesting
input.** `decimal-9-2-spaced` covers `decimal(9, 2)`, which every
implementation I checked already accepts. The clause the spec actually added in
apache/iceberg#16798 is broader — "optional whitespace around parameters **and
separators**" — and that is where implementations diverge: PyIceberg accepts
`decimal(9, 2)` but rejects `decimal( 9 , 2 )` and `decimal(9 ,2)`, while
Java's `decimal\(\s*(\d+)\s*,\s*(\d+)\s*\)` accepts all of them. The prototype
in sungwy/iceberg-testing had that case, with the note "A reader stricter than
that diverges here"; it is the one input on this surface that separates a
conforming parser from a strict one, so it seems worth carrying over.
On the `canonical` field for decimal, I would leave it as you have it. The
clause here says no byte-exact form is pinned, and while apache/iceberg#16798
intended to pin `decimal(P, S)` — Java's `DecimalType.toString()` emits the
spaced form, and so does PyIceberg — the sentence that merged points at a table
whose template still reads `"decimal(<P>,<S>)"` with no space. That is a
spec-text question rather than something to settle in a fixture, and for read
conformance `accept` is the field that carries the weight anyway.
--
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]
