Skip to content

Commit 40f63d0

Browse files
committed
fix(finisher_api): resolve FindInBatches cursor primary key from dest schema
FindInBatches builds its batch cursor by reading the primary key of the last scanned row through Statement.Schema. When a different Model is set (e.g. Model(A).FindInBatches(&[]B{})), Statement.Schema describes A, whose field index does not apply to B, so the read returns the wrong field (panic "reflect: Field index out of range" on older versions, "sql: unsupported type ..." on master), breaking pagination. Resolve the primary field from dest's own schema, matched by the Model's primary-key column name so the cursor value stays in sync with the ORDER BY / comparison column. Mirrors how Scan already re-parses a mismatched dest. When dest has no field for that column, fail with ErrPrimaryKeyRequired. Adds a regression test covering []B and []*B destinations. Closes #7737
1 parent 1d6ce99 commit 40f63d0

2 files changed

Lines changed: 131 additions & 4 deletions

File tree

finisher_api.go

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,8 @@ func (db *DB) FindInBatches(dest interface{}, batchSize int, fc func(tx *DB, bat
191191
queryDB = tx
192192
rowsAffected int64
193193
batch int
194+
195+
batchPrimaryField *schema.Field // resolved lazily below (#7737)
194196
)
195197

196198
// user specified offset or limit
@@ -238,12 +240,41 @@ func (db *DB) FindInBatches(dest interface{}, batchSize int, fc func(tx *DB, bat
238240

239241
// Optimize for-break
240242
resultsValue := reflect.Indirect(reflect.ValueOf(dest))
241-
if result.Statement.Schema.PrioritizedPrimaryField == nil {
242-
tx.AddError(ErrPrimaryKeyRequired)
243-
break
243+
244+
// The ORDER BY and cursor comparison use the Model's primary-key column,
245+
// but the value is read from the scanned dest rows. When a different
246+
// Model is set (e.g. Model(A).FindInBatches(&[]B{})), the Model's field
247+
// index doesn't apply to dest's type, so resolve the dest field mapping
248+
// to that same primary-key column — keeping column and value in sync and
249+
// mirroring how Scan re-parses a mismatched dest (#7737).
250+
if batchPrimaryField == nil {
251+
modelSchema := result.Statement.Schema
252+
if modelSchema == nil || modelSchema.PrioritizedPrimaryField == nil {
253+
tx.AddError(ErrPrimaryKeyRequired)
254+
break
255+
}
256+
257+
batchPrimaryField = modelSchema.PrioritizedPrimaryField
258+
destType := resultsValue.Type().Elem()
259+
if destType.Kind() == reflect.Ptr {
260+
destType = destType.Elem()
261+
}
262+
if destType != modelSchema.ModelType {
263+
parsed, err := schema.Parse(dest, db.cacheStore, db.NamingStrategy)
264+
if err != nil {
265+
tx.AddError(err)
266+
break
267+
}
268+
// No dest field maps to the Model's primary-key column: there is
269+
// no valid cursor value, so fail rather than risk a wrong-index read.
270+
if batchPrimaryField = parsed.LookUpField(modelSchema.PrioritizedPrimaryField.DBName); batchPrimaryField == nil {
271+
tx.AddError(ErrPrimaryKeyRequired)
272+
break
273+
}
274+
}
244275
}
245276

246-
primaryValue, zero := result.Statement.Schema.PrioritizedPrimaryField.ValueOf(tx.Statement.Context, resultsValue.Index(resultsValue.Len()-1))
277+
primaryValue, zero := batchPrimaryField.ValueOf(tx.Statement.Context, resultsValue.Index(resultsValue.Len()-1))
247278
if zero {
248279
tx.AddError(ErrPrimaryKeyRequired)
249280
break

tests/findinbatches_repro_test.go

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
package tests_test
2+
3+
import (
4+
"fmt"
5+
"testing"
6+
7+
"gorm.io/gorm"
8+
. "gorm.io/gorm/utils/tests"
9+
)
10+
11+
// Regression test for go-gorm/gorm#7737:
12+
// Model(A).FindInBatches(&[]B) must page correctly when B embeds A but has a
13+
// different field layout. Before the fix, the batch cursor read the primary key
14+
// through the Model's schema (A) against B's rows, yielding the wrong field
15+
// (panic on older versions, "unsupported type" SQL error on current master),
16+
// which broke pagination across batches.
17+
18+
type userExtended struct {
19+
User
20+
ExtraFoo int64
21+
}
22+
23+
func TestFindInBatchesModelDestMismatch(t *testing.T) {
24+
const total = 5
25+
created := make(map[uint]bool, total)
26+
for i := 0; i < total; i++ {
27+
u := User{Name: "fib_7737"}
28+
if err := DB.Create(&u).Error; err != nil {
29+
t.Fatalf("Create failed: %v", err)
30+
}
31+
created[u.ID] = true
32+
}
33+
34+
// query sets a Model that differs from the dest element type, and selects an
35+
// extra column that only exists on the dest — forcing the schema mismatch.
36+
query := func() *gorm.DB {
37+
return DB.Model(&User{}).Where("name = ?", "fib_7737").Select("*, 42 AS extra_foo")
38+
}
39+
40+
// assertPagedOnce verifies every created row was visited exactly once across
41+
// batches (no duplicate, no skip) — i.e. the primary-key cursor is correct.
42+
assertPagedOnce := func(t *testing.T, seen map[uint]bool, rowsAffected int64) {
43+
t.Helper()
44+
if int(rowsAffected) != total {
45+
t.Fatalf("expected RowsAffected=%d, got %d", total, rowsAffected)
46+
}
47+
if len(seen) != total {
48+
t.Fatalf("expected to page over %d unique rows, saw %d", total, len(seen))
49+
}
50+
for id := range created {
51+
if !seen[id] {
52+
t.Fatalf("row id %d was skipped by the cursor", id)
53+
}
54+
}
55+
}
56+
57+
// batchSize (2) < total (5) forces the primary-key cursor path across batches.
58+
t.Run("slice of struct", func(t *testing.T) {
59+
var batch []userExtended
60+
seen := make(map[uint]bool, total)
61+
result := query().FindInBatches(&batch, 2, func(tx *gorm.DB, _ int) error {
62+
for _, r := range batch {
63+
if r.ExtraFoo != 42 {
64+
return fmt.Errorf("ExtraFoo not scanned into dest: got %d", r.ExtraFoo)
65+
}
66+
if seen[r.ID] {
67+
return fmt.Errorf("row id %d visited twice (broken cursor)", r.ID)
68+
}
69+
seen[r.ID] = true
70+
}
71+
return nil
72+
})
73+
if result.Error != nil {
74+
t.Fatalf("FindInBatches returned error: %v", result.Error)
75+
}
76+
assertPagedOnce(t, seen, result.RowsAffected)
77+
})
78+
79+
t.Run("slice of pointer", func(t *testing.T) {
80+
var batch []*userExtended
81+
seen := make(map[uint]bool, total)
82+
result := query().FindInBatches(&batch, 2, func(tx *gorm.DB, _ int) error {
83+
for _, r := range batch {
84+
if seen[r.ID] {
85+
return fmt.Errorf("row id %d visited twice (broken cursor)", r.ID)
86+
}
87+
seen[r.ID] = true
88+
}
89+
return nil
90+
})
91+
if result.Error != nil {
92+
t.Fatalf("FindInBatches returned error: %v", result.Error)
93+
}
94+
assertPagedOnce(t, seen, result.RowsAffected)
95+
})
96+
}

0 commit comments

Comments
 (0)