Skip to content

Commit

Permalink
fix: deep copy strings during scan (#499)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
asdine authored Jun 18, 2023
1 parent 5397cbc commit c5d6f3b
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 1 deletion.
41 changes: 41 additions & 0 deletions db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 5 additions & 1 deletion document/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit c5d6f3b

Please sign in to comment.