This is an automated email from the ASF dual-hosted git repository.
amoeba pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new 74d5395a8 fix(go/adbc/sqldriver): return owned copies of string/binary
values (#4625)
74d5395a8 is described below
commit 74d5395a880cc3301b83b771d1d756c729c67558
Author: İrem Çağın Yurttürk <[email protected]>
AuthorDate: Wed Jul 29 21:20:25 2026 +0300
fix(go/adbc/sqldriver): return owned copies of string/binary values (#4625)
### Problem
`rows.Next` returned `array.String.Value` (and
`LargeString`/`Binary`/`LargeBinary`) directly as the `driver.Value`.
Those are zero-copy values that alias the underlying Arrow data buffer.
`database/sql` retains the `driver.Value`s after `Next` returns — and
after the underlying record batch (and its buffer) is released. For ADBC
results the batch buffers are C-owned, so once the batch is released the
value reads back as garbage.
Minimal reproduction: reading a `VARCHAR` from a DuckDB ADBC query
through this `sqldriver` returns a corrupted string (e.g.
`"\xb6|:u\x14\x7f\x00"`, stale memory) instead of the actual text.
Reading the same result directly at the Arrow level (before release) is
correct, which localizes the problem to `rows.Next` handing back an
alias rather than an owned value.
### What changed
Copy string/binary values into Go-owned memory (`strings.Clone` /
`bytes.Clone`) so they outlive the record batch.
### Why it surfaced now
The zero-copy return predates this (it's been in `rows.Next` since the
original `database/sql` wrapper, #97). It was surfaced by the arrow-go
v18.7 bump — specifically apache/arrow-go#793, which replaced the
`nativeCRecordBatchReader` finalizer with deterministic atomic
Retain/Release. C memory is now freed promptly when the batch is
released instead of lazily at GC, so the latent aliasing became a
use-after-free.
---------
Signed-off-by: iremcaginyurtturk <[email protected]>
---
go/adbc/sqldriver/driver.go | 12 ++++++---
go/adbc/sqldriver/driver_internals_test.go | 41 ++++++++++++++++++++++++++++++
2 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/go/adbc/sqldriver/driver.go b/go/adbc/sqldriver/driver.go
index 5ab33dcc1..df2855263 100644
--- a/go/adbc/sqldriver/driver.go
+++ b/go/adbc/sqldriver/driver.go
@@ -18,6 +18,7 @@
package sqldriver
import (
+ "bytes"
"context"
"database/sql"
"database/sql/driver"
@@ -720,13 +721,16 @@ func (r *rows) Next(dest []driver.Value) error {
case *array.Float16:
dest[i] = col.Value(int(r.curRow))
case *array.String:
- dest[i] = col.Value(int(r.curRow))
+ // col.Value returns a string aliasing the Arrow buffer
(zero-copy).
+ // database/sql retains driver.Values past this call,
after the record
+ // is released, so hand back an owned copy to avoid a
dangling read.
+ dest[i] = strings.Clone(col.Value(int(r.curRow)))
case *array.LargeString:
- dest[i] = col.Value(int(r.curRow))
+ dest[i] = strings.Clone(col.Value(int(r.curRow)))
case *array.Binary:
- dest[i] = col.Value(int(r.curRow))
+ dest[i] = bytes.Clone(col.Value(int(r.curRow)))
case *array.LargeBinary:
- dest[i] = col.Value(int(r.curRow))
+ dest[i] = bytes.Clone(col.Value(int(r.curRow)))
case *array.Date32:
dest[i] = col.Value(int(r.curRow)).ToTime()
case *array.Date64:
diff --git a/go/adbc/sqldriver/driver_internals_test.go
b/go/adbc/sqldriver/driver_internals_test.go
index 33a85062f..ac562053b 100644
--- a/go/adbc/sqldriver/driver_internals_test.go
+++ b/go/adbc/sqldriver/driver_internals_test.go
@@ -485,3 +485,44 @@ func TestArrFromVal(t *testing.T) {
})
}
}
+
+// TestRowsStringValuesAreOwnedCopies is a regression test for a
use-after-free:
+// array.String.Value returns a string that aliases the Arrow data buffer, so
+// handing it straight to database/sql leaves a dangling reference once the
+// record batch (and its buffer, which for ADBC/C-Data results is C-owned) is
+// released — the value then reads back as garbage. rows.Next must return an
+// owned copy. This was surfaced by the arrow-go v18.7 bump.
+func TestRowsStringValuesAreOwnedCopies(t *testing.T) {
+ mem := memory.DefaultAllocator
+
+ sb := array.NewStringBuilder(mem)
+ sb.Append("UNKNOWN")
+ strArr := sb.NewStringArray()
+ defer strArr.Release()
+ sb.Release()
+
+ schema := arrow.NewSchema([]arrow.Field{{Name: "age", Type:
arrow.BinaryTypes.String, Nullable: true}}, nil)
+ rec := array.NewRecordBatch(schema, []arrow.Array{strArr}, 1)
+ defer rec.Release()
+
+ rdr, err := array.NewRecordReader(schema, []arrow.RecordBatch{rec})
+ require.NoError(t, err)
+ defer rdr.Release()
+
+ r := &rows{rdr: rdr}
+ dest := make([]driver.Value, 1)
+ require.NoError(t, r.Next(dest))
+
+ got, ok := dest[0].(string)
+ require.True(t, ok, "expected string, got %T", dest[0])
+ require.Equal(t, "UNKNOWN", got)
+
+ // Clobber the underlying Arrow data buffer. An owned copy is
unaffected; a
+ // zero-copy alias would now read back as the overwritten bytes.
+ raw := strArr.ValueBytes()
+ for i := range raw {
+ raw[i] = 'X'
+ }
+ require.Equal(t, "UNKNOWN", got,
+ "rows.Next returned a zero-copy alias of the Arrow buffer
instead of an owned copy")
+}
