mirror of
https://gitee.com/johng/gf
synced 2026-06-06 02:25:47 +08:00
refactor(database/gdb): simplify order and group by alias quoting (bu… (#4555)
## What this PR does Revert the auto table prefix behavior in `Order()` and `Group()` introduced by #4521. ## Why PR #4521 attempted to resolve column ambiguity in GROUP BY/ORDER BY with MySQL JOIN by automatically adding table prefixes to unqualified columns. However, this approach has issues: 1. When using `.As()` to set table alias, it uses the original table name instead of the alias, causing errors in PostgreSQL and other databases 2. The framework cannot reliably determine which table the user intends when multiple tables have the same column 3. Adds hidden behavior that users may not expect ## Example of the issue (#4554) ```go db.Model("demo_a").As("a"). LeftJoin("demo_b", "b", "a.id=b.data_id"). Order("sort").All() Expected (v2.9.5): ORDER BY "sort" Actual (v2.9.6): ORDER BY "demo_a".sort -- Wrong! Should use alias "a" or no prefix Solution Revert to v2.9.5 behavior: Order("sort") generates ORDER BY "sort" without auto-prefixing. Users should explicitly specify table prefix when needed: Order("a.sort"). Closes #4554 ``` --------- Co-authored-by: hailaz <739476267@qq.com>
This commit is contained in:
Jack Ling
@ -1,236 +0,0 @@
|
||||
// Copyright GoFrame Author(https://goframe.org). All Rights Reserved.
|
||||
//
|
||||
// This Source Code Form is subject to the terms of the MIT License.
|
||||
// If a copy of the MIT was not distributed with this file,
|
||||
// You can obtain one at https://github.com/gogf/gf.
|
||||
|
||||
package mysql_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/gogf/gf/v2/database/gdb"
|
||||
"github.com/gogf/gf/v2/frame/g"
|
||||
"github.com/gogf/gf/v2/os/gtime"
|
||||
"github.com/gogf/gf/v2/test/gtest"
|
||||
)
|
||||
|
||||
// Test_Model_Group_WithJoin tests GROUP BY with JOIN queries
|
||||
func Test_Model_Group_WithJoin(t *testing.T) {
|
||||
var (
|
||||
table1 = gtime.TimestampNanoStr() + "_user"
|
||||
table2 = gtime.TimestampNanoStr() + "_user_detail"
|
||||
)
|
||||
createInitTable(table1)
|
||||
defer dropTable(table1)
|
||||
createInitTable(table2)
|
||||
defer dropTable(table2)
|
||||
|
||||
gtest.C(t, func(t *gtest.T) {
|
||||
// Test basic GROUP BY with JOIN - unqualified column should be auto-prefixed
|
||||
// This prevents "Column 'id' in group statement is ambiguous" error
|
||||
r, err := db.Model(table1+" u").
|
||||
Fields("u.id", "u.nickname", "COUNT(*) as count").
|
||||
LeftJoin(table2+" ud", "u.id = ud.id").
|
||||
Where("u.id", g.Slice{1, 2}).
|
||||
Group("id").
|
||||
Order("u.id asc").All()
|
||||
t.AssertNil(err)
|
||||
t.Assert(len(r), 2)
|
||||
t.Assert(r[0]["id"], "1")
|
||||
t.Assert(r[1]["id"], "2")
|
||||
|
||||
// Test GROUP BY with already qualified column
|
||||
r, err = db.Model(table1+" u").
|
||||
Fields("u.id", "u.nickname", "COUNT(*) as count").
|
||||
LeftJoin(table2+" ud", "u.id = ud.id").
|
||||
Where("u.id", g.Slice{1, 2}).
|
||||
Group("u.id").
|
||||
Order("u.id asc").All()
|
||||
t.AssertNil(err)
|
||||
t.Assert(len(r), 2)
|
||||
t.Assert(r[0]["id"], "1")
|
||||
t.Assert(r[1]["id"], "2")
|
||||
|
||||
// Test GROUP BY with multiple columns
|
||||
r, err = db.Model(table1+" u").
|
||||
Fields("u.id", "u.nickname", "COUNT(*) as count").
|
||||
LeftJoin(table2+" ud", "u.id = ud.id").
|
||||
Where("u.id", g.Slice{1, 2}).
|
||||
Group("id", "nickname").
|
||||
Order("u.id asc").All()
|
||||
t.AssertNil(err)
|
||||
t.Assert(len(r), 2)
|
||||
|
||||
// Test GROUP BY with Raw expression
|
||||
r, err = db.Model(table1+" u").
|
||||
Fields("u.id", "u.nickname", "COUNT(*) as count").
|
||||
LeftJoin(table2+" ud", "u.id = ud.id").
|
||||
Where("u.id", g.Slice{1, 2}).
|
||||
Group(gdb.Raw("u.id")).
|
||||
Order("u.id asc").All()
|
||||
t.AssertNil(err)
|
||||
t.Assert(len(r), 2)
|
||||
t.Assert(r[0]["id"], "1")
|
||||
t.Assert(r[1]["id"], "2")
|
||||
|
||||
// Test GROUP BY on non-primary table should work correctly
|
||||
r, err = db.Model(table1+" u").
|
||||
Fields("ud.id", "COUNT(*) as count").
|
||||
LeftJoin(table2+" ud", "u.id = ud.id").
|
||||
Where("u.id", g.Slice{1, 2}).
|
||||
Group("ud.id").
|
||||
Order("ud.id asc").All()
|
||||
t.AssertNil(err)
|
||||
// Should have results from the joined table
|
||||
t.Assert(len(r) > 0, true)
|
||||
})
|
||||
}
|
||||
|
||||
// Test_Model_Order_WithJoin tests ORDER BY with JOIN queries
|
||||
func Test_Model_Order_WithJoin(t *testing.T) {
|
||||
var (
|
||||
table1 = gtime.TimestampNanoStr() + "_user"
|
||||
table2 = gtime.TimestampNanoStr() + "_user_detail"
|
||||
)
|
||||
createInitTable(table1)
|
||||
defer dropTable(table1)
|
||||
createInitTable(table2)
|
||||
defer dropTable(table2)
|
||||
|
||||
gtest.C(t, func(t *gtest.T) {
|
||||
// Test ORDER BY with JOIN - unqualified column should be auto-prefixed
|
||||
r, err := db.Model(table1+" u").
|
||||
LeftJoin(table2+" ud", "u.id = ud.id").
|
||||
Where("u.id", g.Slice{1, 2}).
|
||||
Order("id desc").All()
|
||||
t.AssertNil(err)
|
||||
t.Assert(len(r), 2)
|
||||
t.Assert(r[0]["id"], "2")
|
||||
t.Assert(r[1]["id"], "1")
|
||||
|
||||
// Test ORDER BY with already qualified column
|
||||
r, err = db.Model(table1+" u").
|
||||
LeftJoin(table2+" ud", "u.id = ud.id").
|
||||
Where("u.id", g.Slice{1, 2}).
|
||||
Order("u.id asc").All()
|
||||
t.AssertNil(err)
|
||||
t.Assert(len(r), 2)
|
||||
t.Assert(r[0]["id"], "1")
|
||||
t.Assert(r[1]["id"], "2")
|
||||
|
||||
// Test ORDER BY with Raw expression
|
||||
r, err = db.Model(table1+" u").
|
||||
LeftJoin(table2+" ud", "u.id = ud.id").
|
||||
Where("u.id", g.Slice{1, 2}).
|
||||
Order(gdb.Raw("u.id asc")).All()
|
||||
t.AssertNil(err)
|
||||
t.Assert(len(r), 2)
|
||||
t.Assert(r[0]["id"], "1")
|
||||
t.Assert(r[1]["id"], "2")
|
||||
|
||||
// Test multiple ORDER BY clauses with JOIN
|
||||
r, err = db.Model(table1+" u").
|
||||
LeftJoin(table2+" ud", "u.id = ud.id").
|
||||
Order("id asc").Order("nickname asc").All()
|
||||
t.AssertNil(err)
|
||||
t.Assert(len(r) > 0, true)
|
||||
|
||||
// Test ORDER BY with asc/desc keywords
|
||||
r, err = db.Model(table1+" u").
|
||||
LeftJoin(table2+" ud", "u.id = ud.id").
|
||||
Where("u.id", g.Slice{1, 2}).
|
||||
Order("id asc").All()
|
||||
t.AssertNil(err)
|
||||
t.Assert(len(r), 2)
|
||||
t.Assert(r[0]["id"], "1")
|
||||
t.Assert(r[1]["id"], "2")
|
||||
})
|
||||
}
|
||||
|
||||
// Test_Model_Group_And_Order_WithJoin tests combined GROUP BY and ORDER BY with JOINs
|
||||
func Test_Model_Group_And_Order_WithJoin(t *testing.T) {
|
||||
var (
|
||||
table1 = gtime.TimestampNanoStr() + "_user"
|
||||
table2 = gtime.TimestampNanoStr() + "_user_detail"
|
||||
)
|
||||
createInitTable(table1)
|
||||
defer dropTable(table1)
|
||||
createInitTable(table2)
|
||||
defer dropTable(table2)
|
||||
|
||||
gtest.C(t, func(t *gtest.T) {
|
||||
// Test combined GROUP BY and ORDER BY with JOIN
|
||||
r, err := db.Model(table1+" u").
|
||||
Fields("u.id", "COUNT(*) as count").
|
||||
LeftJoin(table2+" ud", "u.id = ud.id").
|
||||
Where("u.id", g.Slice{1, 2}).
|
||||
Group("id").
|
||||
Order("id desc").All()
|
||||
t.AssertNil(err)
|
||||
t.Assert(len(r), 2)
|
||||
t.Assert(r[0]["id"], "2")
|
||||
t.Assert(r[1]["id"], "1")
|
||||
|
||||
// Test with already qualified GROUP BY and unqualified ORDER BY
|
||||
r, err = db.Model(table1+" u").
|
||||
Fields("u.id", "COUNT(*) as count").
|
||||
LeftJoin(table2+" ud", "u.id = ud.id").
|
||||
Where("u.id", g.Slice{1, 2}).
|
||||
Group("u.id").
|
||||
Order("id asc").All()
|
||||
t.AssertNil(err)
|
||||
t.Assert(len(r), 2)
|
||||
t.Assert(r[0]["id"], "1")
|
||||
t.Assert(r[1]["id"], "2")
|
||||
|
||||
// Test with unqualified GROUP BY and qualified ORDER BY
|
||||
r, err = db.Model(table1+" u").
|
||||
Fields("u.id", "COUNT(*) as count").
|
||||
LeftJoin(table2+" ud", "u.id = ud.id").
|
||||
Where("u.id", g.Slice{1, 2}).
|
||||
Group("id").
|
||||
Order("u.id desc").All()
|
||||
t.AssertNil(err)
|
||||
t.Assert(len(r), 2)
|
||||
t.Assert(r[0]["id"], "2")
|
||||
t.Assert(r[1]["id"], "1")
|
||||
|
||||
// Test with both unqualified
|
||||
r, err = db.Model(table1+" u").
|
||||
Fields("u.id", "COUNT(*) as count").
|
||||
LeftJoin(table2+" ud", "u.id = ud.id").
|
||||
Where("u.id", g.Slice{1, 2}).
|
||||
Group("id").
|
||||
Order("id").All()
|
||||
t.AssertNil(err)
|
||||
t.Assert(len(r), 2)
|
||||
})
|
||||
}
|
||||
|
||||
// Test_Model_Join_Without_Alias tests JOIN without table aliases
|
||||
func Test_Model_Join_Without_Alias(t *testing.T) {
|
||||
var (
|
||||
table1 = gtime.TimestampNanoStr() + "_user"
|
||||
table2 = gtime.TimestampNanoStr() + "_user_detail"
|
||||
)
|
||||
createInitTable(table1)
|
||||
defer dropTable(table1)
|
||||
createInitTable(table2)
|
||||
defer dropTable(table2)
|
||||
|
||||
gtest.C(t, func(t *gtest.T) {
|
||||
// Test GROUP BY and ORDER BY with JOIN but without aliases
|
||||
// This should still work correctly
|
||||
r, err := db.Model(table1).
|
||||
Fields(table1+".id", "COUNT(*) as count").
|
||||
LeftJoin(table2, table1+".id = "+table2+".id").
|
||||
Where(table1+".id", g.Slice{1, 2}).
|
||||
Group(table1 + ".id").
|
||||
Order(table1 + ".id asc").All()
|
||||
t.AssertNil(err)
|
||||
t.Assert(len(r), 2)
|
||||
t.Assert(r[0]["id"], "1")
|
||||
t.Assert(r[1]["id"], "2")
|
||||
})
|
||||
}
|
||||
@ -7,7 +7,7 @@
|
||||
package gdb
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strings"
|
||||
|
||||
"github.com/gogf/gf/v2/text/gstr"
|
||||
"github.com/gogf/gf/v2/util/gconv"
|
||||
@ -30,7 +30,6 @@ func (m *Model) Order(orderBy ...any) *Model {
|
||||
core = m.db.GetCore()
|
||||
model = m.getModel()
|
||||
)
|
||||
|
||||
for _, v := range orderBy {
|
||||
if model.orderBy != "" {
|
||||
model.orderBy += ","
|
||||
@ -41,47 +40,13 @@ func (m *Model) Order(orderBy ...any) *Model {
|
||||
default:
|
||||
orderByStr := gconv.String(v)
|
||||
if gstr.Contains(orderByStr, " ") {
|
||||
// Handle "column asc/desc" format
|
||||
parts := gstr.SplitAndTrim(orderByStr, " ")
|
||||
if len(parts) >= 2 {
|
||||
columnPart := parts[0]
|
||||
orderPart := gstr.Join(parts[1:], " ")
|
||||
|
||||
// Check if column part is qualified
|
||||
if gstr.Contains(columnPart, ".") {
|
||||
model.orderBy += core.QuoteString(columnPart) + " " + orderPart
|
||||
} else {
|
||||
// Try to get the correct prefix for this field
|
||||
prefix := m.getPrefixByField(columnPart)
|
||||
if prefix != "" {
|
||||
model.orderBy += core.QuoteString(fmt.Sprintf("%s.%s", prefix, columnPart)) + " " + orderPart
|
||||
} else {
|
||||
// If we can't determine the table, just quote the field
|
||||
model.orderBy += core.QuoteWord(columnPart) + " " + orderPart
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// Fallback for complex expressions
|
||||
model.orderBy += core.QuoteString(orderByStr)
|
||||
}
|
||||
model.orderBy += core.QuoteString(orderByStr)
|
||||
} else {
|
||||
if gstr.Equal(orderByStr, "ASC") || gstr.Equal(orderByStr, "DESC") {
|
||||
model.orderBy = gstr.TrimRight(model.orderBy, ",")
|
||||
model.orderBy += " " + orderByStr
|
||||
} else {
|
||||
// Check if column is already qualified
|
||||
if gstr.Contains(orderByStr, ".") {
|
||||
model.orderBy += core.QuoteString(orderByStr)
|
||||
} else {
|
||||
// Try to get the correct prefix for this field
|
||||
prefix := m.getPrefixByField(orderByStr)
|
||||
if prefix != "" {
|
||||
model.orderBy += core.QuoteString(fmt.Sprintf("%s.%s", prefix, orderByStr))
|
||||
} else {
|
||||
// If we can't determine the table, just quote the field
|
||||
model.orderBy += core.QuoteWord(orderByStr)
|
||||
}
|
||||
}
|
||||
model.orderBy += core.QuoteWord(orderByStr)
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -113,7 +78,7 @@ func (m *Model) OrderRandom() *Model {
|
||||
}
|
||||
|
||||
// Group sets the "GROUP BY" statement for the model.
|
||||
func (m *Model) Group(groupBy ...any) *Model {
|
||||
func (m *Model) Group(groupBy ...string) *Model {
|
||||
if len(groupBy) == 0 {
|
||||
return m
|
||||
}
|
||||
@ -122,29 +87,9 @@ func (m *Model) Group(groupBy ...any) *Model {
|
||||
model = m.getModel()
|
||||
)
|
||||
|
||||
for _, v := range groupBy {
|
||||
if model.groupBy != "" {
|
||||
model.groupBy += ","
|
||||
}
|
||||
switch v.(type) {
|
||||
case Raw, *Raw:
|
||||
model.groupBy += gconv.String(v)
|
||||
default:
|
||||
groupByStr := gconv.String(v)
|
||||
if gstr.Contains(groupByStr, ".") {
|
||||
// Already qualified (e.g., "table.column")
|
||||
model.groupBy += core.QuoteString(groupByStr)
|
||||
} else {
|
||||
// Try to get the correct prefix for this field
|
||||
prefix := m.getPrefixByField(groupByStr)
|
||||
if prefix != "" {
|
||||
model.groupBy += core.QuoteString(fmt.Sprintf("%s.%s", prefix, groupByStr))
|
||||
} else {
|
||||
// If we can't determine the table, just quote the field
|
||||
model.groupBy += core.QuoteWord(groupByStr)
|
||||
}
|
||||
}
|
||||
}
|
||||
if model.groupBy != "" {
|
||||
model.groupBy += ","
|
||||
}
|
||||
model.groupBy += core.QuoteString(strings.Join(groupBy, ","))
|
||||
return model
|
||||
}
|
||||
|
||||
@ -749,110 +749,16 @@ func (m *Model) getHolderAndArgsAsSubModel(ctx context.Context) (holder string,
|
||||
return
|
||||
}
|
||||
|
||||
func (m *Model) parseTableAlias(tableInitStr string) (alias string, tableName string) {
|
||||
// Split by space to separate table from alias
|
||||
// Format can be: `table` alias or `table` AS `alias` or just table alias
|
||||
parts := gstr.SplitAndTrim(tableInitStr, " ")
|
||||
charL, charR := m.db.GetCore().GetChars()
|
||||
|
||||
if len(parts) >= 2 {
|
||||
// Check if second part is "AS" keyword
|
||||
if gstr.Equal(parts[1], "AS") && len(parts) >= 3 {
|
||||
// Format: table AS alias
|
||||
alias = gstr.Trim(parts[2], charL+charR)
|
||||
tableName = gstr.Trim(parts[0], charL+charR)
|
||||
} else if !gstr.Equal(parts[1], "AS") {
|
||||
// Format: table alias (without AS keyword)
|
||||
alias = gstr.Trim(parts[1], charL+charR)
|
||||
tableName = gstr.Trim(parts[0], charL+charR)
|
||||
} else {
|
||||
// Only table name with "AS" keyword but no alias
|
||||
tableName = gstr.Trim(parts[0], charL+charR)
|
||||
}
|
||||
} else if len(parts) == 1 {
|
||||
// No alias, use table name directly
|
||||
tableName = gstr.Trim(parts[0], charL+charR)
|
||||
}
|
||||
|
||||
return alias, tableName
|
||||
}
|
||||
|
||||
func (m *Model) getAutoPrefix() string {
|
||||
autoPrefix := ""
|
||||
if gstr.Contains(m.tables, " JOIN ") {
|
||||
// Try to get alias from tablesInit
|
||||
alias, _ := m.parseTableAlias(m.tablesInit)
|
||||
if alias != "" {
|
||||
autoPrefix = m.QuoteWord(alias)
|
||||
}
|
||||
|
||||
// Fallback to table name if alias not found
|
||||
if autoPrefix == "" {
|
||||
autoPrefix = m.QuoteWord(
|
||||
m.db.GetCore().guessPrimaryTableName(m.tablesInit),
|
||||
)
|
||||
}
|
||||
autoPrefix = m.QuoteWord(
|
||||
m.db.GetCore().guessPrimaryTableName(m.tablesInit),
|
||||
)
|
||||
}
|
||||
return autoPrefix
|
||||
}
|
||||
|
||||
// getPrefixByField attempts to find the correct table prefix for a given field name
|
||||
// by checking which table in the JOIN contains this field. It returns:
|
||||
// - The quoted table prefix/alias if the field belongs to a specific joined table
|
||||
// - An empty string if the field cannot be determined or belongs to multiple tables
|
||||
//
|
||||
// This function searches through all tables involved in the query (main table and joined tables)
|
||||
// and checks their schema to find which table contains the specified field.
|
||||
func (m *Model) getPrefixByField(fieldName string) string {
|
||||
// If there's no JOIN, no need to add prefix
|
||||
if !gstr.Contains(m.tables, " JOIN ") {
|
||||
return ""
|
||||
}
|
||||
|
||||
// Get all table names and aliases involved in the query
|
||||
var tables = make(map[string]string) // map[alias/tableName]realTableName
|
||||
|
||||
// Add main table
|
||||
alias, tableName := m.parseTableAlias(m.tablesInit)
|
||||
if alias != "" {
|
||||
tables[alias] = tableName
|
||||
} else if tableName != "" {
|
||||
tables[tableName] = tableName
|
||||
}
|
||||
|
||||
// Add joined tables from tableAliasMap
|
||||
for alias, tableName := range m.tableAliasMap {
|
||||
tables[alias] = tableName
|
||||
}
|
||||
|
||||
// Check which table contains this field
|
||||
var matchedPrefix string
|
||||
var matchCount int
|
||||
|
||||
for aliasOrTable, realTable := range tables {
|
||||
tableFields, err := m.TableFields(realTable)
|
||||
if err != nil {
|
||||
// If we can't get table fields, skip this table
|
||||
continue
|
||||
}
|
||||
|
||||
// Check if this table contains the field
|
||||
if _, exists := tableFields[fieldName]; exists {
|
||||
matchedPrefix = aliasOrTable
|
||||
matchCount++
|
||||
}
|
||||
}
|
||||
|
||||
// Only return prefix if field uniquely belongs to one table
|
||||
if matchCount == 1 {
|
||||
return m.QuoteWord(matchedPrefix)
|
||||
}
|
||||
|
||||
// If field exists in multiple tables or not found, return empty
|
||||
// to avoid ambiguity - user should specify the table explicitly
|
||||
return ""
|
||||
}
|
||||
|
||||
func (m *Model) getFieldsAsStr() string {
|
||||
var (
|
||||
fieldsStr string
|
||||
|
||||
Reference in New Issue
Block a user