Skip to content

Commit

Permalink
GH-39672: [Go] Time to Date32/Date64 conversion issues for non-UTC ti…
Browse files Browse the repository at this point in the history
…mezones (#39674)

A failing unit test in release verification led to discovering an issue with timestamp to date conversions for non-utc timezones. 

Upon some investigation I was able to determine that it was the conflation of casting conversion behavior (normalize to cast a Timestamp to a Date) vs flat conversion. I've fixed this conflation of concerns and the version of the methods which are exported properly converts non-UTC timezones to dates without affecting Casting behavior.

### Are these changes tested?
yes

### Are there any user-facing changes?
The methods `Date32FromTime` and `Date64FromTime` will properly handle timezones now.

* Closes: #39672

Authored-by: Matt Topol <zotthewizard@gmail.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
  • Loading branch information
zeroshade authored Jan 18, 2024
1 parent 858574d commit 55afcf0
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 10 deletions.
8 changes: 8 additions & 0 deletions go/arrow/compute/internal/kernels/cast_temporal.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ func TimestampToDate32(ctx *exec.KernelCtx, batch *exec.ExecSpan, out *exec.Exec

return ScalarUnaryNotNull(func(_ *exec.KernelCtx, arg0 arrow.Timestamp, _ *error) arrow.Date32 {
tm := fnToTime(arg0)
if _, offset := tm.Zone(); offset != 0 {
// normalize the tm
tm = tm.Add(time.Duration(offset) * time.Second).UTC()
}
return arrow.Date32FromTime(tm)
})(ctx, batch, out)
}
Expand All @@ -125,6 +129,10 @@ func TimestampToDate64(ctx *exec.KernelCtx, batch *exec.ExecSpan, out *exec.Exec

return ScalarUnaryNotNull(func(_ *exec.KernelCtx, arg0 arrow.Timestamp, _ *error) arrow.Date64 {
tm := fnToTime(arg0)
if _, offset := tm.Zone(); offset != 0 {
// normalize the tm
tm = tm.Add(time.Duration(offset) * time.Second).UTC()
}
return arrow.Date64FromTime(tm)
})(ctx, batch, out)
}
Expand Down
10 changes: 0 additions & 10 deletions go/arrow/datatype_fixedwidth.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,6 @@ type (

// Date32FromTime returns a Date32 value from a time object
func Date32FromTime(t time.Time) Date32 {
if _, offset := t.Zone(); offset != 0 {
// properly account for timezone adjustments before we calculate
// the number of days by adjusting the time and converting to UTC
t = t.Add(time.Duration(offset) * time.Second).UTC()
}
return Date32(t.Truncate(24*time.Hour).Unix() / int64((time.Hour * 24).Seconds()))
}

Expand All @@ -88,11 +83,6 @@ func (d Date32) FormattedString() string {

// Date64FromTime returns a Date64 value from a time object
func Date64FromTime(t time.Time) Date64 {
if _, offset := t.Zone(); offset != 0 {
// properly account for timezone adjustments before we calculate
// the actual value by adjusting the time and converting to UTC
t = t.Add(time.Duration(offset) * time.Second).UTC()
}
// truncate to the start of the day to get the correct value
t = t.Truncate(24 * time.Hour)
return Date64(t.Unix()*1e3 + int64(t.Nanosecond())/1e6)
Expand Down
10 changes: 10 additions & 0 deletions go/arrow/datatype_fixedwidth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,3 +428,13 @@ func TestMonthIntervalType(t *testing.T) {
t.Fatalf("invalid type stringer: got=%q, want=%q", got, want)
}
}

func TestDateFromTime(t *testing.T) {
loc, _ := time.LoadLocation("Asia/Hong_Kong")
tm := time.Date(2024, time.January, 18, 3, 0, 0, 0, loc)

wantD32 := time.Date(2024, time.January, 17, 0, 0, 0, 0, time.UTC).Truncate(24*time.Hour).Unix() / int64((time.Hour * 24).Seconds())
wantD64 := time.Date(2024, time.January, 17, 0, 0, 0, 0, time.UTC).UnixMilli()
assert.EqualValues(t, wantD64, arrow.Date64FromTime(tm))
assert.EqualValues(t, wantD32, arrow.Date32FromTime(tm))
}

0 comments on commit 55afcf0

Please sign in to comment.