From 572293b6027daa1c4278dcafd0175203cab2a8b5 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" <3815206+peco-engineer-bot[bot]@users.noreply.github.com> Date: Wed, 21 Oct 2026 04:05:07 +0000 Subject: [PATCH 1/5] TIME (time64[ns]) type unhandled (#505) Signed-off-by: peco-engineer-bot[bot] <3815206+peco-engineer-bot[bot]@users.noreply.github.com> --- driver_e2e_test.go | 39 ++++++ internal/rows/arrowbased/arrowRows.go | 6 + internal/rows/arrowbased/columnValues.go | 66 +++++++++++ internal/rows/arrowbased/time_test.go | 145 +++++++++++++++++++++++ 4 files changed, 256 insertions(+) create mode 100644 internal/rows/arrowbased/time_test.go diff --git a/driver_e2e_test.go b/driver_e2e_test.go index 4bdaeb84..7a057c94 100644 --- a/driver_e2e_test.go +++ b/driver_e2e_test.go @@ -709,3 +709,42 @@ func TestE2ECloudFetchExactRowCount(t *testing.T) { require.Equal(t, int64(wantRows), rowCount, "CloudFetch must surface exactly the requested rows, with no Arrow padding") } + +// TestE2ETimeTypeScan is the reproducer for databricks/databricks-sql-go#505: +// a SQL TIME column arrives over the Thrift Arrow path as a time64[ns] array, +// which the arrow row scanner had no arm for, so the whole result failed with +// "arrow row scanner unhandled type time64[ns]" before any row was returned. +// A TIME must scan as a wall-clock time-of-day (no date, no location) carrying +// the correct hour/minute/second/fractional component. +func TestE2ETimeTypeScan(t *testing.T) { + host, httpPath, token := pecoTestingCreds(t) + + connector, err := NewConnector( + WithServerHostname(host), + WithPort(443), + WithHTTPPath(httpPath), + WithAccessToken(token), + ) + require.NoError(t, err) + + db := sql.OpenDB(connector) + defer db.Close() //nolint:errcheck + + ctx := context.Background() + + rows, err := db.QueryContext(ctx, "SELECT TIME '09:15:00.123456'") + require.NoError(t, err) + defer rows.Close() //nolint:errcheck + + require.True(t, rows.Next(), "expected one row") + + var dest time.Time + require.NoError(t, rows.Scan(&dest), "TIME column must scan without the unhandled-type error") + + assert.Equal(t, 9, dest.Hour()) + assert.Equal(t, 15, dest.Minute()) + assert.Equal(t, 0, dest.Second()) + assert.Equal(t, 123456000, dest.Nanosecond()) + + require.NoError(t, rows.Err()) +} diff --git a/internal/rows/arrowbased/arrowRows.go b/internal/rows/arrowbased/arrowRows.go index 58970d85..e666f9b7 100644 --- a/internal/rows/arrowbased/arrowRows.go +++ b/internal/rows/arrowbased/arrowRows.go @@ -651,6 +651,12 @@ func (vcm *arrowValueContainerMaker) makeColumnValueContainer(t arrow.DataType, case *arrow.TimestampType: return ×tampValueContainer{location: location, toTimestampFn: toTimestampFn}, nil + case *arrow.Time32Type: + return &timeValueContainer{location: location, nsPerUnit: timeUnitToNanos(t.Unit)}, nil + + case *arrow.Time64Type: + return &timeValueContainer{location: location, nsPerUnit: timeUnitToNanos(t.Unit)}, nil + case *arrow.BinaryType: return &columnValuesTyped[*array.Binary, []byte]{}, nil diff --git a/internal/rows/arrowbased/columnValues.go b/internal/rows/arrowbased/columnValues.go index 73aeb01f..659fd1e5 100644 --- a/internal/rows/arrowbased/columnValues.go +++ b/internal/rows/arrowbased/columnValues.go @@ -473,6 +473,72 @@ func (dvc *dateValueContainer) SetValueArray(colData arrow.ArrayData) error { return nil } +// timeUnitToNanos converts an Arrow time unit to its length in nanoseconds. +func timeUnitToNanos(unit arrow.TimeUnit) int64 { + switch unit { + case arrow.Second: + return int64(time.Second) + case arrow.Millisecond: + return int64(time.Millisecond) + case arrow.Microsecond: + return int64(time.Microsecond) + default: // arrow.Nanosecond + return 1 + } +} + +// timeValueContainer scans an Arrow time32/time64 column (a SQL TIME value, +// i.e. a time-of-day with no calendar date and no timezone). The Arrow value +// counts time units since midnight; we render it as a wall-clock time.Time +// anchored on the Unix epoch date in UTC so Hour/Minute/Second/Nanosecond read +// back correctly without introducing a spurious date or location shift. +type timeValueContainer struct { + timeArray arrow.Array + nsPerUnit int64 + location *time.Location +} + +var _ columnValues = (*timeValueContainer)(nil) + +func (tvc *timeValueContainer) Value(i int) (any, error) { + var unitsSinceMidnight int64 + switch a := tvc.timeArray.(type) { + case *array.Time32: + unitsSinceMidnight = int64(a.Value(i)) + case *array.Time64: + unitsSinceMidnight = int64(a.Value(i)) + default: + return nil, errors.New(errArrowRowsUnhandledArrowType(tvc.timeArray.DataType().String())) + } + + ns := unitsSinceMidnight * tvc.nsPerUnit + val := time.Date(1970, time.January, 1, 0, 0, 0, 0, time.UTC). + Add(time.Duration(ns)).In(tvc.location) + return val, nil +} + +func (tvc *timeValueContainer) IsNull(i int) bool { + return tvc.timeArray.IsNull(i) +} + +func (tvc *timeValueContainer) Release() { + if tvc.timeArray != nil { + tvc.timeArray.Release() + } +} + +func (tvc *timeValueContainer) SetValueArray(colData arrow.ArrayData) error { + switch colData.DataType().(type) { + case *arrow.Time32Type: + tvc.timeArray = array.NewTime32Data(colData) + case *arrow.Time64Type: + tvc.timeArray = array.NewTime64Data(colData) + default: + return errors.New(errArrowRowsUnhandledArrowType(colData.DataType().String())) + } + return nil +} + type timestampValueContainer struct { timeArray *array.Timestamp location *time.Location diff --git a/internal/rows/arrowbased/time_test.go b/internal/rows/arrowbased/time_test.go new file mode 100644 index 00000000..f1a59a08 --- /dev/null +++ b/internal/rows/arrowbased/time_test.go @@ -0,0 +1,145 @@ +package arrowbased + +import ( + "testing" + "time" + + "github.com/apache/arrow/go/v12/arrow" + "github.com/apache/arrow/go/v12/arrow/array" + "github.com/apache/arrow/go/v12/arrow/memory" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// buildTime64Array builds a single-element arrow Time64 array in the given unit +// holding the provided raw value (value since midnight in that unit). +func buildTime64Array(t *testing.T, unit arrow.TimeUnit, v arrow.Time64) arrow.ArrayData { + t.Helper() + dt := &arrow.Time64Type{Unit: unit} + b := array.NewTime64Builder(memory.DefaultAllocator, dt) + defer b.Release() + b.Append(v) + arr := b.NewArray() + defer arr.Release() + arr.Data().Retain() + return arr.Data() +} + +// buildTime32Array builds a single-element arrow Time32 array in the given unit. +func buildTime32Array(t *testing.T, unit arrow.TimeUnit, v arrow.Time32) arrow.ArrayData { + t.Helper() + dt := &arrow.Time32Type{Unit: unit} + b := array.NewTime32Builder(memory.DefaultAllocator, dt) + defer b.Release() + b.Append(v) + arr := b.NewArray() + defer arr.Release() + arr.Data().Retain() + return arr.Data() +} + +// TestTime64ValueContainer guards the time64 -> time-of-day conversion for both +// nanosecond and microsecond Arrow units. A TIME '09:15:00.123456' arrives as a +// time64 value counting units since midnight; the scanned value must be a +// wall-clock time-of-day (Hour/Minute/Second/Nanosecond) in UTC, anchored on the +// epoch date (no meaningful calendar date, no location shift). +func TestTime64ValueContainer(t *testing.T) { + vcm := &arrowValueContainerMaker{} + + cases := []struct { + name string + unit arrow.TimeUnit + raw arrow.Time64 + wantNs int + }{ + { + name: "nanosecond", + unit: arrow.Nanosecond, + // 09:15:00.123456 -> (9*3600+15*60)*1e9 + 123456000 ns since midnight + raw: arrow.Time64((9*3600+15*60)*int64(time.Second) + 123456000), + wantNs: 123456000, + }, + { + name: "microsecond", + unit: arrow.Microsecond, + // 09:15:00.123456 -> (9*3600+15*60)*1e6 + 123456 us since midnight + raw: arrow.Time64((9*3600+15*60)*int64(time.Second/time.Microsecond) + 123456), + wantNs: 123456000, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dt := &arrow.Time64Type{Unit: tc.unit} + holder, err := vcm.makeColumnValueContainer(dt, time.UTC, nil, nil) + require.NoError(t, err) + + data := buildTime64Array(t, tc.unit, tc.raw) + defer data.Release() + require.NoError(t, holder.SetValueArray(data)) + + v, err := holder.Value(0) + require.NoError(t, err) + + tv, ok := v.(time.Time) + require.True(t, ok, "expected time.Time, got %T", v) + + assert.Equal(t, 9, tv.Hour()) + assert.Equal(t, 15, tv.Minute()) + assert.Equal(t, 0, tv.Second()) + assert.Equal(t, tc.wantNs, tv.Nanosecond()) + assert.Equal(t, time.UTC, tv.Location()) + }) + } +} + +// TestTime32ValueContainer is the defensive edge: a TIME carried as Arrow time32 +// (second / millisecond units) must also scan as a wall-clock time-of-day rather +// than hitting the unhandled-type error. +func TestTime32ValueContainer(t *testing.T) { + vcm := &arrowValueContainerMaker{} + + cases := []struct { + name string + unit arrow.TimeUnit + raw arrow.Time32 + wantNs int + }{ + { + name: "second", + unit: arrow.Second, + raw: arrow.Time32(9*3600 + 15*60), + wantNs: 0, + }, + { + name: "millisecond", + unit: arrow.Millisecond, + raw: arrow.Time32((9*3600+15*60)*1000 + 123), + wantNs: 123000000, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dt := &arrow.Time32Type{Unit: tc.unit} + holder, err := vcm.makeColumnValueContainer(dt, time.UTC, nil, nil) + require.NoError(t, err) + + data := buildTime32Array(t, tc.unit, tc.raw) + defer data.Release() + require.NoError(t, holder.SetValueArray(data)) + + v, err := holder.Value(0) + require.NoError(t, err) + + tv, ok := v.(time.Time) + require.True(t, ok, "expected time.Time, got %T", v) + + assert.Equal(t, 9, tv.Hour()) + assert.Equal(t, 15, tv.Minute()) + assert.Equal(t, 0, tv.Second()) + assert.Equal(t, tc.wantNs, tv.Nanosecond()) + assert.Equal(t, time.UTC, tv.Location()) + }) + } +} From f8923be2b5f9757db80d8969fe4aa1e37511e962 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Wed, 21 Oct 2026 04:16:36 +0000 Subject: [PATCH 2/5] ai: apply changes for #508 (1 review thread) Addresses: - #4332939800 at internal/rows/arrowbased/columnValues.go:531 Signed-off-by: peco-engineer-bot[bot] --- internal/rows/arrowbased/arrowRows.go | 4 +-- internal/rows/arrowbased/columnValues.go | 7 +++-- internal/rows/arrowbased/time_test.go | 34 ++++++++++++++++++++++-- 3 files changed, 39 insertions(+), 6 deletions(-) diff --git a/internal/rows/arrowbased/arrowRows.go b/internal/rows/arrowbased/arrowRows.go index e666f9b7..50f9f402 100644 --- a/internal/rows/arrowbased/arrowRows.go +++ b/internal/rows/arrowbased/arrowRows.go @@ -652,10 +652,10 @@ func (vcm *arrowValueContainerMaker) makeColumnValueContainer(t arrow.DataType, return ×tampValueContainer{location: location, toTimestampFn: toTimestampFn}, nil case *arrow.Time32Type: - return &timeValueContainer{location: location, nsPerUnit: timeUnitToNanos(t.Unit)}, nil + return &timeValueContainer{nsPerUnit: timeUnitToNanos(t.Unit)}, nil case *arrow.Time64Type: - return &timeValueContainer{location: location, nsPerUnit: timeUnitToNanos(t.Unit)}, nil + return &timeValueContainer{nsPerUnit: timeUnitToNanos(t.Unit)}, nil case *arrow.BinaryType: return &columnValuesTyped[*array.Binary, []byte]{}, nil diff --git a/internal/rows/arrowbased/columnValues.go b/internal/rows/arrowbased/columnValues.go index 659fd1e5..92a20faf 100644 --- a/internal/rows/arrowbased/columnValues.go +++ b/internal/rows/arrowbased/columnValues.go @@ -495,7 +495,6 @@ func timeUnitToNanos(unit arrow.TimeUnit) int64 { type timeValueContainer struct { timeArray arrow.Array nsPerUnit int64 - location *time.Location } var _ columnValues = (*timeValueContainer)(nil) @@ -512,8 +511,12 @@ func (tvc *timeValueContainer) Value(i int) (any, error) { } ns := unitsSinceMidnight * tvc.nsPerUnit + // A SQL TIME is a zoneless wall-clock time-of-day, so we keep the value in + // UTC and do NOT apply cfg.Location: converting into a non-UTC location + // would shift the hour/minute (and could roll the anchor date off the + // epoch), which is wrong for a value that carries no timezone. val := time.Date(1970, time.January, 1, 0, 0, 0, 0, time.UTC). - Add(time.Duration(ns)).In(tvc.location) + Add(time.Duration(ns)) return val, nil } diff --git a/internal/rows/arrowbased/time_test.go b/internal/rows/arrowbased/time_test.go index f1a59a08..88a85500 100644 --- a/internal/rows/arrowbased/time_test.go +++ b/internal/rows/arrowbased/time_test.go @@ -46,10 +46,16 @@ func buildTime32Array(t *testing.T, unit arrow.TimeUnit, v arrow.Time32) arrow.A func TestTime64ValueContainer(t *testing.T) { vcm := &arrowValueContainerMaker{} + // nonUTC is a fixed positive-offset location used to prove that a zoneless + // SQL TIME is never shifted by cfg.Location (a positive offset would also + // roll the epoch anchor date backwards if the value were converted). + nonUTC := time.FixedZone("UTC+5", 5*3600) + cases := []struct { name string unit arrow.TimeUnit raw arrow.Time64 + loc *time.Location wantNs int }{ { @@ -57,6 +63,7 @@ func TestTime64ValueContainer(t *testing.T) { unit: arrow.Nanosecond, // 09:15:00.123456 -> (9*3600+15*60)*1e9 + 123456000 ns since midnight raw: arrow.Time64((9*3600+15*60)*int64(time.Second) + 123456000), + loc: time.UTC, wantNs: 123456000, }, { @@ -64,6 +71,16 @@ func TestTime64ValueContainer(t *testing.T) { unit: arrow.Microsecond, // 09:15:00.123456 -> (9*3600+15*60)*1e6 + 123456 us since midnight raw: arrow.Time64((9*3600+15*60)*int64(time.Second/time.Microsecond) + 123456), + loc: time.UTC, + wantNs: 123456000, + }, + { + // A non-UTC cfg.Location must NOT shift a zoneless TIME: the hour + // stays 09 and the value stays on the UTC epoch date. + name: "nanosecond non-UTC location", + unit: arrow.Nanosecond, + raw: arrow.Time64((9*3600+15*60)*int64(time.Second) + 123456000), + loc: nonUTC, wantNs: 123456000, }, } @@ -71,7 +88,7 @@ func TestTime64ValueContainer(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { dt := &arrow.Time64Type{Unit: tc.unit} - holder, err := vcm.makeColumnValueContainer(dt, time.UTC, nil, nil) + holder, err := vcm.makeColumnValueContainer(dt, tc.loc, nil, nil) require.NoError(t, err) data := buildTime64Array(t, tc.unit, tc.raw) @@ -99,30 +116,43 @@ func TestTime64ValueContainer(t *testing.T) { func TestTime32ValueContainer(t *testing.T) { vcm := &arrowValueContainerMaker{} + nonUTC := time.FixedZone("UTC+5", 5*3600) + cases := []struct { name string unit arrow.TimeUnit raw arrow.Time32 + loc *time.Location wantNs int }{ { name: "second", unit: arrow.Second, raw: arrow.Time32(9*3600 + 15*60), + loc: time.UTC, wantNs: 0, }, { name: "millisecond", unit: arrow.Millisecond, raw: arrow.Time32((9*3600+15*60)*1000 + 123), + loc: time.UTC, wantNs: 123000000, }, + { + // A non-UTC cfg.Location must NOT shift a zoneless TIME. + name: "second non-UTC location", + unit: arrow.Second, + raw: arrow.Time32(9*3600 + 15*60), + loc: nonUTC, + wantNs: 0, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { dt := &arrow.Time32Type{Unit: tc.unit} - holder, err := vcm.makeColumnValueContainer(dt, time.UTC, nil, nil) + holder, err := vcm.makeColumnValueContainer(dt, tc.loc, nil, nil) require.NoError(t, err) data := buildTime32Array(t, tc.unit, tc.raw) From 0943916e21cd4b8a427d8295efe9cd46fbe8b8bf Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Wed, 21 Oct 2026 04:26:14 +0000 Subject: [PATCH 3/5] ai: apply changes for #508 (1 review thread) Addresses: - #4333004011 at internal/rows/arrowbased/arrowRows.go:654 Signed-off-by: peco-engineer-bot[bot] --- internal/arrowscan/arrowscan.go | 32 +++++++ internal/arrowscan/arrowscan_test.go | 86 ++++++++++++++++++- .../rows/arrowbased/arrowscan_parity_test.go | 10 +++ 3 files changed, 124 insertions(+), 4 deletions(-) diff --git a/internal/arrowscan/arrowscan.go b/internal/arrowscan/arrowscan.go index c6705955..aceb3801 100644 --- a/internal/arrowscan/arrowscan.go +++ b/internal/arrowscan/arrowscan.go @@ -214,6 +214,17 @@ func scanCell(col arrow.Array, row int, loc *time.Location, keys *StructKeyCache // round-trips. The Thrift default path never hits this — it receives TIMESTAMP // as a preformatted server string — so this divergence is kernel-only. return inLocation(timestampToTime(int64(c.Value(row)), dt.Unit), loc), nil + case *array.Time32: + // SQL TIME (time-of-day, no calendar date, no timezone). Match the Thrift + // path's timeValueContainer: render as a wall-clock time.Time anchored on + // the Unix epoch in UTC and do NOT apply loc — a zoneless TIME must not be + // shifted by cfg.Location (that would move the hour and could roll the + // anchor date off the epoch). + dt := col.DataType().(*arrow.Time32Type) + return timeOfDayToTime(int64(c.Value(row)), dt.Unit), nil + case *array.Time64: + dt := col.DataType().(*arrow.Time64Type) + return timeOfDayToTime(int64(c.Value(row)), dt.Unit), nil case *array.Decimal128: dt := col.DataType().(*arrow.Decimal128Type) if options.DecimalAsFloat { @@ -380,6 +391,27 @@ func timestampToTime(v int64, unit arrow.TimeUnit) time.Time { } } +// timeOfDayToTime renders an arrow time32/time64 value (units since midnight) as +// a wall-clock time.Time anchored on the Unix epoch date in UTC, matching the +// Thrift path's timeValueContainer. The value is zoneless, so it is intentionally +// left in UTC (no loc applied). The multiply cannot overflow: a valid TIME is +// bounded by 24h, so even in nanoseconds units*nsPerUnit stays well within int64. +func timeOfDayToTime(unitsSinceMidnight int64, unit arrow.TimeUnit) time.Time { + var nsPerUnit int64 + switch unit { + case arrow.Second: + nsPerUnit = int64(time.Second) + case arrow.Millisecond: + nsPerUnit = int64(time.Millisecond) + case arrow.Microsecond: + nsPerUnit = int64(time.Microsecond) + default: // arrow.Nanosecond + nsPerUnit = 1 + } + return time.Date(1970, time.January, 1, 0, 0, 0, 0, time.UTC). + Add(time.Duration(unitsSinceMidnight * nsPerUnit)) +} + // inLocation renders t in loc, matching the Thrift path's .In(location); a nil // loc leaves the value in UTC (arrow's ToTime default). func inLocation(t time.Time, loc *time.Location) time.Time { diff --git a/internal/arrowscan/arrowscan_test.go b/internal/arrowscan/arrowscan_test.go index e68745eb..a60adfb2 100644 --- a/internal/arrowscan/arrowscan_test.go +++ b/internal/arrowscan/arrowscan_test.go @@ -104,11 +104,12 @@ func TestScanCellScalars(t *testing.T) { t.Run("unsupported_type_errors", func(t *testing.T) { // An unhandled arrow type must error, not return a silently wrong value. - // Duration/MonthInterval are now handled (see TestScanCellInterval); use a - // type with no scan arm. - b := array.NewTime32Builder(pool, &arrow.Time32Type{Unit: arrow.Second}) + // Duration/MonthInterval are now handled (see TestScanCellInterval) and + // Time32/Time64 are now handled (see TestScanCellTimeOfDay); use a type with + // no scan arm. + b := array.NewMonthDayNanoIntervalBuilder(pool) defer b.Release() - b.Append(1000) + b.Append(arrow.MonthDayNanoInterval{Months: 1, Days: 2, Nanoseconds: 3}) arr := b.NewArray() defer arr.Release() if _, err := ScanCell(arr, 0, nil); err == nil { @@ -225,6 +226,83 @@ func TestScanCellInterval(t *testing.T) { } } +// SQL TIME (arrow time32/time64) arrives as units-since-midnight and must render +// to the same epoch-anchored UTC wall-clock time.Time the Thrift path produces +// (timeValueContainer). It is zoneless, so loc must NOT shift the value. +func TestScanCellTimeOfDay(t *testing.T) { + pool := memory.NewGoAllocator() + // 13:01:01.5 as the wall-clock time-of-day both paths must return. + want := time.Date(1970, time.January, 1, 13, 1, 1, 500_000_000, time.UTC) + + t.Run("time32_seconds", func(t *testing.T) { + b := array.NewTime32Builder(pool, &arrow.Time32Type{Unit: arrow.Second}) + defer b.Release() + b.Append(arrow.Time32(13*3600 + 1*60 + 1)) // 13:01:01 + arr := b.NewArray() + defer arr.Release() + v, err := ScanCell(arr, 0, nil) + if err != nil { + t.Fatal(err) + } + got := v.(time.Time) + wantSec := time.Date(1970, time.January, 1, 13, 1, 1, 0, time.UTC) + if !got.Equal(wantSec) { + t.Errorf("got %v, want %v", got, wantSec) + } + }) + + t.Run("time32_millis", func(t *testing.T) { + b := array.NewTime32Builder(pool, &arrow.Time32Type{Unit: arrow.Millisecond}) + defer b.Release() + b.Append(arrow.Time32((13*3600+1*60+1)*1000 + 500)) // 13:01:01.500 + arr := b.NewArray() + defer arr.Release() + v, err := ScanCell(arr, 0, nil) + if err != nil { + t.Fatal(err) + } + if got := v.(time.Time); !got.Equal(want) { + t.Errorf("got %v, want %v", got, want) + } + }) + + t.Run("time64_micros", func(t *testing.T) { + b := array.NewTime64Builder(pool, &arrow.Time64Type{Unit: arrow.Microsecond}) + defer b.Release() + b.Append(arrow.Time64((13*3600+1*60+1)*1_000_000 + 500_000)) // 13:01:01.500000 + arr := b.NewArray() + defer arr.Release() + v, err := ScanCell(arr, 0, nil) + if err != nil { + t.Fatal(err) + } + if got := v.(time.Time); !got.Equal(want) { + t.Errorf("got %v, want %v", got, want) + } + }) + + t.Run("time64_nanos_zoneless_ignores_loc", func(t *testing.T) { + loc, err := time.LoadLocation("America/New_York") + if err != nil { + t.Skipf("tz database unavailable: %v", err) + } + b := array.NewTime64Builder(pool, &arrow.Time64Type{Unit: arrow.Nanosecond}) + defer b.Release() + b.Append(arrow.Time64((13*3600+1*60+1)*int64(time.Second) + 500_000_000)) + arr := b.NewArray() + defer arr.Release() + // A zoneless TIME must not be shifted by loc: same wall-clock, still UTC. + v, err := ScanCell(arr, 0, loc) + if err != nil { + t.Fatal(err) + } + got := v.(time.Time) + if !got.Equal(want) || got.Location() != time.UTC { + t.Errorf("got %v (%v), want %v (UTC)", got, got.Location(), want) + } + }) +} + // ScanCell renders DATE / TIMESTAMP in the requested location, matching the // Thrift path's .In(location); a nil location leaves the value in UTC. func TestScanCellTimestampLocation(t *testing.T) { diff --git a/internal/rows/arrowbased/arrowscan_parity_test.go b/internal/rows/arrowbased/arrowscan_parity_test.go index a42856d2..7963f110 100644 --- a/internal/rows/arrowbased/arrowscan_parity_test.go +++ b/internal/rows/arrowbased/arrowscan_parity_test.go @@ -385,6 +385,16 @@ func TestArrowbasedKernelTopLevelScalarParity(t *testing.T) { b.Append(arrow.Date32FromTime(time.Date(2026, 7, 9, 0, 0, 0, 0, time.UTC))) return b.NewArray() }}, + {"time32_millis", func() arrow.Array { + b := array.NewTime32Builder(pool, &arrow.Time32Type{Unit: arrow.Millisecond}) + b.Append(arrow.Time32((13*3600+1*60+1)*1000 + 500)) // 13:01:01.500 + return b.NewArray() + }}, + {"time64_micros", func() arrow.Array { + b := array.NewTime64Builder(pool, &arrow.Time64Type{Unit: arrow.Microsecond}) + b.Append(arrow.Time64((13*3600 + 1*60 + 1) * 1_000_000)) // 13:01:01 + return b.NewArray() + }}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { From 177ce66d09b52cf675ac3ccdbe5c7bdedc38ec78 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Wed, 21 Oct 2026 04:34:17 +0000 Subject: [PATCH 4/5] ai: apply changes for #508 (1 review thread) Addresses: - #4333050880 at internal/rows/arrowbased/columnValues.go:518 Signed-off-by: peco-engineer-bot[bot] --- internal/rows/arrowbased/columnValues.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/internal/rows/arrowbased/columnValues.go b/internal/rows/arrowbased/columnValues.go index 92a20faf..5b7893b5 100644 --- a/internal/rows/arrowbased/columnValues.go +++ b/internal/rows/arrowbased/columnValues.go @@ -520,6 +520,23 @@ func (tvc *timeValueContainer) Value(i int) (any, error) { return val, nil } +// ValueString renders the TIME as a zoneless time-of-day string (HH:MM:SS +// with optional fractional seconds). It is used when a TIME appears nested +// inside a complex type (STRUCT/LIST/MAP): the default time.Time marshaling +// emits t.String(), which would leak the spurious 1970-01-01 epoch anchor +// date for a value that carries no calendar date. See marshalScalar. +func (tvc *timeValueContainer) ValueString(i int) string { + v, err := tvc.Value(i) + if err != nil { + return "" + } + t, ok := v.(time.Time) + if !ok { + return "" + } + return t.Format("15:04:05.999999999") +} + func (tvc *timeValueContainer) IsNull(i int) bool { return tvc.timeArray.IsNull(i) } @@ -666,6 +683,13 @@ func marshalScalar(holder columnValues, i int, val any) ([]byte, error) { if d, ok := holder.(*decimal128Container); ok { return []byte(d.ValueString(i)), nil } + // A SQL TIME is a zoneless time-of-day. Rendering it via the generic + // time.Time path would emit t.String() with the 1970-01-01 epoch anchor + // date (e.g. "1970-01-01 09:15:00 +0000 UTC"), leaking a spurious date for + // a value that carries none. Emit just the time-of-day as a JSON string. + if t, ok := holder.(*timeValueContainer); ok { + return json.Marshal(t.ValueString(i)) + } return marshal(val) } From f630b23999f631810d888814587a494af2182f3f Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Wed, 21 Oct 2026 04:43:25 +0000 Subject: [PATCH 5/5] ai: apply changes for #508 (1 review thread) Addresses: - #4333081459 at internal/arrowscan/arrowscan.go:225 Signed-off-by: peco-engineer-bot[bot] --- internal/arrowscan/arrowscan.go | 17 +++++++++++++++++ .../rows/arrowbased/arrowscan_parity_test.go | 17 +++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/internal/arrowscan/arrowscan.go b/internal/arrowscan/arrowscan.go index aceb3801..5333be2b 100644 --- a/internal/arrowscan/arrowscan.go +++ b/internal/arrowscan/arrowscan.go @@ -412,6 +412,15 @@ func timeOfDayToTime(unitsSinceMidnight int64, unit arrow.TimeUnit) time.Time { Add(time.Duration(unitsSinceMidnight * nsPerUnit)) } +// formatTimeOfDay renders an arrow time32/time64 value as the zoneless +// time-of-day string the Thrift path emits for a nested TIME +// (timeValueContainer.ValueString: "15:04:05.999999999"). Top-level TIME returns +// a time.Time (see the scanCell Time32/Time64 arms); nested TIME must instead be +// a bare string so the JSON leaf does not leak the epoch anchor date. +func formatTimeOfDay(unitsSinceMidnight int64, unit arrow.TimeUnit) string { + return timeOfDayToTime(unitsSinceMidnight, unit).Format("15:04:05.999999999") +} + // inLocation renders t in loc, matching the Thrift path's .In(location); a nil // loc leaves the value in UTC (arrow's ToTime default). func inLocation(t time.Time, loc *time.Location) time.Time { @@ -476,6 +485,14 @@ func writeJSON(b *strings.Builder, col arrow.Array, row int, loc *time.Location, // "3.140000104904175". The Thrift nested path marshals the native float32, // so widening here would break byte-parity for ARRAY/MAP/STRUCT<…FLOAT…>. return writeScalarJSON(b, c.Value(row)) + case *array.Time32: + // Nested SQL TIME: emit the zoneless time-of-day string, matching the + // Thrift path's timeValueContainer.ValueString (marshalScalar json.Marshals + // it). Falling through to writeScalarJSON would quote t.String() and leak + // the 1970-01-01 epoch anchor date, diverging from Thrift. + return writeScalarJSON(b, formatTimeOfDay(int64(c.Value(row)), col.DataType().(*arrow.Time32Type).Unit)) + case *array.Time64: + return writeScalarJSON(b, formatTimeOfDay(int64(c.Value(row)), col.DataType().(*arrow.Time64Type).Unit)) default: v, err := scalarForJSON(col, row, loc) if err != nil { diff --git a/internal/rows/arrowbased/arrowscan_parity_test.go b/internal/rows/arrowbased/arrowscan_parity_test.go index 7963f110..7b50b802 100644 --- a/internal/rows/arrowbased/arrowscan_parity_test.go +++ b/internal/rows/arrowbased/arrowscan_parity_test.go @@ -169,6 +169,23 @@ func TestArrowbasedKernelRenderParity(t *testing.T) { b.FieldBuilder(0).(*array.TimestampBuilder).Append(arrow.Timestamp(ts.UnixMicro())) return b.NewArray() }}, + // Nested SQL TIME leaf: both backends must emit the zoneless time-of-day + // string ("09:15:30"), NOT a quoted time.Time .String() that would leak + // the 1970-01-01 epoch anchor date. time32 is seconds-unit here. + {"nested_time32", func() arrow.Array { + dt := arrow.StructOf(arrow.Field{Name: "t", Type: arrow.FixedWidthTypes.Time32s}) + b := array.NewStructBuilder(pool, dt) + b.Append(true) + b.FieldBuilder(0).(*array.Time32Builder).Append(arrow.Time32(9*3600 + 15*60 + 30)) + return b.NewArray() + }}, + {"nested_time64", func() arrow.Array { + dt := arrow.StructOf(arrow.Field{Name: "t", Type: arrow.FixedWidthTypes.Time64us}) + b := array.NewStructBuilder(pool, dt) + b.Append(true) + b.FieldBuilder(0).(*array.Time64Builder).Append(arrow.Time64((9*3600+15*60+30)*1_000_000 + 123_456)) + return b.NewArray() + }}, // VARIANT and GEOMETRY arrive over Arrow as plain STRING columns (verified // live: a top-level VARIANT is the JSON *string* "{\"a\":1}", GEOMETRY is the // WKT string "POINT(1 2)"). Nested inside a container they are string leaves,