Skip to content

Commit 612e545

Browse files
authored
fix(databse/gdb): use COUNT(1) if fields number is greater than 1 even when parameter useFieldForCount is true in AllAndCount/ScanAndCount (gogf#4701)
## Summary Fix bug where `AllAndCount(true)` with multiple fields generates invalid SQL `COUNT(field1, field2, ...)` causing syntax error. ## Root Cause When `useFieldForCount=true`, the COUNT query inherits the fields configuration from the model: ```go // Before (buggy code) if !useFieldForCount { countModel.fields = []any{Raw("1")} } // When useFieldForCount=true, fields remain as ["id", "nickname"] // Generates: SELECT COUNT(id, nickname) FROM table ❌ ``` ## Fix Always use `COUNT(1)` regardless of `useFieldForCount` parameter since COUNT() accepts only one argument: ```go // After (fixed code) // Always use COUNT(1) for counting, regardless of useFieldForCount. // COUNT() accepts only one argument, so we can't use multiple fields. countModel.fields = []any{Raw("1")} ``` Applied to both `AllAndCount()` and `ScanAndCount()` methods. ## Tests Added `Test_Issue4698` with 5 test cases: 1. AllAndCount(true) with multiple fields 2. AllAndCount(false) with multiple fields (baseline) 3. ScanAndCount with multiple fields 4. AllAndCount with single field 5. AllAndCount with WHERE condition All tests verify that COUNT generates valid SQL and returns correct count. ## Related Fixes gogf#4698 Ref gogf#4703 (discovered during pagination test development)
1 parent bbdd442 commit 612e545

4 files changed

Lines changed: 131 additions & 29 deletions

File tree

contrib/drivers/mysql/mysql_z_unit_issue_test.go

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1983,3 +1983,86 @@ func Test_Issue4697(t *testing.T) {
19831983
t.AssertNE(result[0]["nickname"], nil)
19841984
})
19851985
}
1986+
1987+
// https://github.com/gogf/gf/issues/4698
1988+
func Test_Issue4698(t *testing.T) {
1989+
table := createInitTable()
1990+
defer dropTable(table)
1991+
1992+
// Test 1: AllAndCount with multiple fields should generate valid COUNT SQL
1993+
gtest.C(t, func(t *gtest.T) {
1994+
result, count, err := db.Model(table).Fields("id, nickname").AllAndCount(true)
1995+
t.AssertNil(err)
1996+
t.Assert(count, TableSize)
1997+
t.Assert(len(result), TableSize)
1998+
t.AssertNE(result[0]["id"], nil)
1999+
t.AssertNE(result[0]["nickname"], nil)
2000+
t.Assert(result[0]["passport"], nil)
2001+
})
2002+
2003+
// Test 2: AllAndCount(false) with multiple fields
2004+
gtest.C(t, func(t *gtest.T) {
2005+
result, count, err := db.Model(table).Fields("id, nickname").AllAndCount(false)
2006+
t.AssertNil(err)
2007+
t.Assert(count, TableSize)
2008+
t.Assert(len(result), TableSize)
2009+
})
2010+
2011+
// Test 3: ScanAndCount with multiple fields
2012+
gtest.C(t, func(t *gtest.T) {
2013+
type User struct {
2014+
Id int
2015+
Nickname string
2016+
}
2017+
var users []User
2018+
var total int
2019+
err := db.Model(table).Fields("id, nickname").ScanAndCount(&users, &total, true)
2020+
t.AssertNil(err)
2021+
t.Assert(total, TableSize)
2022+
t.Assert(len(users), TableSize)
2023+
t.AssertGT(users[0].Id, 0)
2024+
t.AssertNE(users[0].Nickname, "")
2025+
})
2026+
2027+
// Test 4: AllAndCount with single field and useFieldForCount=true
2028+
gtest.C(t, func(t *gtest.T) {
2029+
result, count, err := db.Model(table).Fields("id").AllAndCount(true)
2030+
t.AssertNil(err)
2031+
t.Assert(count, TableSize)
2032+
t.Assert(len(result), TableSize)
2033+
t.Assert(len(result[0]), 1)
2034+
})
2035+
2036+
// Test 5: AllAndCount with Where condition
2037+
gtest.C(t, func(t *gtest.T) {
2038+
result, count, err := db.Model(table).Fields("id, nickname").Where("id<?", 5).AllAndCount(true)
2039+
t.AssertNil(err)
2040+
t.Assert(count, 4)
2041+
t.Assert(len(result), 4)
2042+
})
2043+
2044+
// Test 6: Distinct + AllAndCount(false) should use COUNT(1), not COUNT(DISTINCT 1)
2045+
gtest.C(t, func(t *gtest.T) {
2046+
result, count, err := db.Model(table).Fields("nickname").Distinct().AllAndCount(false)
2047+
t.AssertNil(err)
2048+
// COUNT(1) should return total rows, not distinct count
2049+
t.Assert(count, TableSize)
2050+
t.AssertGT(len(result), 0)
2051+
})
2052+
2053+
// Test 7: Distinct + AllAndCount(true) with single field should use COUNT(DISTINCT nickname)
2054+
gtest.C(t, func(t *gtest.T) {
2055+
_, count, err := db.Model(table).Fields("nickname").Distinct().AllAndCount(true)
2056+
t.AssertNil(err)
2057+
// COUNT(DISTINCT nickname) should return distinct count
2058+
t.Assert(count, TableSize)
2059+
})
2060+
2061+
// Test 8: Distinct + multiple fields + AllAndCount(true) should fallback to COUNT(1)
2062+
gtest.C(t, func(t *gtest.T) {
2063+
result, count, err := db.Model(table).Fields("id, nickname").Distinct().AllAndCount(true)
2064+
t.AssertNil(err)
2065+
t.Assert(count, TableSize)
2066+
t.Assert(len(result), TableSize)
2067+
})
2068+
}

