chore: allow optional whitespace around parameters similar to other language implementations

[sungwy]

Reference: apache/iceberg#16798

Rationale for this change

PyIceberg is a bit more strict in its parsing than the other language implementations. This allows json string representations like decimal( 9, 2 ) like the other libraries.

Are these changes tested?

Unit tested

Are there any user-facing changes?

Yes, more permissive metadata parsing by PyIceberg clients.

[Copilot]

Pull request overview

This PR makes PyIceberg’s decimal type parsing more permissive by allowing optional whitespace around decimal parameters, aligning behavior with other Iceberg language implementations.

Changes:

• Update the decimal type parsing regex to tolerate whitespace around precision/scale and separators.
• Add unit tests covering multiple whitespace variants for decimal(P,S) JSON deserialization.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
tests/test_types.py	Adds parametrized tests to ensure DecimalType deserialization accepts optional whitespace variants.
pyiceberg/types.py	Relaxes the decimal parsing regex to allow whitespace around precision/scale and punctuation.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[geruh]

Should we align this logic to the other regex too?

https://github.com/apache/iceberg/blob/41c8ee43b46017843b304241d250737fd10837af/api/src/main/java/org/apache/iceberg/types/Types.java#L66-L73

  private static final Pattern FIXED = Pattern.compile("fixed\\[\\s*(\\d+)\\s*\\]");
  private static final Pattern GEOMETRY_PARAMETERS =
      Pattern.compile("geometry\\s*(?:\\(\\s*([^)]*?)\\s*\\))?", Pattern.CASE_INSENSITIVE);
  private static final Pattern GEOGRAPHY_PARAMETERS =
      Pattern.compile(
          "geography\\s*(?:\\(\\s*([^,]*?)\\s*(?:,\\s*(\\w*)\\s*)?\\))?", Pattern.CASE_INSENSITIVE);
  private static final Pattern DECIMAL =
      Pattern.compile("decimal\\(\\s*(\\d+)\\s*,\\s*(\\d+)\\s*\\)");

[rambleraptor]

+1

[github-actions]

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

[rambleraptor]

@sungwy can you make the proposed changes and we can hopefully get this merged in?

[github-actions]

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

[github-actions]

This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

[moomindani]

@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.

[geruh]

yup was trying to nudge in this direction...