Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions driver_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
49 changes: 49 additions & 0 deletions internal/arrowscan/arrowscan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Comment thread
peco-review-bot[bot] marked this conversation as resolved.
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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
86 changes: 82 additions & 4 deletions internal/arrowscan/arrowscan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions internal/rows/arrowbased/arrowRows.go
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,12 @@ func (vcm *arrowValueContainerMaker) makeColumnValueContainer(t arrow.DataType,
case *arrow.TimestampType:
return &timestampValueContainer{location: location, toTimestampFn: toTimestampFn}, nil

case *arrow.Time32Type:
Comment thread
peco-review-bot[bot] marked this conversation as resolved.
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

Expand Down
27 changes: 27 additions & 0 deletions internal/rows/arrowbased/arrowscan_parity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -385,6 +402,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) {
Expand Down
93 changes: 93 additions & 0 deletions internal/rows/arrowbased/columnValues.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Comment thread
peco-review-bot[bot] marked this conversation as resolved.
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) {
Comment thread
peco-review-bot[bot] marked this conversation as resolved.
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
Expand Down Expand Up @@ -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)
}

Expand Down
Loading
Loading