contrib/drivers/sqlite/sqlite_z_unit_model_test.go

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ import (
2020
"github.com/gogf/gf/v2/container/gvar"
2121
"github.com/gogf/gf/v2/database/gdb"
2222
"github.com/gogf/gf/v2/encoding/gjson"
23-
"github.com/gogf/gf/v2/errors/gcode"
24-
"github.com/gogf/gf/v2/errors/gerror"
2523
"github.com/gogf/gf/v2/frame/g"
2624
"github.com/gogf/gf/v2/os/glog"
2725
"github.com/gogf/gf/v2/os/gtime"
@@ -698,17 +696,17 @@ func Test_Model_AllAndCount(t *testing.T) {
698696
t.Assert(all[0]["passport"], "user_1")
699697
t.Assert(count, 1)
700698
})
701-
// AllAndCount with Join return CodeDbOperationError
699+
// AllAndCount with Join and multiple fields
700+
// Regression test for #4698 - should use COUNT(1) not COUNT(field1, field2, ...)
702701
gtest.C(t, func(t *gtest.T) {
703702
all, count, err := db.Model(table).As("u1").
704703
LeftJoin(tableName2, "u2", "u2.id=u1.id").
705704
Fields("u1.passport,u1.id,u2.name,u2.age").
706705
Where("u1.id<2").
707706
AllAndCount(true)
708-
t.AssertNE(err, nil)
709-
t.AssertEQ(gerror.Code(err), gcode.CodeDbOperationError)
710-
t.Assert(count, 0)
711-
t.Assert(all, nil)
707+
t.AssertNil(err)
708+
t.Assert(len(all), 1)
709+
t.Assert(count, 1)
712710
})
713711
}
714712

@@ -1390,10 +1388,10 @@ func Test_Model_ScanAndCount(t *testing.T) {
13901388
Fields("u1.passport,u1.id,u2.name,u2.age").
13911389
Where("u1.id<2").
13921390
ScanAndCount(&users, &count, true)
1393-
t.AssertNE(err, nil)
1394-
t.Assert(gerror.Code(err), gcode.CodeDbOperationError)
1395-
t.Assert(count, 0)
1396-
t.AssertEQ(users, nil)
1391+
// Regression test for #4698 - should use COUNT(1) not COUNT(field1, field2, ...)
1392+
t.AssertNil(err)
1393+
t.Assert(len(users), 1)
1394+
t.Assert(count, 1)
13971395
})
13981396
}
13991397

contrib/drivers/sqlitecgo/sqlitecgo_z_unit_model_test.go

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ import (
2020
"github.com/gogf/gf/v2/container/gvar"
2121
"github.com/gogf/gf/v2/database/gdb"
2222
"github.com/gogf/gf/v2/encoding/gjson"
23-
"github.com/gogf/gf/v2/errors/gcode"
24-
"github.com/gogf/gf/v2/errors/gerror"
2523
"github.com/gogf/gf/v2/frame/g"
2624
"github.com/gogf/gf/v2/os/glog"
2725
"github.com/gogf/gf/v2/os/gtime"
@@ -657,17 +655,22 @@ func Test_Model_AllAndCount(t *testing.T) {
657655
t.Assert(all[0]["passport"], "user_1")
658656
t.Assert(count, 1)
659657
})
660-
// AllAndCount with Join return CodeDbOperationError
658+
// AllAndCount with Join and useFieldForCount=true
659+
// Regression test for #4698 - verifies COUNT(1) is used instead of COUNT(multiple fields)
661660
gtest.C(t, func(t *gtest.T) {
662661
all, count, err := db.Model(table).As("u1").
663662
LeftJoin(tableName2, "u2", "u2.id=u1.id").
664663
Fields("u1.passport,u1.id,u2.name,u2.age").
665664
Where("u1.id<2").
666665
AllAndCount(true)
667-
t.AssertNE(err, nil)
668-
t.AssertEQ(gerror.Code(err), gcode.CodeDbOperationError)
669-
t.Assert(count, 0)
670-
t.Assert(all, nil)
666+
t.AssertNil(err)
667+
t.Assert(len(all), 1)
668+
t.Assert(len(all[0]), 4)
669+
t.Assert(all[0]["id"], 1)
670+
t.Assert(all[0]["age"], 18)
671+
t.Assert(all[0]["name"], "table2_1")
672+
t.Assert(all[0]["passport"], "user_1")
673+
t.Assert(count, 1)
671674
})
672675
}
673676

