Refactor scanMulti to only call scan once (#339)

.github/workflows/release.yml

@@ -17,7 +17,7 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@v3
         with:
-          go-version: '1.20'
+          go-version: 1.21
       - name: Run GoReleaser
         uses: goreleaser/goreleaser-action@v2
         with:

.github/workflows/test.yml

@@ -13,7 +13,7 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-latest]
-        go: [1.17, 1.18, 1.19, '1.20']
+        go: [1.17, 1.18, 1.19, '1.20', 1.21]
     runs-on: ${{ matrix.os }}
     steps:
       - name: Set up Go 1.x
@@ -33,7 +33,7 @@ jobs:
       - name: Set up Go 1.x
         uses: actions/setup-go@v3
         with:
-          go-version: '1.20'
+          go-version: 1.21
       - name: Check out code into the Go module directory
         uses: actions/checkout@v2
         with:

collection.go

@@ -7,7 +7,8 @@ import (
 type slice interface {
 	table
 	Reset()
-	Add() *Document
+	CreateDocument() *Document
+	Append(doc *Document)
 	Get(index int) *Document
 	Len() int
 	Meta() DocumentMeta
@@ -142,19 +143,22 @@ func (c Collection) Reset() {
 
 // Add new document into collection.
 func (c Collection) Add() *Document {
-	var (
-		index = c.Len()
-		typ   = c.rt.Elem()
-		drv   = reflect.Zero(typ)
-	)
-
-	if typ.Kind() == reflect.Ptr && drv.IsNil() {
-		drv = reflect.New(drv.Type().Elem())
-	}
+	c.Append(c.CreateDocument())
+	return c.Get(c.Len() - 1)
+}
 
-	c.rv.Set(reflect.Append(c.rv, drv))
+// CreateDocument returns new document with zero values.
+func (c Collection) CreateDocument() *Document {
+	return newZeroDocument(c.rt.Elem())
+}
 
-	return NewDocument(c.rvIndex(index).Addr())
+// Append new document into collection.
+func (c Collection) Append(doc *Document) {
+	if c.rt.Elem().Kind() == reflect.Ptr {
+		c.rv.Set(reflect.Append(c.rv, doc.rv.Addr()))
+	} else {
+		c.rv.Set(reflect.Append(c.rv, doc.rv))
+	}
 }
 
 // Truncate collection.

cursor.go

@@ -1,7 +1,6 @@
 package rel
 
 import (
-	"database/sql"
 	"reflect"
 )
 
@@ -63,48 +62,42 @@ func scanMulti(cur Cursor, keyField string, keyType reflect.Type, cols map[any][
 		return err
 	}
 
-	var (
-		found       = false
-		keyValue    = reflect.New(keyType)
-		keyScanners = make([]any, len(fields))
-	)
-
-	for i, field := range fields {
+	keyFound := false
+	for _, field := range fields {
 		if keyField == field {
-			found = true
-			keyScanners[i] = keyValue.Interface()
-		} else {
-			// need to create distinct copies
-			// otherwise next scan result will be corrupted
-			keyScanners[i] = &sql.RawBytes{}
+			keyFound = true
 		}
 	}
 
-	if !found && fields != nil {
+	if !keyFound && fields != nil {
 		panic("rel: primary key row does not exists")
 	}
 
+	var doc *Document
+	for k := range cols {
+		for _, col := range cols[k] {
+			doc = col.CreateDocument()
+			break
+		}
+		break
+	}
+
 	// scan the result
 	for cur.Next() {
 		// scan key
-		if err := cur.Scan(keyScanners...); err != nil {
+		if err := cur.Scan(doc.Scanners(fields)...); err != nil {
 			return err
 		}
 
-		var (
-			key = reflect.Indirect(keyValue).Interface()
-		)
+		key, found := doc.Value(keyField)
+		mustTrue(found, "rel: key field not found")
 
 		for _, col := range cols[key] {
-			var (
-				doc      = col.Add()
-				scanners = doc.Scanners(fields)
-			)
-
-			if err := cur.Scan(scanners...); err != nil {
-				return err
-			}
+			col.Append(doc)
 		}
+
+		// create new doc for next scan
+		doc = doc.CreateDocument()
 	}
 
 	return nil

cursor_test.go

@@ -58,9 +58,13 @@ func (tc *testCursor) MockScan(ret ...any) *mock.Call {
 		Return(func(scanners ...any) error {
 			for i := 0; i < len(scanners); i++ {
 				if v, ok := scanners[i].(sql.Scanner); ok {
-					v.Scan(ret[i])
+					if err := v.Scan(ret[i]); err != nil {
+						return err
+					}
 				} else {
-					convertAssign(scanners[i], ret[i])
+					if err := convertAssign(scanners[i], ret[i]); err != nil {
+						return err
+					}
 				}
 			}
 
@@ -192,8 +196,8 @@ func TestScanMulti(t *testing.T) {
 	cur.On("Fields").Return([]string{"id", "name", "age", "created_at", "updated_at"}, nil).Once()
 
 	cur.On("Next").Return(true).Twice()
-	cur.MockScan(10, "Del Piero", nil, now, nil).Times(3)
-	cur.MockScan(11, "Nedved", 46, now, now).Twice()
+	cur.MockScan(10, "Del Piero", nil, now, nil).Once()
+	cur.MockScan(11, "Nedved", 46, now, now).Once()
 	cur.On("Next").Return(false).Once()
 
 	assert.Nil(t, scanMulti(cur, keyField, keyType, cols))
@@ -237,7 +241,6 @@ func TestScanMulti_scanError(t *testing.T) {
 	cur.On("Fields").Return([]string{"id", "name", "age", "created_at", "updated_at"}, nil).Once()
 
 	cur.On("Next").Return(true).Once()
-	cur.MockScan(11, "Nedved", 46, Now, Now).Once()
 	cur.On("Scan", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(err).Once()
 
 	assert.Equal(t, err, scanMulti(cur, keyField, keyType, cols))
@@ -324,8 +327,8 @@ func TestScanMulti_multipleTimes(t *testing.T) {
 	cur.On("Fields").Return([]string{"id", "name", "age", "created_at", "updated_at"}, nil).Once()
 
 	cur.On("Next").Return(true).Twice()
-	cur.MockScan(10, "Del Piero", nil, now, nil).Times(3)
-	cur.MockScan(11, "Nedved", 46, now, now).Twice()
+	cur.MockScan(10, "Del Piero", nil, now, nil).Once()
+	cur.MockScan(11, "Nedved", 46, now, now).Once()
 	cur.On("Next").Return(false).Once()
 
 	assert.Nil(t, scanMulti(cur, keyField, keyType, cols))
@@ -360,8 +363,8 @@ func TestScanMulti_multipleTimes(t *testing.T) {
 	cur.On("Fields").Return([]string{"id", "name", "age", "created_at", "updated_at"}, nil).Once()
 
 	cur.On("Next").Return(true).Twice()
-	cur.MockScan(12, "Linus Torvalds", 52, now, nil).Times(3)
-	cur.MockScan(13, "Tim Cook", 61, now, now).Twice()
+	cur.MockScan(12, "Linus Torvalds", 52, now, nil).Once()
+	cur.MockScan(13, "Tim Cook", 61, now, now).Once()
 	cur.On("Next").Return(false).Once()
 
 	assert.Nil(t, scanMulti(cur, keyField, keyType, cols))

document.go

@@ -261,15 +261,27 @@ func (d Document) association(name string) (Association, bool) {
 func (d Document) Reset() {
 }
 
-// Add returns this document.
-func (d *Document) Add() *Document {
-	// if d.rv is a null pointer, set it to a new struct.
+// CreateDocument returns new document with zero values.
+func (d Document) CreateDocument() *Document {
+	return newZeroDocument(d.rt)
+}
+
+// Append is alias for Assign for compatibility with internal slice interface
+func (d *Document) Append(o *Document) {
+	d.Assign(o)
+}
+
+// Assign document value to this document.
+func (d *Document) Assign(o *Document) {
 	if d.rv.Kind() == reflect.Ptr && d.rv.IsNil() {
-		d.rv.Set(reflect.New(d.rv.Type().Elem()))
+		d.rv.Set(o.rv.Addr())
 		d.rv = d.rv.Elem()
+	} else {
+		d.rv.Set(o.rv)
 	}
 
-	return d
+	d.meta = o.meta
+	d.v = o.v
 }
 
 // Get always returns this document, this is a noop for compatibility with collection.
@@ -336,3 +348,14 @@ func newDocument(v any, rv reflect.Value, readonly bool) *Document {
 		meta: getDocumentMeta(rt, false),
 	}
 }
+
+func newZeroDocument(rt reflect.Type) *Document {
+	if rt.Kind() == reflect.Ptr {
+		rt = rt.Elem()
+	}
+
+	rv := reflect.New(rt)
+	rv.Elem().Set(reflect.Zero(rt))
+
+	return NewDocument(rv)
+}

document_test.go

@@ -571,7 +571,6 @@ func TestDocument_Slice(t *testing.T) {
 		doc.Reset()
 		assert.Equal(t, 1, doc.Len())
 		assert.Equal(t, doc, doc.Get(0))
-		assert.Equal(t, doc, doc.Add())
 	})
 }
 

go.mod

@@ -1,6 +1,6 @@
 module github.com/go-rel/rel
 
-go 1.20
+go 1.21
 
 require (
 	github.com/jinzhu/inflection v1.0.0