mzzz-zzm commented on PR #983:
URL: https://github.com/apache/iceberg-go/pull/983#issuecomment-4388509155
During my integration tests against a real S3-backed Iceberg catalog, I
found two pre-existing bugs in `manifest.go` and `literals.go` that surface
when any table has a decimal partition column. Neither appears to be tracked in
the issue tracker yet, flagging here since they're adjacent to the changes in
this PR.
---
**`convertDecimalValue` passes the wrong argument to `DecimalRequiredBytes`
(`manifest.go`)**
```go
fixedSize := internal.DecimalRequiredBytes(len(dec.String()))
```
`DecimalRequiredBytes` expects the column's declared **precision** (e.g.
`10` for `decimal(10, 2)`), but receives the character length of the printed
value instead. These are unrelated: `"10.00"` has length 5, which happens to
match `DecimalRequiredBytes(10) = 5` by coincidence, but `"1.00"` has length 4
→ `DecimalRequiredBytes(4) = 2` bytes instead of the correct 5. For
`decimal(18, 0)`, the value `"1"` gives 1 byte instead of 8.
The resulting slice length doesn't match the Avro schema's `fixed[N]` field,
and the encoder rejects it:
```
avro: field data_file.partition.<field>: cannot use []uint8 with Avro type
fixed
```
**Repro**: write a manifest with a `decimal(10, 2)` partition column and
value `1.00`. The fix is to thread the column's declared precision from the
partition field's `DecimalType` into `convertDecimalValue`.
---
**`DecimalLiteral.Type()` hardcodes precision 9 (`literals.go`)**
```go
func (d DecimalLiteral) Type() Type { return DecimalTypeOf(9, d.Scale) }
```
`DecimalLiteral` carries `Val` and `Scale` but not the declared precision,
so `Type()` silently returns `decimal(9, scale)` regardless of the actual
column type. Any path that calls `.Type()` on a manifest-read decimal literal —
bounds checks, cast decisions, predicate projection — will operate against the
wrong precision.
---
Both bugs exist in `main` (`v0.5.0`) and are not introduced by this PR.
Happy to open separate issues and/or PRs for them if that's preferred.
--
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]
