zeroshade commented on code in PR #558:
URL: https://github.com/apache/arrow-go/pull/558#discussion_r3561126196


##########
arrow/extensions/timestamp_with_offset.go:
##########
@@ -0,0 +1,592 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package extensions
+
+import (
+       "fmt"
+       "iter"
+       "math"
+       "reflect"
+       "slices"
+       "strings"
+       "time"
+
+       "github.com/apache/arrow-go/v18/arrow"
+       "github.com/apache/arrow-go/v18/arrow/array"
+       "github.com/apache/arrow-go/v18/arrow/memory"
+       "github.com/apache/arrow-go/v18/internal/json"
+)
+
+// TimestampWithOffsetType represents a timestamp column that stores a 
timezone offset per row instead of
+// applying the same timezone offset to the entire column.
+type TimestampWithOffsetType struct {
+       arrow.ExtensionBase
+}
+
+func isOffsetTypeOk(offsetType arrow.DataType) bool {
+       switch offsetType := offsetType.(type) {
+       case *arrow.Int16Type:
+               return true
+       case *arrow.DictionaryType:
+               return arrow.TypeEqual(offsetType.ValueType, 
arrow.PrimitiveTypes.Int16)
+       case *arrow.RunEndEncodedType:
+               return offsetType.ValidRunEndsType(offsetType.RunEnds()) &&
+                       arrow.TypeEqual(offsetType.Encoded(), 
arrow.PrimitiveTypes.Int16)
+               // FIXME: Technically this should be non-nullable, but a Arrow 
IPC does not deserialize
+               // ValueNullable properly, so enforcing this here would always 
fail when reading from an IPC
+               // stream
+               // !offsetType.ValueNullable
+       default:
+               return false
+       }
+}
+
+// Whether the storageType is compatible with TimestampWithOffset.
+//
+// Returns (time_unit, offset_type, ok). If ok is false, time_unit and 
offset_type are garbage.
+func isDataTypeCompatible(storageType arrow.DataType) (unit arrow.TimeUnit, 
offsetType arrow.DataType, ok bool) {
+       unit = arrow.Second
+       offsetType = arrow.PrimitiveTypes.Int16
+       ok = false
+
+       st, compat := storageType.(*arrow.StructType)
+       if !compat || st.NumFields() != 2 {
+               return
+       }
+
+       if ts, compat := st.Field(0).Type.(*arrow.TimestampType); compat && 
ts.TimeZone == "UTC" {
+               unit = ts.TimeUnit()
+       } else {
+               return
+       }
+
+       maybeOffset := st.Field(1)
+       offsetType = maybeOffset.Type
+
+       ok = st.Field(0).Name == "timestamp" &&
+               !st.Field(0).Nullable &&
+               maybeOffset.Name == "offset_minutes" &&
+               isOffsetTypeOk(offsetType) &&
+               !maybeOffset.Nullable
+       return
+}
+
+// NewTimestampWithOffsetType creates a new TimestampWithOffsetType with the 
underlying storage type set correctly to
+// Struct(timestamp=Timestamp(T, "UTC"), offset_minutes=Int16), where T is any 
TimeUnit.
+func NewTimestampWithOffsetType(unit arrow.TimeUnit) *TimestampWithOffsetType {
+       v, _ := NewTimestampWithOffsetTypeCustomOffset(unit, 
arrow.PrimitiveTypes.Int16)
+       // SAFETY: This should never error as Int16 is always a valid offset 
type
+
+       return v
+}
+
+// NewTimestampWithOffsetTypeCustomOffset creates a new 
TimestampWithOffsetType with the underlying storage type set correctly to
+// Struct(timestamp=Timestamp(T, "UTC"), offset_minutes=O), where T is any 
TimeUnit and O is a valid offset type.
+//
+// The error will be populated if the data type is not a valid encoding of the 
offsets field.
+func NewTimestampWithOffsetTypeCustomOffset(unit arrow.TimeUnit, offsetType 
arrow.DataType) (*TimestampWithOffsetType, error) {
+       if !isOffsetTypeOk(offsetType) {
+               return nil, fmt.Errorf("invalid offset type %s", offsetType)
+       }
+
+       return &TimestampWithOffsetType{
+               ExtensionBase: arrow.ExtensionBase{
+                       Storage: arrow.StructOf(
+                               arrow.Field{
+                                       Name: "timestamp",
+                                       Type: &arrow.TimestampType{
+                                               Unit:     unit,
+                                               TimeZone: "UTC",
+                                       },
+                                       Nullable: false,
+                               },
+                               arrow.Field{
+                                       Name:     "offset_minutes",
+                                       Type:     offsetType,
+                                       Nullable: false,
+                               },
+                       ),
+               },
+       }, nil
+}
+
+type DictIndexType interface {
+       *arrow.Int8Type | *arrow.Int16Type | *arrow.Int32Type | 
*arrow.Int64Type |
+               *arrow.Uint8Type | *arrow.Uint16Type | *arrow.Uint32Type | 
*arrow.Uint64Type
+}
+
+type RunEndsType interface {
+       *arrow.Int16Type | *arrow.Int32Type | *arrow.Int64Type
+}
+
+// NewTimestampWithOffsetTypeDictionaryEncoded creates a new 
TimestampWithOffsetType with the underlying storage type set correctly to
+// Struct(timestamp=Timestamp(T, "UTC"), offset_minutes=Dictionary(I, Int16)), 
where T is any TimeUnit and I is a
+// valid Dictionary index type.
+func NewTimestampWithOffsetTypeDictionaryEncoded[I DictIndexType](unit 
arrow.TimeUnit, index I) *TimestampWithOffsetType {
+       offsetType := arrow.DictionaryType{
+               IndexType: arrow.DataType(index),
+               ValueType: arrow.PrimitiveTypes.Int16,
+               Ordered:   false,
+       }
+       v, _ := NewTimestampWithOffsetTypeCustomOffset(unit, &offsetType)
+       // SAFETY: This should never error as DictIndexType is always a valid 
index type
+
+       return v
+}
+
+// NewTimestampWithOffsetTypeRunEndEncoded creates a new 
TimestampWithOffsetType with the underlying storage type set correctly to
+// Struct(timestamp=Timestamp(T, "UTC"), offset_minutes=RunEndEncoded(E, 
Int16)), where T is any TimeUnit and E is a
+// valid run-ends type.
+func NewTimestampWithOffsetTypeRunEndEncoded[E RunEndsType](unit 
arrow.TimeUnit, runEnds E) *TimestampWithOffsetType {
+       offsetType := arrow.RunEndEncodedOf(arrow.DataType(runEnds), 
arrow.PrimitiveTypes.Int16)
+
+       v, _ := NewTimestampWithOffsetTypeCustomOffset(unit, offsetType)
+       // SAFETY: This should never error as RunEndsType always a valid run 
ends type
+
+       return v
+
+}
+
+func (b *TimestampWithOffsetType) ArrayType() reflect.Type {
+       return reflect.TypeOf(TimestampWithOffsetArray{})
+}
+
+func (b *TimestampWithOffsetType) ExtensionName() string { return 
"arrow.timestamp_with_offset" }
+
+func (b *TimestampWithOffsetType) String() string {
+       return fmt.Sprintf("extension<%s>", b.ExtensionName())
+}
+
+func (e *TimestampWithOffsetType) MarshalJSON() ([]byte, error) {
+       return []byte(fmt.Sprintf(`{"name":"%s","metadata":%s}`, 
e.ExtensionName(), e.Serialize())), nil
+}
+
+func (b *TimestampWithOffsetType) Serialize() string { return "" }
+
+func (b *TimestampWithOffsetType) Deserialize(storageType arrow.DataType, data 
string) (arrow.ExtensionType, error) {
+       timeUnit, offsetType, ok := isDataTypeCompatible(storageType)
+       if !ok {
+               return nil, fmt.Errorf("invalid storage type for 
TimestampWithOffsetType: %s", storageType.Name())
+       }
+
+       return NewTimestampWithOffsetTypeCustomOffset(timeUnit, offsetType)
+}
+
+func (b *TimestampWithOffsetType) ExtensionEquals(other arrow.ExtensionType) 
bool {
+       return b.ExtensionName() == other.ExtensionName() &&
+               arrow.TypeEqual(b.StorageType(), other.StorageType())
+}
+
+func (b *TimestampWithOffsetType) OffsetType() arrow.DataType {
+       return b.ExtensionBase.Storage.(*arrow.StructType).Field(1).Type
+}
+
+func (b *TimestampWithOffsetType) TimeUnit() arrow.TimeUnit {
+       return 
b.ExtensionBase.Storage.(*arrow.StructType).Field(0).Type.(*arrow.TimestampType).TimeUnit()
+}
+
+func (b *TimestampWithOffsetType) NewBuilder(mem memory.Allocator) 
array.Builder {
+       v, _ := NewTimestampWithOffsetBuilder(mem, b.TimeUnit(), b.OffsetType())
+       return v
+}
+
+// TimestampWithOffsetArray is a simple array of struct
+type TimestampWithOffsetArray struct {
+       array.ExtensionArrayBase
+}
+
+func (a *TimestampWithOffsetArray) String() string {
+       var o strings.Builder
+       o.WriteString("[")
+       for i := 0; i < a.Len(); i++ {
+               if i > 0 {
+                       o.WriteString(" ")
+               }
+               switch {
+               case a.IsNull(i):
+                       o.WriteString(array.NullValueStr)
+               default:
+                       fmt.Fprintf(&o, "\"%s\"", a.Value(i))
+               }
+       }
+       o.WriteString("]")
+       return o.String()
+}
+
+func timeFromFieldValues(utcTimestamp arrow.Timestamp, offsetMinutes int16, 
unit arrow.TimeUnit) time.Time {
+       hours := offsetMinutes / 60
+       minutes := offsetMinutes % 60
+       if minutes < 0 {
+               minutes = -minutes
+       }
+
+       loc := time.FixedZone(fmt.Sprintf("UTC%+03d:%02d", hours, minutes), 
int(offsetMinutes)*60)
+       return utcTimestamp.ToTime(unit).In(loc)
+}

Review Comment:
   Fixed in e7ef6d8d: the zone-name sign is now derived from the whole offset, 
so a sub-hour negative offset keeps its sign (e.g. -30 minutes now formats as 
"UTC-00:30" instead of "UTC+00:30"). Added 
TestTimestampWithOffsetSubHourNegativeZone as a regression test.



##########
arrow/extensions/timestamp_with_offset.go:
##########
@@ -0,0 +1,592 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package extensions
+
+import (
+       "fmt"
+       "iter"
+       "math"
+       "reflect"
+       "slices"
+       "strings"
+       "time"
+
+       "github.com/apache/arrow-go/v18/arrow"
+       "github.com/apache/arrow-go/v18/arrow/array"
+       "github.com/apache/arrow-go/v18/arrow/memory"
+       "github.com/apache/arrow-go/v18/internal/json"
+)
+
+// TimestampWithOffsetType represents a timestamp column that stores a 
timezone offset per row instead of
+// applying the same timezone offset to the entire column.
+type TimestampWithOffsetType struct {
+       arrow.ExtensionBase
+}
+
+func isOffsetTypeOk(offsetType arrow.DataType) bool {
+       switch offsetType := offsetType.(type) {
+       case *arrow.Int16Type:
+               return true
+       case *arrow.DictionaryType:
+               return arrow.TypeEqual(offsetType.ValueType, 
arrow.PrimitiveTypes.Int16)
+       case *arrow.RunEndEncodedType:
+               return offsetType.ValidRunEndsType(offsetType.RunEnds()) &&
+                       arrow.TypeEqual(offsetType.Encoded(), 
arrow.PrimitiveTypes.Int16)
+               // FIXME: Technically this should be non-nullable, but a Arrow 
IPC does not deserialize
+               // ValueNullable properly, so enforcing this here would always 
fail when reading from an IPC
+               // stream
+               // !offsetType.ValueNullable
+       default:
+               return false
+       }
+}
+
+// Whether the storageType is compatible with TimestampWithOffset.
+//
+// Returns (time_unit, offset_type, ok). If ok is false, time_unit and 
offset_type are garbage.
+func isDataTypeCompatible(storageType arrow.DataType) (unit arrow.TimeUnit, 
offsetType arrow.DataType, ok bool) {
+       unit = arrow.Second
+       offsetType = arrow.PrimitiveTypes.Int16
+       ok = false
+
+       st, compat := storageType.(*arrow.StructType)
+       if !compat || st.NumFields() != 2 {
+               return
+       }
+
+       if ts, compat := st.Field(0).Type.(*arrow.TimestampType); compat && 
ts.TimeZone == "UTC" {
+               unit = ts.TimeUnit()
+       } else {
+               return
+       }
+
+       maybeOffset := st.Field(1)
+       offsetType = maybeOffset.Type
+
+       ok = st.Field(0).Name == "timestamp" &&
+               !st.Field(0).Nullable &&
+               maybeOffset.Name == "offset_minutes" &&
+               isOffsetTypeOk(offsetType) &&
+               !maybeOffset.Nullable
+       return
+}
+
+// NewTimestampWithOffsetType creates a new TimestampWithOffsetType with the 
underlying storage type set correctly to
+// Struct(timestamp=Timestamp(T, "UTC"), offset_minutes=Int16), where T is any 
TimeUnit.
+func NewTimestampWithOffsetType(unit arrow.TimeUnit) *TimestampWithOffsetType {
+       v, _ := NewTimestampWithOffsetTypeCustomOffset(unit, 
arrow.PrimitiveTypes.Int16)
+       // SAFETY: This should never error as Int16 is always a valid offset 
type
+
+       return v
+}
+
+// NewTimestampWithOffsetTypeCustomOffset creates a new 
TimestampWithOffsetType with the underlying storage type set correctly to
+// Struct(timestamp=Timestamp(T, "UTC"), offset_minutes=O), where T is any 
TimeUnit and O is a valid offset type.
+//
+// The error will be populated if the data type is not a valid encoding of the 
offsets field.
+func NewTimestampWithOffsetTypeCustomOffset(unit arrow.TimeUnit, offsetType 
arrow.DataType) (*TimestampWithOffsetType, error) {
+       if !isOffsetTypeOk(offsetType) {
+               return nil, fmt.Errorf("invalid offset type %s", offsetType)
+       }
+
+       return &TimestampWithOffsetType{
+               ExtensionBase: arrow.ExtensionBase{
+                       Storage: arrow.StructOf(
+                               arrow.Field{
+                                       Name: "timestamp",
+                                       Type: &arrow.TimestampType{
+                                               Unit:     unit,
+                                               TimeZone: "UTC",
+                                       },
+                                       Nullable: false,
+                               },
+                               arrow.Field{
+                                       Name:     "offset_minutes",
+                                       Type:     offsetType,
+                                       Nullable: false,
+                               },
+                       ),
+               },
+       }, nil
+}
+
+type DictIndexType interface {
+       *arrow.Int8Type | *arrow.Int16Type | *arrow.Int32Type | 
*arrow.Int64Type |
+               *arrow.Uint8Type | *arrow.Uint16Type | *arrow.Uint32Type | 
*arrow.Uint64Type
+}
+
+type RunEndsType interface {
+       *arrow.Int16Type | *arrow.Int32Type | *arrow.Int64Type
+}
+
+// NewTimestampWithOffsetTypeDictionaryEncoded creates a new 
TimestampWithOffsetType with the underlying storage type set correctly to
+// Struct(timestamp=Timestamp(T, "UTC"), offset_minutes=Dictionary(I, Int16)), 
where T is any TimeUnit and I is a
+// valid Dictionary index type.
+func NewTimestampWithOffsetTypeDictionaryEncoded[I DictIndexType](unit 
arrow.TimeUnit, index I) *TimestampWithOffsetType {
+       offsetType := arrow.DictionaryType{
+               IndexType: arrow.DataType(index),
+               ValueType: arrow.PrimitiveTypes.Int16,
+               Ordered:   false,
+       }
+       v, _ := NewTimestampWithOffsetTypeCustomOffset(unit, &offsetType)
+       // SAFETY: This should never error as DictIndexType is always a valid 
index type
+
+       return v
+}
+
+// NewTimestampWithOffsetTypeRunEndEncoded creates a new 
TimestampWithOffsetType with the underlying storage type set correctly to
+// Struct(timestamp=Timestamp(T, "UTC"), offset_minutes=RunEndEncoded(E, 
Int16)), where T is any TimeUnit and E is a
+// valid run-ends type.
+func NewTimestampWithOffsetTypeRunEndEncoded[E RunEndsType](unit 
arrow.TimeUnit, runEnds E) *TimestampWithOffsetType {
+       offsetType := arrow.RunEndEncodedOf(arrow.DataType(runEnds), 
arrow.PrimitiveTypes.Int16)
+
+       v, _ := NewTimestampWithOffsetTypeCustomOffset(unit, offsetType)
+       // SAFETY: This should never error as RunEndsType always a valid run 
ends type
+
+       return v
+
+}
+
+func (b *TimestampWithOffsetType) ArrayType() reflect.Type {
+       return reflect.TypeOf(TimestampWithOffsetArray{})
+}
+
+func (b *TimestampWithOffsetType) ExtensionName() string { return 
"arrow.timestamp_with_offset" }
+
+func (b *TimestampWithOffsetType) String() string {
+       return fmt.Sprintf("extension<%s>", b.ExtensionName())
+}
+
+func (e *TimestampWithOffsetType) MarshalJSON() ([]byte, error) {
+       return []byte(fmt.Sprintf(`{"name":"%s","metadata":%s}`, 
e.ExtensionName(), e.Serialize())), nil
+}
+
+func (b *TimestampWithOffsetType) Serialize() string { return "" }
+
+func (b *TimestampWithOffsetType) Deserialize(storageType arrow.DataType, data 
string) (arrow.ExtensionType, error) {
+       timeUnit, offsetType, ok := isDataTypeCompatible(storageType)
+       if !ok {
+               return nil, fmt.Errorf("invalid storage type for 
TimestampWithOffsetType: %s", storageType.Name())
+       }
+
+       return NewTimestampWithOffsetTypeCustomOffset(timeUnit, offsetType)
+}
+
+func (b *TimestampWithOffsetType) ExtensionEquals(other arrow.ExtensionType) 
bool {
+       return b.ExtensionName() == other.ExtensionName() &&
+               arrow.TypeEqual(b.StorageType(), other.StorageType())
+}
+
+func (b *TimestampWithOffsetType) OffsetType() arrow.DataType {
+       return b.ExtensionBase.Storage.(*arrow.StructType).Field(1).Type
+}
+
+func (b *TimestampWithOffsetType) TimeUnit() arrow.TimeUnit {
+       return 
b.ExtensionBase.Storage.(*arrow.StructType).Field(0).Type.(*arrow.TimestampType).TimeUnit()
+}
+
+func (b *TimestampWithOffsetType) NewBuilder(mem memory.Allocator) 
array.Builder {
+       v, _ := NewTimestampWithOffsetBuilder(mem, b.TimeUnit(), b.OffsetType())
+       return v
+}
+
+// TimestampWithOffsetArray is a simple array of struct
+type TimestampWithOffsetArray struct {
+       array.ExtensionArrayBase
+}
+
+func (a *TimestampWithOffsetArray) String() string {
+       var o strings.Builder
+       o.WriteString("[")
+       for i := 0; i < a.Len(); i++ {
+               if i > 0 {
+                       o.WriteString(" ")
+               }
+               switch {
+               case a.IsNull(i):
+                       o.WriteString(array.NullValueStr)
+               default:
+                       fmt.Fprintf(&o, "\"%s\"", a.Value(i))
+               }
+       }
+       o.WriteString("]")
+       return o.String()
+}
+
+func timeFromFieldValues(utcTimestamp arrow.Timestamp, offsetMinutes int16, 
unit arrow.TimeUnit) time.Time {
+       hours := offsetMinutes / 60
+       minutes := offsetMinutes % 60
+       if minutes < 0 {
+               minutes = -minutes
+       }
+
+       loc := time.FixedZone(fmt.Sprintf("UTC%+03d:%02d", hours, minutes), 
int(offsetMinutes)*60)
+       return utcTimestamp.ToTime(unit).In(loc)
+}
+
+func fieldValuesFromTime(t time.Time, unit arrow.TimeUnit) (arrow.Timestamp, 
int16) {
+       _, offsetSeconds := t.Zone()
+       offsetMinutes := int16(offsetSeconds / 60)
+       ts, _ := arrow.TimestampFromTime(t.UTC(), unit)
+       return ts, offsetMinutes
+}
+
+// Get the raw arrow values at the given index
+//
+// SAFETY: the value at i must not be nil
+func (a *TimestampWithOffsetArray) rawValueUnsafe(i int) (arrow.Timestamp, 
int16, arrow.TimeUnit) {
+       structs := a.Storage().(*array.Struct)
+
+       timestampField := structs.Field(0)
+       timestamps := timestampField.(*array.Timestamp)
+
+       timeUnit := timestampField.DataType().(*arrow.TimestampType).Unit
+       utcTimestamp := timestamps.Value(i)
+
+       var offsetMinutes int16
+
+       switch offsets := structs.Field(1).(type) {
+       case *array.Int16:
+               offsetMinutes = offsets.Value(i)
+       case *array.Dictionary:
+               offsetMinutes = 
offsets.Dictionary().(*array.Int16).Value(offsets.GetValueIndex(i))
+       case *array.RunEndEncoded:
+               offsetMinutes = 
offsets.Values().(*array.Int16).Value(offsets.GetPhysicalIndex(i))
+       }
+
+       return utcTimestamp, offsetMinutes, timeUnit
+}
+
+func (a *TimestampWithOffsetArray) Value(i int) time.Time {
+       if a.IsNull(i) {
+               return time.Time{}
+       }
+       utcTimestamp, offsetMinutes, timeUnit := a.rawValueUnsafe(i)
+       return timeFromFieldValues(utcTimestamp, offsetMinutes, timeUnit)
+}
+
+// Iterates over the array returning the timestamp at each position.
+//
+// The second parameter indicates whether the timestamp is valid or not.
+//
+// This will iterate using the fastest method given the underlying storage 
array
+func (a *TimestampWithOffsetArray) iterValues() iter.Seq2[time.Time, bool] {
+       return func(yield func(time.Time, bool) bool) {
+               structs := a.Storage().(*array.Struct)
+               offsets := structs.Field(1)
+               if reeOffsets, isRee := offsets.(*array.RunEndEncoded); isRee {
+                       timestampField := structs.Field(0)
+                       timeUnit := 
timestampField.DataType().(*arrow.TimestampType).Unit
+                       timestamps := timestampField.(*array.Timestamp)
+
+                       offsetValues := reeOffsets.Values().(*array.Int16)
+                       // Run-ends are absolute over the unsliced offsets 
child, so a sliced
+                       // array must begin at the physical run covering its 
logical offset and
+                       // advance using absolute positions (logicalOffset + i).
+                       logicalOffset := reeOffsets.Offset()
+                       offsetPhysicalIdx := reeOffsets.GetPhysicalOffset()
+
+                       var getRunEnd (func(int) int)
+                       switch arr := reeOffsets.RunEndsArr().(type) {
+                       case *array.Int16:
+                               getRunEnd = func(idx int) int { return 
int(arr.Value(idx)) }
+                       case *array.Int32:
+                               getRunEnd = func(idx int) int { return 
int(arr.Value(idx)) }
+                       case *array.Int64:
+                               getRunEnd = func(idx int) int { return 
int(arr.Value(idx)) }
+                       }
+
+                       for i := 0; i < a.Len(); i++ {
+                               if logicalOffset+i >= 
getRunEnd(offsetPhysicalIdx) {
+                                       offsetPhysicalIdx += 1
+                               }
+
+                               var ts time.Time
+                               valid := a.IsValid(i)
+                               if valid {
+                                       utcTimestamp := timestamps.Value(i)
+                                       offsetMinutes := 
offsetValues.Value(offsetPhysicalIdx)
+                                       v := timeFromFieldValues(utcTimestamp, 
offsetMinutes, timeUnit)
+                                       ts = v
+                               }
+
+                               if !yield(ts, valid) {
+                                       return
+                               }
+                       }
+               } else {
+                       for i := 0; i < a.Len(); i++ {
+                               var ts time.Time
+                               valid := a.IsValid(i)
+                               if valid {
+                                       utcTimestamp, offsetMinutes, timeUnit 
:= a.rawValueUnsafe(i)
+                                       v := timeFromFieldValues(utcTimestamp, 
offsetMinutes, timeUnit)
+                                       ts = v
+                               }
+
+                               if !yield(ts, valid) {
+                                       return
+                               }
+                       }
+               }
+       }
+}
+
+func (a *TimestampWithOffsetArray) Values() []time.Time {
+       return slices.Collect(func(yield func(time.Time) bool) {
+               for time := range a.iterValues() {
+                       if !yield(time) {
+                               return
+                       }
+               }
+       })
+}
+
+func (a *TimestampWithOffsetArray) ValueStr(i int) string {
+       switch {
+       case a.IsNull(i):
+               return array.NullValueStr
+       default:
+               return a.Value(i).String()
+       }
+}
+
+func (a *TimestampWithOffsetArray) MarshalJSON() ([]byte, error) {
+       values := make([]interface{}, a.Len())
+       i := 0
+       for ts, valid := range a.iterValues() {
+               if !valid {
+                       values[i] = nil
+               } else {
+                       values[i] = &ts
+               }
+               i += 1
+       }
+       return json.Marshal(values)
+}

Review Comment:
   Addressed in e7ef6d8d: MarshalJSON now stores the timestamp by value instead 
of the address of the range variable. (For the record, under this module's Go 
1.25 per-iteration loop semantics it was not actually an aliasing bug — the 
existing exact-JSON roundtrip test asserts distinct per-row values — but 
marshaling by value is clearer.)



##########
arrow/extensions/timestamp_with_offset.go:
##########
@@ -0,0 +1,592 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package extensions
+
+import (
+       "fmt"
+       "iter"
+       "math"
+       "reflect"
+       "slices"
+       "strings"
+       "time"
+
+       "github.com/apache/arrow-go/v18/arrow"
+       "github.com/apache/arrow-go/v18/arrow/array"
+       "github.com/apache/arrow-go/v18/arrow/memory"
+       "github.com/apache/arrow-go/v18/internal/json"
+)
+
+// TimestampWithOffsetType represents a timestamp column that stores a 
timezone offset per row instead of
+// applying the same timezone offset to the entire column.
+type TimestampWithOffsetType struct {
+       arrow.ExtensionBase
+}
+
+func isOffsetTypeOk(offsetType arrow.DataType) bool {
+       switch offsetType := offsetType.(type) {
+       case *arrow.Int16Type:
+               return true
+       case *arrow.DictionaryType:
+               return arrow.TypeEqual(offsetType.ValueType, 
arrow.PrimitiveTypes.Int16)
+       case *arrow.RunEndEncodedType:
+               return offsetType.ValidRunEndsType(offsetType.RunEnds()) &&
+                       arrow.TypeEqual(offsetType.Encoded(), 
arrow.PrimitiveTypes.Int16)
+               // FIXME: Technically this should be non-nullable, but a Arrow 
IPC does not deserialize
+               // ValueNullable properly, so enforcing this here would always 
fail when reading from an IPC
+               // stream
+               // !offsetType.ValueNullable
+       default:
+               return false
+       }
+}
+
+// Whether the storageType is compatible with TimestampWithOffset.
+//
+// Returns (time_unit, offset_type, ok). If ok is false, time_unit and 
offset_type are garbage.
+func isDataTypeCompatible(storageType arrow.DataType) (unit arrow.TimeUnit, 
offsetType arrow.DataType, ok bool) {
+       unit = arrow.Second
+       offsetType = arrow.PrimitiveTypes.Int16
+       ok = false
+
+       st, compat := storageType.(*arrow.StructType)
+       if !compat || st.NumFields() != 2 {
+               return
+       }
+
+       if ts, compat := st.Field(0).Type.(*arrow.TimestampType); compat && 
ts.TimeZone == "UTC" {
+               unit = ts.TimeUnit()
+       } else {
+               return
+       }
+
+       maybeOffset := st.Field(1)
+       offsetType = maybeOffset.Type
+
+       ok = st.Field(0).Name == "timestamp" &&
+               !st.Field(0).Nullable &&
+               maybeOffset.Name == "offset_minutes" &&
+               isOffsetTypeOk(offsetType) &&
+               !maybeOffset.Nullable
+       return
+}
+
+// NewTimestampWithOffsetType creates a new TimestampWithOffsetType with the 
underlying storage type set correctly to
+// Struct(timestamp=Timestamp(T, "UTC"), offset_minutes=Int16), where T is any 
TimeUnit.
+func NewTimestampWithOffsetType(unit arrow.TimeUnit) *TimestampWithOffsetType {
+       v, _ := NewTimestampWithOffsetTypeCustomOffset(unit, 
arrow.PrimitiveTypes.Int16)
+       // SAFETY: This should never error as Int16 is always a valid offset 
type
+
+       return v
+}
+
+// NewTimestampWithOffsetTypeCustomOffset creates a new 
TimestampWithOffsetType with the underlying storage type set correctly to
+// Struct(timestamp=Timestamp(T, "UTC"), offset_minutes=O), where T is any 
TimeUnit and O is a valid offset type.
+//
+// The error will be populated if the data type is not a valid encoding of the 
offsets field.
+func NewTimestampWithOffsetTypeCustomOffset(unit arrow.TimeUnit, offsetType 
arrow.DataType) (*TimestampWithOffsetType, error) {
+       if !isOffsetTypeOk(offsetType) {
+               return nil, fmt.Errorf("invalid offset type %s", offsetType)
+       }
+
+       return &TimestampWithOffsetType{
+               ExtensionBase: arrow.ExtensionBase{
+                       Storage: arrow.StructOf(
+                               arrow.Field{
+                                       Name: "timestamp",
+                                       Type: &arrow.TimestampType{
+                                               Unit:     unit,
+                                               TimeZone: "UTC",
+                                       },
+                                       Nullable: false,
+                               },
+                               arrow.Field{
+                                       Name:     "offset_minutes",
+                                       Type:     offsetType,
+                                       Nullable: false,
+                               },
+                       ),
+               },
+       }, nil
+}
+
+type DictIndexType interface {
+       *arrow.Int8Type | *arrow.Int16Type | *arrow.Int32Type | 
*arrow.Int64Type |
+               *arrow.Uint8Type | *arrow.Uint16Type | *arrow.Uint32Type | 
*arrow.Uint64Type
+}
+
+type RunEndsType interface {
+       *arrow.Int16Type | *arrow.Int32Type | *arrow.Int64Type
+}
+
+// NewTimestampWithOffsetTypeDictionaryEncoded creates a new 
TimestampWithOffsetType with the underlying storage type set correctly to
+// Struct(timestamp=Timestamp(T, "UTC"), offset_minutes=Dictionary(I, Int16)), 
where T is any TimeUnit and I is a
+// valid Dictionary index type.
+func NewTimestampWithOffsetTypeDictionaryEncoded[I DictIndexType](unit 
arrow.TimeUnit, index I) *TimestampWithOffsetType {
+       offsetType := arrow.DictionaryType{
+               IndexType: arrow.DataType(index),
+               ValueType: arrow.PrimitiveTypes.Int16,
+               Ordered:   false,
+       }
+       v, _ := NewTimestampWithOffsetTypeCustomOffset(unit, &offsetType)
+       // SAFETY: This should never error as DictIndexType is always a valid 
index type
+
+       return v
+}
+
+// NewTimestampWithOffsetTypeRunEndEncoded creates a new 
TimestampWithOffsetType with the underlying storage type set correctly to
+// Struct(timestamp=Timestamp(T, "UTC"), offset_minutes=RunEndEncoded(E, 
Int16)), where T is any TimeUnit and E is a
+// valid run-ends type.
+func NewTimestampWithOffsetTypeRunEndEncoded[E RunEndsType](unit 
arrow.TimeUnit, runEnds E) *TimestampWithOffsetType {
+       offsetType := arrow.RunEndEncodedOf(arrow.DataType(runEnds), 
arrow.PrimitiveTypes.Int16)
+
+       v, _ := NewTimestampWithOffsetTypeCustomOffset(unit, offsetType)
+       // SAFETY: This should never error as RunEndsType always a valid run 
ends type
+
+       return v
+
+}
+
+func (b *TimestampWithOffsetType) ArrayType() reflect.Type {
+       return reflect.TypeOf(TimestampWithOffsetArray{})
+}
+
+func (b *TimestampWithOffsetType) ExtensionName() string { return 
"arrow.timestamp_with_offset" }
+
+func (b *TimestampWithOffsetType) String() string {
+       return fmt.Sprintf("extension<%s>", b.ExtensionName())
+}
+
+func (e *TimestampWithOffsetType) MarshalJSON() ([]byte, error) {
+       return []byte(fmt.Sprintf(`{"name":"%s","metadata":%s}`, 
e.ExtensionName(), e.Serialize())), nil
+}
+
+func (b *TimestampWithOffsetType) Serialize() string { return "" }
+
+func (b *TimestampWithOffsetType) Deserialize(storageType arrow.DataType, data 
string) (arrow.ExtensionType, error) {
+       timeUnit, offsetType, ok := isDataTypeCompatible(storageType)
+       if !ok {
+               return nil, fmt.Errorf("invalid storage type for 
TimestampWithOffsetType: %s", storageType.Name())
+       }
+
+       return NewTimestampWithOffsetTypeCustomOffset(timeUnit, offsetType)
+}
+
+func (b *TimestampWithOffsetType) ExtensionEquals(other arrow.ExtensionType) 
bool {
+       return b.ExtensionName() == other.ExtensionName() &&
+               arrow.TypeEqual(b.StorageType(), other.StorageType())
+}
+
+func (b *TimestampWithOffsetType) OffsetType() arrow.DataType {
+       return b.ExtensionBase.Storage.(*arrow.StructType).Field(1).Type
+}
+
+func (b *TimestampWithOffsetType) TimeUnit() arrow.TimeUnit {
+       return 
b.ExtensionBase.Storage.(*arrow.StructType).Field(0).Type.(*arrow.TimestampType).TimeUnit()
+}
+
+func (b *TimestampWithOffsetType) NewBuilder(mem memory.Allocator) 
array.Builder {
+       v, _ := NewTimestampWithOffsetBuilder(mem, b.TimeUnit(), b.OffsetType())
+       return v
+}
+
+// TimestampWithOffsetArray is a simple array of struct
+type TimestampWithOffsetArray struct {
+       array.ExtensionArrayBase
+}
+
+func (a *TimestampWithOffsetArray) String() string {
+       var o strings.Builder
+       o.WriteString("[")
+       for i := 0; i < a.Len(); i++ {
+               if i > 0 {
+                       o.WriteString(" ")
+               }
+               switch {
+               case a.IsNull(i):
+                       o.WriteString(array.NullValueStr)
+               default:
+                       fmt.Fprintf(&o, "\"%s\"", a.Value(i))
+               }
+       }
+       o.WriteString("]")
+       return o.String()
+}
+
+func timeFromFieldValues(utcTimestamp arrow.Timestamp, offsetMinutes int16, 
unit arrow.TimeUnit) time.Time {
+       hours := offsetMinutes / 60
+       minutes := offsetMinutes % 60
+       if minutes < 0 {
+               minutes = -minutes
+       }
+
+       loc := time.FixedZone(fmt.Sprintf("UTC%+03d:%02d", hours, minutes), 
int(offsetMinutes)*60)
+       return utcTimestamp.ToTime(unit).In(loc)
+}
+
+func fieldValuesFromTime(t time.Time, unit arrow.TimeUnit) (arrow.Timestamp, 
int16) {
+       _, offsetSeconds := t.Zone()
+       offsetMinutes := int16(offsetSeconds / 60)
+       ts, _ := arrow.TimestampFromTime(t.UTC(), unit)
+       return ts, offsetMinutes
+}
+
+// Get the raw arrow values at the given index
+//
+// SAFETY: the value at i must not be nil
+func (a *TimestampWithOffsetArray) rawValueUnsafe(i int) (arrow.Timestamp, 
int16, arrow.TimeUnit) {
+       structs := a.Storage().(*array.Struct)
+
+       timestampField := structs.Field(0)
+       timestamps := timestampField.(*array.Timestamp)
+
+       timeUnit := timestampField.DataType().(*arrow.TimestampType).Unit
+       utcTimestamp := timestamps.Value(i)
+
+       var offsetMinutes int16
+
+       switch offsets := structs.Field(1).(type) {
+       case *array.Int16:
+               offsetMinutes = offsets.Value(i)
+       case *array.Dictionary:
+               offsetMinutes = 
offsets.Dictionary().(*array.Int16).Value(offsets.GetValueIndex(i))
+       case *array.RunEndEncoded:
+               offsetMinutes = 
offsets.Values().(*array.Int16).Value(offsets.GetPhysicalIndex(i))
+       }
+
+       return utcTimestamp, offsetMinutes, timeUnit
+}
+
+func (a *TimestampWithOffsetArray) Value(i int) time.Time {
+       if a.IsNull(i) {
+               return time.Time{}
+       }
+       utcTimestamp, offsetMinutes, timeUnit := a.rawValueUnsafe(i)
+       return timeFromFieldValues(utcTimestamp, offsetMinutes, timeUnit)
+}
+
+// Iterates over the array returning the timestamp at each position.
+//
+// The second parameter indicates whether the timestamp is valid or not.
+//
+// This will iterate using the fastest method given the underlying storage 
array
+func (a *TimestampWithOffsetArray) iterValues() iter.Seq2[time.Time, bool] {
+       return func(yield func(time.Time, bool) bool) {
+               structs := a.Storage().(*array.Struct)
+               offsets := structs.Field(1)
+               if reeOffsets, isRee := offsets.(*array.RunEndEncoded); isRee {
+                       timestampField := structs.Field(0)
+                       timeUnit := 
timestampField.DataType().(*arrow.TimestampType).Unit
+                       timestamps := timestampField.(*array.Timestamp)
+
+                       offsetValues := reeOffsets.Values().(*array.Int16)
+                       // Run-ends are absolute over the unsliced offsets 
child, so a sliced
+                       // array must begin at the physical run covering its 
logical offset and
+                       // advance using absolute positions (logicalOffset + i).
+                       logicalOffset := reeOffsets.Offset()
+                       offsetPhysicalIdx := reeOffsets.GetPhysicalOffset()
+
+                       var getRunEnd (func(int) int)
+                       switch arr := reeOffsets.RunEndsArr().(type) {
+                       case *array.Int16:
+                               getRunEnd = func(idx int) int { return 
int(arr.Value(idx)) }
+                       case *array.Int32:
+                               getRunEnd = func(idx int) int { return 
int(arr.Value(idx)) }
+                       case *array.Int64:
+                               getRunEnd = func(idx int) int { return 
int(arr.Value(idx)) }
+                       }
+
+                       for i := 0; i < a.Len(); i++ {
+                               if logicalOffset+i >= 
getRunEnd(offsetPhysicalIdx) {
+                                       offsetPhysicalIdx += 1
+                               }
+
+                               var ts time.Time
+                               valid := a.IsValid(i)
+                               if valid {
+                                       utcTimestamp := timestamps.Value(i)
+                                       offsetMinutes := 
offsetValues.Value(offsetPhysicalIdx)
+                                       v := timeFromFieldValues(utcTimestamp, 
offsetMinutes, timeUnit)
+                                       ts = v
+                               }
+
+                               if !yield(ts, valid) {
+                                       return
+                               }
+                       }
+               } else {
+                       for i := 0; i < a.Len(); i++ {
+                               var ts time.Time
+                               valid := a.IsValid(i)
+                               if valid {
+                                       utcTimestamp, offsetMinutes, timeUnit 
:= a.rawValueUnsafe(i)
+                                       v := timeFromFieldValues(utcTimestamp, 
offsetMinutes, timeUnit)
+                                       ts = v
+                               }
+
+                               if !yield(ts, valid) {
+                                       return
+                               }
+                       }
+               }
+       }
+}
+
+func (a *TimestampWithOffsetArray) Values() []time.Time {
+       return slices.Collect(func(yield func(time.Time) bool) {
+               for time := range a.iterValues() {
+                       if !yield(time) {
+                               return
+                       }
+               }
+       })
+}

Review Comment:
   Fixed in e7ef6d8d: renamed the loop variable to `t` so it no longer shadows 
the `time` package.



-- 
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]

Reply via email to