zeroshade commented on code in PR #1312:
URL: https://github.com/apache/iceberg-go/pull/1312#discussion_r3566791279
##########
types.go:
##########
@@ -571,17 +570,17 @@ func (m *MapType) UnmarshalJSON(b []byte) error {
return nil
}
-func validateFixedLength(length int) error {
- if length < 0 {
- return fmt.Errorf("fixed length must be non-negative, got %d",
length)
+func validateFixedLength(n int) error {
+ if n <= 0 {
Review Comment:
Non-blocking: tightening this to reject `n <= 0` means `fixed[0]` /
`FixedTypeOf(0)` now error/panic where they were previously accepted. That's
correct per the Iceberg spec (fixed length must be ≥ 1), but it's technically a
behavior change for any existing caller or persisted data using `fixed[0]` —
worth calling out in the PR description / changelog.
##########
types.go:
##########
@@ -200,16 +205,10 @@ func (t *typeIFace) UnmarshalJSON(b []byte) error {
return fmt.Errorf("%w: %s",
ErrInvalidTypeString, typename)
}
- prec, err := strconv.Atoi(matches[1])
- if err != nil {
- return fmt.Errorf("%w: %s",
ErrInvalidTypeString, typename)
- }
- scale, err := strconv.Atoi(matches[2])
- if err != nil {
- return fmt.Errorf("%w: %s",
ErrInvalidTypeString, typename)
- }
- if err := validateDecimalPrecisionScale(prec,
scale); err != nil {
- return fmt.Errorf("%w: %w",
ErrInvalidTypeString, err)
+ prec, _ := strconv.Atoi(matches[1])
Review Comment:
Minor: the decimal branch now discards the `strconv.Atoi` errors (`prec, _
:=` / `scale, _ :=`), relying on `validateDecimalParams` to reject the
resulting 0 on overflow. The `fixed` branch just above still checks its `Atoi`
error and surfaces `value out of range`. Consider handling the error here too
so overflow input like `decimal(99999999999999999999, 0)` gets a consistent
message.
--
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]
