From c005bbec185187873f664b275a9240e869070942 Mon Sep 17 00:00:00 2001 From: TangoEnSkai <21152231+TangoEnSkai@users.noreply.github.com> Date: Sat, 29 Aug 2026 14:39:17 +0900 Subject: [PATCH] fix: data race on Arrow v12 shared timestamp types (prewarm GetToTimeFunc) Arrow v12's TimestampType.GetToTimeFunc lazily caches the type's *time.Location via GetZone without synchronization (apache/arrow#38795, fixed only in later Arrow versions the driver cannot move to yet, see issue #228). NewArrowRowScanner calls GetToTimeFunc on the shared arrow.FixedWidthTypes.Timestamp_us singleton for every result set, so concurrent queries race on that first call, as reported in issue #179. Warm the cache for all four shared fixed-width timestamp singletons in a package init, before any concurrency is possible, so every later call is a plain read. This is the same workaround users currently have to apply in their own code; doing it in the driver removes the need to import Arrow (a transitive dependency) just to use databricks-sql-go safely from multiple goroutines. The new regression test fails under the race detector if the init is removed. Co-Authored-By: Claude Fable 5 Signed-off-by: TangoEnSkai <21152231+TangoEnSkai@users.noreply.github.com> --- CHANGELOG.md | 3 ++ internal/rows/arrowbased/arrowRows.go | 17 +++++++++++ internal/rows/arrowbased/arrowRows_test.go | 33 ++++++++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f321ff5..08282308 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Release History +## Unreleased +- Fix a data race when timestamp results are read from concurrent queries: Arrow v12's `TimestampType.GetToTimeFunc` lazily caches the type's `*time.Location` without synchronization, and the driver calls it on the shared `arrow.FixedWidthTypes` singletons. The cache is now warmed at package init so later calls are read-only (databricks/databricks-sql-go#179) + ## v1.15.1 (2026-09-01) - Pin the seven per-platform kernel bindings modules to v1.0.0. - Disable kernel telemetry by default when `enableTelemetry` is unset; explicit `true` and `false` values are unchanged (databricks/databricks-sql-go#464). diff --git a/internal/rows/arrowbased/arrowRows.go b/internal/rows/arrowbased/arrowRows.go index 58970d85..a8a41241 100644 --- a/internal/rows/arrowbased/arrowRows.go +++ b/internal/rows/arrowbased/arrowRows.go @@ -35,6 +35,23 @@ type SparkArrowRecord interface { arrow.Record } +// Arrow v12's TimestampType caches its *time.Location lazily on the first +// GetZone/GetToTimeFunc call without synchronization (apache/arrow#38795, +// fixed in later Arrow versions). NewArrowRowScanner calls GetToTimeFunc on +// the shared arrow.FixedWidthTypes singletons, so concurrent queries race on +// that first call. Warming the cache here, before any concurrency is +// possible, makes every later call a plain read. +func init() { + for _, dt := range []arrow.DataType{ + arrow.FixedWidthTypes.Timestamp_s, + arrow.FixedWidthTypes.Timestamp_ms, + arrow.FixedWidthTypes.Timestamp_us, + arrow.FixedWidthTypes.Timestamp_ns, + } { + _, _ = dt.(*arrow.TimestampType).GetToTimeFunc() + } +} + type timeStampFn func(arrow.Timestamp) time.Time type colInfo struct { diff --git a/internal/rows/arrowbased/arrowRows_test.go b/internal/rows/arrowbased/arrowRows_test.go index 713ab314..6c33b11d 100644 --- a/internal/rows/arrowbased/arrowRows_test.go +++ b/internal/rows/arrowbased/arrowRows_test.go @@ -9,6 +9,7 @@ import ( "math/big" "os" "strings" + "sync" "testing" "time" @@ -2398,3 +2399,35 @@ func TestDecimalInComplexTypes(t *testing.T) { assert.Equal(t, `{"col2":null}`, v) }) } + +// Regression test for databricks/databricks-sql-go#179: Arrow v12's +// TimestampType.GetToTimeFunc lazily caches the type's *time.Location without +// synchronization, so the first concurrent calls on the shared +// arrow.FixedWidthTypes timestamp singletons were a data race. The package +// init() in arrowRows.go warms that cache; without it, this test fails under +// the race detector. +func TestSharedTimestampGetToTimeFuncConcurrency(t *testing.T) { + sharedTimestampTypes := []arrow.DataType{ + arrow.FixedWidthTypes.Timestamp_s, + arrow.FixedWidthTypes.Timestamp_ms, + arrow.FixedWidthTypes.Timestamp_us, + arrow.FixedWidthTypes.Timestamp_ns, + } + + var wg sync.WaitGroup + for i := 0; i < 32; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for _, dt := range sharedTimestampTypes { + toTime, err := dt.(*arrow.TimestampType).GetToTimeFunc() + if err != nil { + t.Errorf("GetToTimeFunc failed for %s: %v", dt, err) + return + } + _ = toTime(arrow.Timestamp(0)) + } + }() + } + wg.Wait() +}