From c5d6f3b946694255389c11c1282d43489be43a8f Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Sun, 18 Jun 2023 11:38:12 +0100 Subject: [PATCH] fix: deep copy strings during scan (#499) When decoding a document from the disk, fields of type TEXT are not copied on purpose to avoid doing unnecessary copies. However, when the data reaches the upper layers, usually when it's about to get scanned (i.e. document.Scan, document.StructScan), strings must always get deep cloned before being returned to the user. --- db_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ document/scan.go | 6 +++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/db_test.go b/db_test.go index 0ad7951d..3ddbad2e 100644 --- a/db_test.go +++ b/db_test.go @@ -227,6 +227,47 @@ func TestPrepareThreadSafe(t *testing.T) { assert.NoError(t, err) } +func TestIterateDeepCopy(t *testing.T) { + db, err := genji.Open(":memory:") + assert.NoError(t, err) + defer db.Close() + + err = db.Exec(` + CREATE TABLE foo ( + a integer primary key, + b text not null + ); + + INSERT INTO foo (a, b) VALUES + (1, 'sample text 1'), + (2, 'sample text 2'); + `) + assert.NoError(t, err) + + res, err := db.Query(`SELECT * FROM foo ORDER BY a DESC`) + assert.NoError(t, err) + + type item struct { + A int + B string + } + + var items []*item + err = res.Iterate(func(d types.Document) error { + var i item + err := document.StructScan(d, &i) + assert.NoError(t, err) + + items = append(items, &i) + return nil + }) + assert.NoError(t, err) + + require.Equal(t, len(items), 2) + require.Equal(t, &item{A: 2, B: "sample text 2"}, items[0]) + require.Equal(t, &item{A: 1, B: "sample text 1"}, items[1]) +} + func BenchmarkSelect(b *testing.B) { for size := 1; size <= 10000; size *= 10 { b.Run(fmt.Sprintf("%.05d", size), func(b *testing.B) { diff --git a/document/scan.go b/document/scan.go index c1ce378a..53ea32c9 100644 --- a/document/scan.go +++ b/document/scan.go @@ -275,7 +275,11 @@ func scanValue(v types.Value, ref reflect.Value) error { if err != nil { return err } - ref.SetString(string(types.As[string](v))) + // copy the string to avoid + // keeping a reference to the underlying buffer + // which could be reused + cp := strings.Clone(types.As[string](v)) + ref.SetString(cp) return nil case reflect.Bool: v, err := CastAsBool(v)