You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ra...@apache.org on 2024/02/20 11:21:22 UTC

(arrow) 10/30: GH-39672: [Go] Time to Date32/Date64 conversion issues for non-UTC timezones (#39674)

This is an automated email from the ASF dual-hosted git repository.

raulcd pushed a commit to branch maint-15.0.x
in repository https://gitbox.apache.org/repos/asf/arrow.git

commit 914e62dca7bb43c44a707b815a98579608d180fa
Author: Matt Topol <zo...@gmail.com>
AuthorDate: Thu Jan 18 15:30:38 2024 -0500

    GH-39672: [Go] Time to Date32/Date64 conversion issues for non-UTC timezones (#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 <zo...@gmail.com>
    Signed-off-by: Matt Topol <zo...@gmail.com>
---
 go/arrow/compute/internal/kernels/cast_temporal.go |  8 ++++++++
 go/arrow/datatype_fixedwidth.go                    | 10 ----------
 go/arrow/datatype_fixedwidth_test.go               | 10 ++++++++++
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/go/arrow/compute/internal/kernels/cast_temporal.go b/go/arrow/compute/internal/kernels/cast_temporal.go
index 542a8a4590..48e2bfb6ca 100644
--- a/go/arrow/compute/internal/kernels/cast_temporal.go
+++ b/go/arrow/compute/internal/kernels/cast_temporal.go
@@ -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)
 }
@@ -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)
 }
diff --git a/go/arrow/datatype_fixedwidth.go b/go/arrow/datatype_fixedwidth.go
index 1a3074e59e..6a7071422f 100644
--- a/go/arrow/datatype_fixedwidth.go
+++ b/go/arrow/datatype_fixedwidth.go
@@ -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()))
 }
 
@@ -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)
diff --git a/go/arrow/datatype_fixedwidth_test.go b/go/arrow/datatype_fixedwidth_test.go
index b3cbb465f3..d6caa21e1a 100644
--- a/go/arrow/datatype_fixedwidth_test.go
+++ b/go/arrow/datatype_fixedwidth_test.go
@@ -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))
+}