zeroshade commented on code in PR #1109:
URL: https://github.com/apache/arrow-go/pull/1109#discussion_r3883629762
##########
arrow/scalar/temporal.go:
##########
@@ -40,7 +42,15 @@ func temporalToString(s TemporalScalar) string {
case *Time64:
return time.Unix(0,
int64(s.Value)*int64(s.Unit().Multiplier())).UTC().Format("15:04:05.999999999")
case *Timestamp:
- return time.Unix(0,
int64(s.Value)*int64(s.Unit().Multiplier())).UTC().Format("2006-01-02
15:04:05.999999999")
+ typ := s.DataType().(*arrow.TimestampType)
+ if typ.TimeZone == "" {
+ return time.Unix(0,
int64(s.Value)*int64(s.Unit().Multiplier())).UTC().Format(timestampScalarLayout)
+ }
+ toTime, err := typ.GetToTimeFunc()
+ if err != nil {
+ return "..."
+ }
+ return toTime(s.Value).Format(timestampScalarLayout + "Z0700")
Review Comment:
`Z0700` loses seconds from historical IANA timezone offsets, so this still
does not universally round-trip. For `Europe/Berlin` at year 0000, `String()`
emits `0000-01-01 00:53:28+0053`; parsing it changes the timestamp by 28
seconds. Please preserve second-granularity offsets and add a historical
named-zone regression test.
##########
arrow/compute/expression_test.go:
##########
@@ -37,7 +37,7 @@ type privateFunctionOptions struct {
func (privateFunctionOptions) TypeName() string { return
"privateFunctionOptions" }
func TestExpressionToString(t *testing.T) {
- ts, _ := scalar.MakeScalar("1990-10-23
10:23:33.123456").CastTo(arrow.FixedWidthTypes.Timestamp_ns)
+ ts, _ := scalar.MakeScalar("1990-10-23
10:23:33.123456Z").CastTo(arrow.FixedWidthTypes.Timestamp_ns)
Review Comment:
This fixture now includes a timezone, so the scalar renders with `Z`, but
the expected value below remains timezone-less. Current CI reports expected
`1990-10-23 10:23:33.123456`, actual `1990-10-23 10:23:33.123456Z`. Please
update the expectation so the test suite passes.
--
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]
