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/arrowscan/arrowscan.go b/internal/arrowscan/arrowscan.go index c6705955..fa69e0a9 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,36 @@ 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)) +} + +// 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 { @@ -444,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 { @@ -576,5 +625,15 @@ func scalarForJSON(col arrow.Array, row int, loc *time.Location) (any, error) { if col.IsNull(row) { return nil, nil } + // A TIME map key must render as the zoneless time-of-day string, same as a + // nested TIME value in writeJSON (and the Thrift path's {"09:15:00":…} key). + // ScanCell returns a 1970-01-01-anchored time.Time for TIME, which + // writeJSONKey would quote via .String() and leak the epoch date. + switch c := col.(type) { + case *array.Time32: + return formatTimeOfDay(int64(c.Value(row)), c.DataType().(*arrow.Time32Type).Unit), nil + case *array.Time64: + return formatTimeOfDay(int64(c.Value(row)), c.DataType().(*arrow.Time64Type).Unit), nil + } return ScanCell(col, row, loc) } diff --git a/internal/arrowscan/arrowscan_test.go b/internal/arrowscan/arrowscan_test.go index e68745eb..56977b3b 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) { @@ -402,6 +480,48 @@ func TestScanCellNested(t *testing.T) { } }) + // TIME map keys must render as the zoneless time-of-day string the Thrift + // path emits ({"09:15:00":1}), not the epoch-anchored time.Time.String(). + t.Run("map time32 key", func(t *testing.T) { + b := array.NewMapBuilder(pool, &arrow.Time32Type{Unit: arrow.Second}, arrow.PrimitiveTypes.Int64, false) + defer b.Release() + kb := b.KeyBuilder().(*array.Time32Builder) + ib := b.ItemBuilder().(*array.Int64Builder) + b.Append(true) + kb.Append(arrow.Time32(9*3600 + 15*60)) + ib.Append(1) + arr := b.NewArray() + defer arr.Release() + v, err := ScanCell(arr, 0, nil) + if err != nil { + t.Fatal(err) + } + if v.(string) != `{"09:15:00":1}` { + t.Errorf("got %q, want {\"09:15:00\":1}", v) + } + }) + + t.Run("map time64 key", func(t *testing.T) { + b := array.NewMapBuilder(pool, &arrow.Time64Type{Unit: arrow.Microsecond}, arrow.PrimitiveTypes.Int64, false) + defer b.Release() + kb := b.KeyBuilder().(*array.Time64Builder) + ib := b.ItemBuilder().(*array.Int64Builder) + b.Append(true) + kb.Append(arrow.Time64((9*3600 + 15*60) * 1_000_000)) + ib.Append(1) + kb.Append(arrow.Time64((9*3600+15*60)*1_000_000 + 500_000)) + ib.Append(2) + arr := b.NewArray() + defer arr.Release() + v, err := ScanCell(arr, 0, nil) + if err != nil { + t.Fatal(err) + } + if v.(string) != `{"09:15:00":1,"09:15:00.5":2}` { + t.Errorf("got %q, want {\"09:15:00\":1,\"09:15:00.5\":2}", v) + } + }) + t.Run("nested_decimal_exact", func(t *testing.T) { // A decimal inside a struct must render as an exact JSON number, not a // lossy float64 (19.99, not 19.990000000000002) — matching Thrift's diff --git a/internal/arrowscan/coltype.go b/internal/arrowscan/coltype.go index d1e8a750..ef6a386e 100644 --- a/internal/arrowscan/coltype.go +++ b/internal/arrowscan/coltype.go @@ -79,6 +79,10 @@ func ColumnTypeInfoFor(dt arrow.DataType) ColumnTypeInfo { return ColumnTypeInfo{DatabaseTypeName: "DATE", ScanType: scanTypeDateTime} case arrow.TIMESTAMP: return ColumnTypeInfo{DatabaseTypeName: "TIMESTAMP", ScanType: scanTypeDateTime} + case arrow.TIME32, arrow.TIME64: + // SQL TIME: ScanCellCached returns a time.Time (time-of-day on the Unix + // epoch date, UTC), so report TIME/time.Time to keep metadata in lockstep. + return ColumnTypeInfo{DatabaseTypeName: "TIME", ScanType: scanTypeDateTime} case arrow.DECIMAL128: // Match Thrift's sql.RawBytes scan type even though the kernel renders the // value as an exact string — both convert cleanly to a caller's *string/*[]byte. diff --git a/internal/arrowscan/coltype_test.go b/internal/arrowscan/coltype_test.go index dfb643be..82bd4047 100644 --- a/internal/arrowscan/coltype_test.go +++ b/internal/arrowscan/coltype_test.go @@ -44,6 +44,8 @@ func TestColumnTypeInfoFor(t *testing.T) { {"binary", arrow.BinaryTypes.Binary, "BINARY", raw, math.MaxInt64, true}, {"date32", arrow.FixedWidthTypes.Date32, "DATE", tm, 0, false}, {"timestamp", arrow.FixedWidthTypes.Timestamp_us, "TIMESTAMP", tm, 0, false}, + {"time32", arrow.FixedWidthTypes.Time32ms, "TIME", tm, 0, false}, + {"time64", arrow.FixedWidthTypes.Time64us, "TIME", tm, 0, false}, {"decimal", &arrow.Decimal128Type{Precision: 10, Scale: 2}, "DECIMAL", raw, 0, false}, {"list", arrow.ListOf(arrow.PrimitiveTypes.Int64), "ARRAY", raw, math.MaxInt64, true}, {"map", arrow.MapOf(arrow.BinaryTypes.String, arrow.PrimitiveTypes.Int64), "MAP", raw, math.MaxInt64, true}, @@ -84,6 +86,8 @@ func TestColumnTypeInfoScanTypeCoversScanner(t *testing.T) { arrow.PrimitiveTypes.Float32, arrow.PrimitiveTypes.Float64, arrow.BinaryTypes.String, arrow.BinaryTypes.Binary, arrow.FixedWidthTypes.Date32, arrow.FixedWidthTypes.Timestamp_us, + arrow.FixedWidthTypes.Time32s, arrow.FixedWidthTypes.Time32ms, + arrow.FixedWidthTypes.Time64us, arrow.FixedWidthTypes.Time64ns, &arrow.Decimal128Type{Precision: 10, Scale: 2}, arrow.ListOf(arrow.PrimitiveTypes.Int64), arrow.MapOf(arrow.BinaryTypes.String, arrow.PrimitiveTypes.Int64), diff --git a/internal/rows/arrowbased/arrowRows.go b/internal/rows/arrowbased/arrowRows.go index 58970d85..50f9f402 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{nsPerUnit: timeUnitToNanos(t.Unit)}, nil + + case *arrow.Time64Type: + return &timeValueContainer{nsPerUnit: timeUnitToNanos(t.Unit)}, nil + case *arrow.BinaryType: return &columnValuesTyped[*array.Binary, []byte]{}, nil diff --git a/internal/rows/arrowbased/arrowscan_parity_test.go b/internal/rows/arrowbased/arrowscan_parity_test.go index a42856d2..453b8a53 100644 --- a/internal/rows/arrowbased/arrowscan_parity_test.go +++ b/internal/rows/arrowbased/arrowscan_parity_test.go @@ -112,6 +112,27 @@ func TestArrowbasedKernelRenderParity(t *testing.T) { b.ItemBuilder().(*array.Int64Builder).Append(9) return b.NewArray() }}, + // TIME map keys: Thrift emits the zoneless time-of-day key + // ({"09:15:00":1}); the kernel map-key path must not fall back to the + // epoch-anchored time.Time.String() ("1970-01-01 09:15:00 +0000 UTC"). + {"map_time32_key", func() arrow.Array { + b := array.NewMapBuilder(pool, arrow.FixedWidthTypes.Time32s, arrow.PrimitiveTypes.Int64, false) + b.Append(true) + b.KeyBuilder().(*array.Time32Builder).Append(arrow.Time32(9*3600 + 15*60)) + b.ItemBuilder().(*array.Int64Builder).Append(1) + return b.NewArray() + }}, + {"map_time64_key", func() arrow.Array { + b := array.NewMapBuilder(pool, arrow.FixedWidthTypes.Time64us, arrow.PrimitiveTypes.Int64, false) + b.Append(true) + kb := b.KeyBuilder().(*array.Time64Builder) + ib := b.ItemBuilder().(*array.Int64Builder) + kb.Append(arrow.Time64((9*3600 + 15*60) * 1_000_000)) + ib.Append(1) + kb.Append(arrow.Time64((9*3600+15*60+30)*1_000_000 + 123_456)) + ib.Append(2) + return b.NewArray() + }}, {"nested_float32", func() arrow.Array { dt := arrow.StructOf(arrow.Field{Name: "f", Type: arrow.PrimitiveTypes.Float32}) b := array.NewStructBuilder(pool, dt) @@ -169,6 +190,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, @@ -385,6 +423,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) { diff --git a/internal/rows/arrowbased/columnValues.go b/internal/rows/arrowbased/columnValues.go index 73aeb01f..5b7893b5 100644 --- a/internal/rows/arrowbased/columnValues.go +++ b/internal/rows/arrowbased/columnValues.go @@ -473,6 +473,92 @@ 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 +} + +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 + // 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)) + 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) +} + +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 @@ -597,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) } diff --git a/internal/rows/arrowbased/time_test.go b/internal/rows/arrowbased/time_test.go new file mode 100644 index 00000000..88a85500 --- /dev/null +++ b/internal/rows/arrowbased/time_test.go @@ -0,0 +1,175 @@ +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{} + + // 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 + }{ + { + 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), + loc: time.UTC, + 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), + 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, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dt := &arrow.Time64Type{Unit: tc.unit} + holder, err := vcm.makeColumnValueContainer(dt, tc.loc, 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{} + + 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, tc.loc, 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()) + }) + } +}