shoemoney opened a new pull request, #3256:
URL: https://github.com/apache/iceberg-rust/pull/3256
## Which issue does this PR close?
- Closes #3229.
## What changes are included in this PR?
`deserialize_decimal` (crates/iceberg/src/spec/datatypes.rs:350-363 on main)
and
`deserialize_fixed` (:376-387 on main) used `trim_start_matches` /
`trim_end_matches`.
Those strip *repeated* occurrences and never require the prefix to be
present, so the
following all parsed successfully before this change:
- `"decimal(5, 2)))))"` -> `Decimal { precision: 5, scale: 2 }`
- `"decimal(decimal(5, 2)))"` -> `Decimal { precision: 5, scale: 2 }`
- `"decimal(5, 2"` (no closing paren) -> `Decimal { precision: 5, scale: 2 }`
- `"fixed[fixed[16]]]"` -> `Fixed(16)`
- `"fixed[16"` -> `Fixed(16)`
Precision was also unvalidated, so `"decimal(50, 2)"` deserialized into a
`PrimitiveType::Decimal` and round-tripped back out, even though
`Type::decimal` and
`Type::decimal_required_bytes` enforce `precision <= MAX_DECIMAL_PRECISION`
(38). This is
reachable from every JSON schema parse (`impl Deserialize for
PrimitiveType`), so a
`Schema` or `TableMetadata` written through iceberg-rust could carry a
decimal type that
iceberg-java and pyiceberg reject; the error only surfaced much later and
far away, in
`arrow::schema` (`validate_decimal_precision_and_scale`) or `avro::schema`.
`scale > precision`
was likewise accepted.
The fix parses strictly with `strip_prefix` / `strip_suffix`, and applies
the invariants the
crate already states elsewhere: `0 < precision <= MAX_DECIMAL_PRECISION` and
`scale <= precision`.
Not fixed deliberately: this PR does not introduce a regex dependency as the
issue suggests,
since `strip_prefix` + `split_once` covers the same grammar with no new
dependency, and it does
not touch the lenient `starts_with("decimal")` dispatch in `impl Deserialize
for PrimitiveType`,
which now simply routes malformed input to a clear error instead of silently
accepting it.
## Are these changes tested?
Yes, two new unit tests in the existing `spec::datatypes` test module.
Before the fix (test only, source pristine):
```
test
spec::datatypes::tests::test_reject_malformed_decimal_and_fixed_type_strings
... FAILED
----
spec::datatypes::tests::test_reject_malformed_decimal_and_fixed_type_strings
stdout ----
panicked at crates/iceberg/src/spec/datatypes.rs:1346:13:
expected "decimal(50, 2)" to be rejected
test result: FAILED. 14 passed; 1 failed; 0 ignored; 0 measured; 1743
filtered out
```
After the fix:
```
$ cargo test -p iceberg --lib spec::
test result: ok. 475 passed; 0 failed; 0 ignored; 0 measured; 1283 filtered
out
$ cargo test -p iceberg --lib
test result: ok. 1758 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
```
`rustfmt` (nightly-2026-04-16, per rust-toolchain.toml) and `cargo clippy -p
iceberg --lib
--all-features` are both clean on the touched file.
## AI Disclosure
Written in conjunction with my pair programmer Claude.
--
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]