@@ -1334,7 +1337,8 @@ func Test_Model_ScanAndCount(t *testing.T) {
13341337
t.Assert(count, 1)
13351338
t.AssertEQ(users[0].Name, "table2_1")
13361339
})
1337-
// ScanAndCount with join return CodeDbOperationError
1340+
// ScanAndCount with join and useFieldForCount=true
1341+
// Regression test for #4698 - verifies COUNT(1) is used instead of COUNT(multiple fields)
13381342
gtest.C(t, func(t *gtest.T) {
13391343
type User struct {
13401344
Id int
@@ -1349,10 +1353,13 @@ func Test_Model_ScanAndCount(t *testing.T) {
13491353
Fields("u1.passport,u1.id,u2.name,u2.age").
13501354
Where("u1.id<2").
13511355
ScanAndCount(&users, &count, true)
1352-
t.AssertNE(err, nil)
1353-
t.Assert(gerror.Code(err), gcode.CodeDbOperationError)
1354-
t.Assert(count, 0)
1355-
t.AssertEQ(users, nil)
1356+
t.AssertNil(err)
1357+
t.Assert(len(users), 1)
1358+
t.Assert(count, 1)
1359+
t.Assert(users[0].Id, 1)
1360+
t.Assert(users[0].Age, 18)
1361+
t.Assert(users[0].Name, "table2_1")
1362+
t.Assert(users[0].Passport, "user_1")
13561363
})
13571364
}
13581365

database/gdb/gdb_model_select.go

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,16 @@ func (m *Model) AllAndCount(useFieldForCount bool) (result Result, totalCount in
5252
// Clone the model for counting
5353
countModel := m.Clone()
5454

55-
// If useFieldForCount is false, set the fields to a constant value of 1 for counting
56-
if !useFieldForCount {
57-
countModel.fields = []any{Raw("1")}
55+
// Decide how to build the COUNT() expression:
56+
// - If caller explicitly wants to use the single field expression for counting,
57+
// honor it (e.g. Fields("DISTINCT col") with useFieldForCount = true).
58+
// - Otherwise, clear fields to let Count() use its default COUNT(1),
59+
// avoiding invalid COUNT(field1, field2, ...) with multiple fields,
60+
// or incorrect COUNT(DISTINCT 1) when Distinct() is set.
61+
if useFieldForCount && len(m.fields) == 1 {
62+
countModel.fields = m.fields
63+
} else {
64+
countModel.fields = nil
5865
}
5966
if len(m.pageCacheOption) > 0 {
6067
countModel = countModel.Cache(m.pageCacheOption[0])
@@ -341,9 +348,16 @@ func (m *Model) Scan(pointer any, where ...any) error {
341348
func (m *Model) ScanAndCount(pointer any, totalCount *int, useFieldForCount bool) (err error) {
342349
// support Fields with *, example: .Fields("a.*, b.name"). Count sql is select count(1) from xxx
343350
countModel := m.Clone()
344-
// If useFieldForCount is false, set the fields to a constant value of 1 for counting
345-
if !useFieldForCount {
346-
countModel.fields = []any{Raw("1")}
351+
// Decide how to build the COUNT() expression:
352+
// - If caller explicitly wants to use the single field expression for counting,
353+
// honor it (e.g. Fields("DISTINCT col") with useFieldForCount = true).
354+
// - Otherwise, clear fields to let Count() use its default COUNT(1),
355+
// avoiding invalid COUNT(field1, field2, ...) with multiple fields,
356+
// or incorrect COUNT(DISTINCT 1) when Distinct() is set.
357+
if useFieldForCount && len(m.fields) == 1 {
358+
countModel.fields = m.fields
359+
} else {
360+
countModel.fields = nil
347361
}
348362
if len(m.pageCacheOption) > 0 {
349363
countModel = countModel.Cache(m.pageCacheOption[0])

0 commit comments

Comments
 (0)