zeroshade commented on code in PR #952:
URL: https://github.com/apache/arrow-go/pull/952#discussion_r3589006427
##########
arrow/array/timestamp.go:
##########
@@ -349,7 +349,10 @@ func (b *TimestampBuilder) UnmarshalOne(dec *json.Decoder)
error {
b.Append(arrow.Timestamp(i))
break
}
- loc, _ := b.dtype.GetZone()
+ loc, err := b.dtype.GetZone()
Review Comment:
This validates the tz only on the non-integer **string** path. Numeric JSON
still bypasses it — the `case json.Number:` and `case float64:` branches below
just append without calling `GetZone()`. So a builder whose type has an invalid
`TimeZone` still accepts numeric JSON like `[0]` without error. If this
boundary is meant to reject invalid timezones, validate once up front
regardless of the value representation.
##########
arrow/ipc/metadata.go:
##########
@@ -986,7 +986,11 @@ func timeFromFB(data flatbuf.Time) (arrow.DataType, error)
{
func timestampFromFB(data flatbuf.Timestamp) (arrow.DataType, error) {
unit := unitFromFB(data.Unit())
tz := string(data.Timezone())
- return &arrow.TimestampType{Unit: unit, TimeZone: tz}, nil
+ typ := &arrow.TimestampType{Unit: unit, TimeZone: tz}
+ if _, err := typ.GetZone(); err != nil {
Review Comment:
`GetZone()` resolves IANA names via `time.LoadLocation`, which needs the
system/embedded tz database. Gating IPC schema *decode* on it means a valid
schema with `tz=America/New_York` is **rejected** on a
minimal/scratch/container runtime that lacks tzdata (and doesn't import the
embedded `time/tzdata`) — a regression, since decode previously carried the tz
string through untouched. Empty tz and fixed offsets like `+09:00` are
unaffected (they skip `LoadLocation`). Consider not failing decode on IANA
resolution here, or documenting that IPC reads now require tzdata.
--
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]
