Skip to content

Commit fe8ba5e

Browse files
ljluestchailaz
ljluestc
and
hailaz
authored
fix(database/gdb): Resolve column ambiguity in GROUP BY/ORDER BY with MySQL JOIN (#4521)
When using JOIN queries in MySQL with the `Group()` method, column names in GROUP BY clauses become ambiguous if multiple tables contain columns with the same name (commonly `id`). This results in MySQL errors like "Column 'id' in group statement is ambiguous". **Example Issue:** ```go model := t.Ctx(ctx).Fields("t_inf_job.*, t_inf_job_attr.*"). LeftJoin("t_inf_job_attr", "t_inf_job.id = t_inf_job_attr.job_id"). Where(t.Columns().Deleted, 0) // This would fail with "Column 'id' in group statement is ambiguous" err = model.Group(t.Columns().Id).Scan(&jobs) ``` ### **Key Changes** 1. **Modified function signature**: `Group(groupBy ...string)` → `Group(groupBy ...any)` to support Raw SQL expressions 2. **Auto-prefixing logic**: When JOINs are detected (by checking for " JOIN " in the tables string), unqualified column names are automatically prefixed with the primary table name 3. **Preserved existing behavior**: Already qualified columns (containing ".") and Raw expressions are handled as before 4. **Added comprehensive test**: `Test_Model_Group_WithJoin` verifies the fix works correctly with JOIN queries --------- Co-authored-by: hailaz <[email protected]>
1 parent 54b7c24 commit fe8ba5e

File tree

4 files changed

+396
-10
lines changed

4 files changed

+396
-10
lines changed

‎contrib/drivers/mysql/mysql_z_unit_feature_model_join_group_order_test.go‎

Lines changed: 236 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
// Copyright GoFrame Author(https://goframe.org). All Rights Reserved.
2+
//
3+
// This Source Code Form is subject to the terms of the MIT License.
4+
// If a copy of the MIT was not distributed with this file,
5+
// You can obtain one at https://github.com/gogf/gf.
6+
7+
package mysql_test
8+
9+
import (
10+
"testing"
11+
12+
"github.com/gogf/gf/v2/database/gdb"
13+
"github.com/gogf/gf/v2/frame/g"
14+
"github.com/gogf/gf/v2/os/gtime"
15+
"github.com/gogf/gf/v2/test/gtest"
16+
)
17+
18+
// Test_Model_Group_WithJoin tests GROUP BY with JOIN queries
19+
func Test_Model_Group_WithJoin(t *testing.T) {
20+
var (
21+
table1 = gtime.TimestampNanoStr() + "_user"
22+
table2 = gtime.TimestampNanoStr() + "_user_detail"
23+
)
24+
createInitTable(table1)
25+
defer dropTable(table1)
26+
createInitTable(table2)
27+
defer dropTable(table2)
28+
29+
gtest.C(t, func(t *gtest.T) {
30+
// Test basic GROUP BY with JOIN - unqualified column should be auto-prefixed
31+
// This prevents "Column 'id' in group statement is ambiguous" error
32+
r, err := db.Model(table1+" u").
33+
Fields("u.id", "u.nickname", "COUNT(*) as count").
34+
LeftJoin(table2+" ud", "u.id = ud.id").
35+
Where("u.id", g.Slice{1, 2}).
36+
Group("id").
37+
Order("u.id asc").All()
38+
t.AssertNil(err)
39+
t.Assert(len(r), 2)
40+
t.Assert(r[0]["id"], "1")
41+
t.Assert(r[1]["id"], "2")
42+
43+
// Test GROUP BY with already qualified column
44+
r, err = db.Model(table1+" u").
45+
Fields("u.id", "u.nickname", "COUNT(*) as count").
46+
LeftJoin(table2+" ud", "u.id = ud.id").
47+
Where("u.id", g.Slice{1, 2}).
48+
Group("u.id").
49+
Order("u.id asc").All()
50+
t.AssertNil(err)
51+
t.Assert(len(r), 2)
52+
t.Assert(r[0]["id"], "1")
53+
t.Assert(r[1]["id"], "2")
54+
55+
// Test GROUP BY with multiple columns
56+
r, err = db.Model(table1+" u").
57+
Fields("u.id", "u.nickname", "COUNT(*) as count").
58+
LeftJoin(table2+" ud", "u.id = ud.id").
59+
Where("u.id", g.Slice{1, 2}).
60+
Group("id", "nickname").
61+
Order("u.id asc").All()
62+
t.AssertNil(err)
63+
t.Assert(len(r), 2)
64+
65+
// Test GROUP BY with Raw expression
66+
r, err = db.Model(table1+" u").
67+
Fields("u.id", "u.nickname", "COUNT(*) as count").
68+
LeftJoin(table2+" ud", "u.id = ud.id").
69+
Where("u.id", g.Slice{1, 2}).
70+
Group(gdb.Raw("u.id")).
71+
Order("u.id asc").All()
72+
t.AssertNil(err)
73+
t.Assert(len(r), 2)
74+
t.Assert(r[0]["id"], "1")
75+
t.Assert(r[1]["id"], "2")
76+
77+
// Test GROUP BY on non-primary table should work correctly
78+
r, err = db.Model(table1+" u").
79+
Fields("ud.id", "COUNT(*) as count").
80+
LeftJoin(table2+" ud", "u.id = ud.id").
81+
Where("u.id", g.Slice{1, 2}).
82+
Group("ud.id").
83+
Order("ud.id asc").All()
84+
t.AssertNil(err)
85+
// Should have results from the joined table
86+
t.Assert(len(r) > 0, true)
87+
})
88+
}
89+
90+
// Test_Model_Order_WithJoin tests ORDER BY with JOIN queries
91+
func Test_Model_Order_WithJoin(t *testing.T) {
92+
var (
93+
table1 = gtime.TimestampNanoStr() + "_user"
94+
table2 = gtime.TimestampNanoStr() + "_user_detail"
95+
)
96+
createInitTable(table1)
97+
defer dropTable(table1)
98+
createInitTable(table2)
99+
defer dropTable(table2)
100+
101+
gtest.C(t, func(t *gtest.T) {
102+
// Test ORDER BY with JOIN - unqualified column should be auto-prefixed
103+
r, err := db.Model(table1+" u").
104+
LeftJoin(table2+" ud", "u.id = ud.id").
105+
Where("u.id", g.Slice{1, 2}).
106+
Order("id desc").All()
107+
t.AssertNil(err)
108+
t.Assert(len(r), 2)
109+
t.Assert(r[0]["id"], "2")
110+
t.Assert(r[1]["id"], "1")
111+
112+
// Test ORDER BY with already qualified column
113+
r, err = db.Model(table1+" u").
114+
LeftJoin(table2+" ud", "u.id = ud.id").
115+
Where("u.id", g.Slice{1, 2}).
116+
Order("u.id asc").All()
117+
t.AssertNil(err)
118+
t.Assert(len(r), 2)
119+
t.Assert(r[0]["id"], "1")
120+
t.Assert(r[1]["id"], "2")
121+
122+
// Test ORDER BY with Raw expression
123+
r, err = db.Model(table1+" u").
124+
LeftJoin(table2+" ud", "u.id = ud.id").
125+
Where("u.id", g.Slice{1, 2}).
126+
Order(gdb.Raw("u.id asc")).All()
127+
t.AssertNil(err)
128+
t.Assert(len(r), 2)
129+
t.Assert(r[0]["id"], "1")
130+
t.Assert(r[1]["id"], "2")
131+
132+
// Test multiple ORDER BY clauses with JOIN
133+
r, err = db.Model(table1+" u").
134+
LeftJoin(table2+" ud", "u.id = ud.id").
135+
Order("id asc").Order("nickname asc").All()
136+
t.AssertNil(err)
137+
t.Assert(len(r) > 0, true)
138+
139+
// Test ORDER BY with asc/desc keywords
140+
r, err = db.Model(table1+" u").
141+
LeftJoin(table2+" ud", "u.id = ud.id").
142+
Where("u.id", g.Slice{1, 2}).
143+
Order("id asc").All()
144+
t.AssertNil(err)
145+
t.Assert(len(r), 2)
146+
t.Assert(r[0]["id"], "1")
147+
t.Assert(r[1]["id"], "2")
148+
})
149+
}
150+
151+
// Test_Model_Group_And_Order_WithJoin tests combined GROUP BY and ORDER BY with JOINs
152+
func Test_Model_Group_And_Order_WithJoin(t *testing.T) {
153+
var (
154+
table1 = gtime.TimestampNanoStr() + "_user"
155+
table2 = gtime.TimestampNanoStr() + "_user_detail"
156+
)
157+
createInitTable(table1)
158+
defer dropTable(table1)
159+
createInitTable(table2)
160+
defer dropTable(table2)
161+
162+
gtest.C(t, func(t *gtest.T) {
163+
// Test combined GROUP BY and ORDER BY with JOIN
164+
r, err := db.Model(table1+" u").
165+
Fields("u.id", "COUNT(*) as count").
166+
LeftJoin(table2+" ud", "u.id = ud.id").
167+
Where("u.id", g.Slice{1, 2}).
168+
Group("id").
169+
Order("id desc").All()
170+
t.AssertNil(err)
171+
t.Assert(len(r), 2)
172+
t.Assert(r[0]["id"], "2")
173+
t.Assert(r[1]["id"], "1")
174+
175+
// Test with already qualified GROUP BY and unqualified ORDER BY
176+
r, err = db.Model(table1+" u").
177+
Fields("u.id", "COUNT(*) as count").
178+
LeftJoin(table2+" ud", "u.id = ud.id").
179+
Where("u.id", g.Slice{1, 2}).
180+
Group("u.id").
181+
Order("id asc").All()
182+
t.AssertNil(err)
183+
t.Assert(len(r), 2)
184+
t.Assert(r[0]["id"], "1")
185+
t.Assert(r[1]["id"], "2")
186+
187+
// Test with unqualified GROUP BY and qualified ORDER BY
188+
r, err = db.Model(table1+" u").
189+
Fields("u.id", "COUNT(*) as count").
190+
LeftJoin(table2+" ud", "u.id = ud.id").
191+
Where("u.id", g.Slice{1, 2}).
192+
Group("id").
193+
Order("u.id desc").All()
194+
t.AssertNil(err)
195+
t.Assert(len(r), 2)
196+
t.Assert(r[0]["id"], "2")
197+
t.Assert(r[1]["id"], "1")
198+
199+
// Test with both unqualified
200+
r, err = db.Model(table1+" u").
201+
Fields("u.id", "COUNT(*) as count").
202+
LeftJoin(table2+" ud", "u.id = ud.id").
203+
Where("u.id", g.Slice{1, 2}).
204+
Group("id").
205+
Order("id").All()
206+
t.AssertNil(err)
207+
t.Assert(len(r), 2)
208+
})
209+
}
210+
211+
// Test_Model_Join_Without_Alias tests JOIN without table aliases
212+
func Test_Model_Join_Without_Alias(t *testing.T) {
213+
var (
214+
table1 = gtime.TimestampNanoStr() + "_user"
215+
table2 = gtime.TimestampNanoStr() + "_user_detail"
216+
)
217+
createInitTable(table1)
218+
defer dropTable(table1)
219+
createInitTable(table2)
220+
defer dropTable(table2)
221+
222+
gtest.C(t, func(t *gtest.T) {
223+
// Test GROUP BY and ORDER BY with JOIN but without aliases
224+
// This should still work correctly
225+
r, err := db.Model(table1).
226+
Fields(table1+".id", "COUNT(*) as count").
227+
LeftJoin(table2, table1+".id = "+table2+".id").
228+
Where(table1+".id", g.Slice{1, 2}).
229+
Group(table1 + ".id").
230+
Order(table1 + ".id asc").All()
231+
t.AssertNil(err)
232+
t.Assert(len(r), 2)
233+
t.Assert(r[0]["id"], "1")
234+
t.Assert(r[1]["id"], "2")
235+
})
236+
}

‎contrib/drivers/mysql/mysql_z_unit_model_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3783,6 +3783,7 @@ func Test_Model_FixGdbJoin(t *testing.T) {
37833783
FieldsPrefix(`rules_template`, "name").
37843784
FieldsPrefix(`common_resource`, `src_instance_id`, "database_kind", "source_type", "ip", "port")
37853785
all, err := orm.OrderAsc("src_instance_id").All()
3786+
t.Assert(err, nil)
37863787
t.Assert(len(all), 4)
37873788
t.Assert(all[0]["pay_mode"], 1)
37883789
t.Assert(all[0]["src_instance_id"], 2)

‎database/gdb/gdb_model_order_group.go‎

Lines changed: 62 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
package gdb
88

99
import (
10-
"strings"
10+
"fmt"
1111

1212
"github.com/gogf/gf/v2/text/gstr"
1313
"github.com/gogf/gf/v2/util/gconv"
@@ -30,6 +30,7 @@ func (m *Model) Order(orderBy ...any) *Model {
3030
core = m.db.GetCore()
3131
model = m.getModel()
3232
)
33+
3334
for _, v := range orderBy {
3435
if model.orderBy != "" {
3536
model.orderBy += ","
@@ -40,13 +41,47 @@ func (m *Model) Order(orderBy ...any) *Model {
4041
default:
4142
orderByStr := gconv.String(v)
4243
if gstr.Contains(orderByStr, " ") {
43-
model.orderBy += core.QuoteString(orderByStr)
44+
// Handle "column asc/desc" format
45+
parts := gstr.SplitAndTrim(orderByStr, " ")
46+
if len(parts) >= 2 {
47+
columnPart := parts[0]
48+
orderPart := gstr.Join(parts[1:], " ")
49+
50+
// Check if column part is qualified
51+
if gstr.Contains(columnPart, ".") {
52+
model.orderBy += core.QuoteString(columnPart) + " " + orderPart
53+
} else {
54+
// Try to get the correct prefix for this field
55+
prefix := m.getPrefixByField(columnPart)
56+
if prefix != "" {
57+
model.orderBy += core.QuoteString(fmt.Sprintf("%s.%s", prefix, columnPart)) + " " + orderPart
58+
} else {
59+
// If we can't determine the table, just quote the field
60+
model.orderBy += core.QuoteWord(columnPart) + " " + orderPart
61+
}
62+
}
63+
} else {
64+
// Fallback for complex expressions
65+
model.orderBy += core.QuoteString(orderByStr)
66+
}
4467
} else {
4568
if gstr.Equal(orderByStr, "ASC") || gstr.Equal(orderByStr, "DESC") {
4669
model.orderBy = gstr.TrimRight(model.orderBy, ",")
4770
model.orderBy += " " + orderByStr
4871
} else {
49-
model.orderBy += core.QuoteWord(orderByStr)
72+
// Check if column is already qualified
73+
if gstr.Contains(orderByStr, ".") {
74+
model.orderBy += core.QuoteString(orderByStr)
75+
} else {
76+
// Try to get the correct prefix for this field
77+
prefix := m.getPrefixByField(orderByStr)
78+
if prefix != "" {
79+
model.orderBy += core.QuoteString(fmt.Sprintf("%s.%s", prefix, orderByStr))
80+
} else {
81+
// If we can't determine the table, just quote the field
82+
model.orderBy += core.QuoteWord(orderByStr)
83+
}
84+
}
5085
}
5186
}
5287
}
@@ -78,7 +113,7 @@ func (m *Model) OrderRandom() *Model {
78113
}
79114

80115
// Group sets the "GROUP BY" statement for the model.
81-
func (m *Model) Group(groupBy ...string) *Model {
116+
func (m *Model) Group(groupBy ...any) *Model {
82117
if len(groupBy) == 0 {
83118
return m
84119
}
@@ -87,9 +122,29 @@ func (m *Model) Group(groupBy ...string) *Model {
87122
model = m.getModel()
88123
)
89124

90-
if model.groupBy != "" {
91-
model.groupBy += ","
125+
for _, v := range groupBy {
126+
if model.groupBy != "" {
127+
model.groupBy += ","
128+
}
129+
switch v.(type) {
130+
case Raw, *Raw:
131+
model.groupBy += gconv.String(v)
132+
default:
133+
groupByStr := gconv.String(v)
134+
if gstr.Contains(groupByStr, ".") {
135+
// Already qualified (e.g., "table.column")
136+
model.groupBy += core.QuoteString(groupByStr)
137+
} else {
138+
// Try to get the correct prefix for this field
139+
prefix := m.getPrefixByField(groupByStr)
140+
if prefix != "" {
141+
model.groupBy += core.QuoteString(fmt.Sprintf("%s.%s", prefix, groupByStr))
142+
} else {
143+
// If we can't determine the table, just quote the field
144+
model.groupBy += core.QuoteWord(groupByStr)
145+
}
146+
}
147+
}
92148
}
93-
model.groupBy += core.QuoteString(strings.Join(groupBy, ","))
94149
return model
95150
}

0 commit comments

Comments
 (0)