Skip to content

Commit 8e7f118

Browse files
freesmefreesme
authored
fix(contrib/drivers/mssql): fix mssql paging sql generate statement error (#3782)
1 parent 3abb947 commit 8e7f118

File tree

2 files changed

+167
-68
lines changed

2 files changed

+167
-68
lines changed

‎contrib/drivers/mssql/mssql_do_filter.go

+35-68
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ import (
1818
)
1919

2020
var (
21-
selectSqlTmp = `SELECT * FROM (SELECT TOP %d * FROM (SELECT TOP %d %s) as TMP1_ ) as TMP2_ `
21+
orderBySqlTmp = `SELECT %s %s OFFSET %d ROWS FETCH NEXT %d ROWS ONLY`
22+
withoutOrderBySqlTmp = `SELECT %s OFFSET %d ROWS FETCH NEXT %d ROWS ONLY`
2223
selectWithOrderSqlTmp = `
2324
SELECT * FROM (SELECT ROW_NUMBER() OVER (ORDER BY %s) as ROWNUMBER_, %s ) as TMP_
2425
WHERE TMP_.ROWNUMBER_ > %d AND TMP_.ROWNUMBER_ <= %d
@@ -78,89 +79,55 @@ func (d *Driver) parseSql(toBeCommittedSql string) (string, error) {
7879

7980
func (d *Driver) handleSelectSqlReplacement(toBeCommittedSql string) (newSql string, err error) {
8081
// SELECT * FROM USER WHERE ID=1 LIMIT 1
81-
match, err := gregex.MatchString(`^SELECT(.+)LIMIT 1$`, toBeCommittedSql)
82+
match, err := gregex.MatchString(`^SELECT(.+?)LIMIT\s+1$`, toBeCommittedSql)
8283
if err != nil {
8384
return "", err
8485
}
8586
if len(match) > 1 {
86-
return fmt.Sprintf(`SELECT TOP 1 %s`, match[1]), nil
87+
return fmt.Sprintf(`SELECT TOP 1 %s`, strings.TrimSpace(match[1])), nil
8788
}
8889

8990
// SELECT * FROM USER WHERE AGE>18 ORDER BY ID DESC LIMIT 100, 200
90-
patten := `^\s*(?i)(SELECT)|(LIMIT\s*(\d+)\s*,\s*(\d+))`
91-
if gregex.IsMatchString(patten, toBeCommittedSql) == false {
91+
pattern := `(?i)SELECT(.+?)(ORDER BY.+?)?\s*LIMIT\s*(\d+)(?:\s*,\s*(\d+))?`
92+
if !gregex.IsMatchString(pattern, toBeCommittedSql) {
9293
return toBeCommittedSql, nil
9394
}
94-
allMatch, err := gregex.MatchAllString(patten, toBeCommittedSql)
95+
96+
allMatch, err := gregex.MatchString(pattern, toBeCommittedSql)
9597
if err != nil {
9698
return "", err
9799
}
98-
var index = 1
99-
// LIMIT statement checks.
100-
if len(allMatch) < 2 ||
101-
(strings.HasPrefix(allMatch[index][0], "LIMIT") == false &&
102-
strings.HasPrefix(allMatch[index][0], "limit") == false) {
103-
return toBeCommittedSql, nil
104-
}
105-
if gregex.IsMatchString("((?i)SELECT)(.+)((?i)LIMIT)", toBeCommittedSql) == false {
106-
return toBeCommittedSql, nil
100+
101+
// SELECT and ORDER BY
102+
selectStr := strings.TrimSpace(allMatch[1])
103+
orderStr := ""
104+
if len(allMatch[2]) > 0 {
105+
orderStr = strings.TrimSpace(allMatch[2])
107106
}
108-
// ORDER BY statement checks.
109-
var (
110-
selectStr = ""
111-
orderStr = ""
112-
haveOrder = gregex.IsMatchString("((?i)SELECT)(.+)((?i)ORDER BY)", toBeCommittedSql)
113-
)
114-
if haveOrder {
115-
queryExpr, _ := gregex.MatchString("((?i)SELECT)(.+)((?i)ORDER BY)", toBeCommittedSql)
116-
if len(queryExpr) != 4 ||
117-
strings.EqualFold(queryExpr[1], "SELECT") == false ||
118-
strings.EqualFold(queryExpr[3], "ORDER BY") == false {
119-
return toBeCommittedSql, nil
120-
}
121-
selectStr = queryExpr[2]
122-
orderExpr, _ := gregex.MatchString("((?i)ORDER BY)(.+)((?i)LIMIT)", toBeCommittedSql)
123-
if len(orderExpr) != 4 ||
124-
strings.EqualFold(orderExpr[1], "ORDER BY") == false ||
125-
strings.EqualFold(orderExpr[3], "LIMIT") == false {
126-
return toBeCommittedSql, nil
127-
}
128-
orderStr = orderExpr[2]
107+
108+
// LIMIT and OFFSET value
109+
first, _ := strconv.Atoi(allMatch[3]) // LIMIT first parameter
110+
limit := 0
111+
if len(allMatch) > 4 && allMatch[4] != "" {
112+
limit, _ = strconv.Atoi(allMatch[4]) // LIMIT second parameter
129113
} else {
130-
queryExpr, _ := gregex.MatchString("((?i)SELECT)(.+)((?i)LIMIT)", toBeCommittedSql)
131-
if len(queryExpr) != 4 ||
132-
strings.EqualFold(queryExpr[1], "SELECT") == false ||
133-
strings.EqualFold(queryExpr[3], "LIMIT") == false {
134-
return toBeCommittedSql, nil
135-
}
136-
selectStr = queryExpr[2]
114+
limit = first
115+
first = 0
137116
}
138-
first, limit := 0, 0
139-
for i := 1; i < len(allMatch[index]); i++ {
140-
if len(strings.TrimSpace(allMatch[index][i])) == 0 {
141-
continue
142-
}
143-
if strings.HasPrefix(allMatch[index][i], "LIMIT") ||
144-
strings.HasPrefix(allMatch[index][i], "limit") {
145-
first, _ = strconv.Atoi(allMatch[index][i+1])
146-
limit, _ = strconv.Atoi(allMatch[index][i+2])
147-
break
148-
}
149-
}
150-
if haveOrder {
151-
toBeCommittedSql = fmt.Sprintf(
152-
selectWithOrderSqlTmp,
153-
orderStr, selectStr, first, first+limit,
117+
118+
if orderStr != "" {
119+
// have ORDER BY clause
120+
newSql = fmt.Sprintf(
121+
orderBySqlTmp,
122+
selectStr, orderStr, first, limit,
123+
)
124+
} else {
125+
// without ORDER BY clause
126+
newSql = fmt.Sprintf(
127+
withoutOrderBySqlTmp,
128+
selectStr, first, limit,
154129
)
155-
return toBeCommittedSql, nil
156130
}
157131

158-
if first == 0 {
159-
first = limit
160-
}
161-
toBeCommittedSql = fmt.Sprintf(
162-
selectSqlTmp,
163-
limit, first+limit, selectStr,
164-
)
165-
return toBeCommittedSql, nil
132+
return newSql, nil
166133
}

‎contrib/drivers/mssql/mssql_do_filter_test.go

+132
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
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 mssql
8+
9+
import (
10+
"context"
11+
"reflect"
12+
"testing"
13+
14+
"github.com/gogf/gf/v2/database/gdb"
15+
"github.com/gogf/gf/v2/test/gtest"
16+
)
17+
18+
func TestDriver_DoFilter(t *testing.T) {
19+
type fields struct {
20+
Core *gdb.Core
21+
}
22+
type args struct {
23+
ctx context.Context
24+
link gdb.Link
25+
sql string
26+
args []interface{}
27+
}
28+
var tests []struct {
29+
name string
30+
fields fields
31+
args args
32+
wantNewSql string
33+
wantNewArgs []interface{}
34+
wantErr bool
35+
}
36+
for _, tt := range tests {
37+
t.Run(tt.name, func(t *testing.T) {
38+
d := &Driver{
39+
Core: tt.fields.Core,
40+
}
41+
gotNewSql, gotNewArgs, err := d.DoFilter(tt.args.ctx, tt.args.link, tt.args.sql, tt.args.args)
42+
if (err != nil) != tt.wantErr {
43+
t.Errorf("DoFilter() error = %v, wantErr %v", err, tt.wantErr)
44+
return
45+
}
46+
if gotNewSql != tt.wantNewSql {
47+
t.Errorf("DoFilter() gotNewSql = %v, want %v", gotNewSql, tt.wantNewSql)
48+
}
49+
if !reflect.DeepEqual(gotNewArgs, tt.wantNewArgs) {
50+
t.Errorf("DoFilter() gotNewArgs = %v, want %v", gotNewArgs, tt.wantNewArgs)
51+
}
52+
})
53+
}
54+
}
55+
56+
func TestDriver_handleSelectSqlReplacement(t *testing.T) {
57+
58+
gtest.C(t, func(t *gtest.T) {
59+
d := &Driver{}
60+
61+
// LIMIT 1
62+
inputSql := "SELECT * FROM User WHERE ID = 1 LIMIT 1"
63+
expectedSql := "SELECT TOP 1 * FROM User WHERE ID = 1"
64+
resultSql, err := d.handleSelectSqlReplacement(inputSql)
65+
t.AssertNil(err)
66+
t.Assert(resultSql, expectedSql)
67+
68+
// LIMIT query with offset and number of rows
69+
inputSql = "SELECT * FROM User ORDER BY ID DESC LIMIT 100, 200"
70+
expectedSql = "SELECT * FROM User ORDER BY ID DESC OFFSET 100 ROWS FETCH NEXT 200 ROWS ONLY"
71+
resultSql, err = d.handleSelectSqlReplacement(inputSql)
72+
t.AssertNil(err)
73+
t.Assert(resultSql, expectedSql)
74+
75+
// Simple query with no LIMIT
76+
inputSql = "SELECT * FROM User WHERE age > 18"
77+
expectedSql = "SELECT * FROM User WHERE age > 18"
78+
resultSql, err = d.handleSelectSqlReplacement(inputSql)
79+
t.AssertNil(err)
80+
t.Assert(resultSql, expectedSql)
81+
82+
// without LIMIT
83+
inputSql = "SELECT * FROM User ORDER BY ID DESC"
84+
expectedSql = "SELECT * FROM User ORDER BY ID DESC"
85+
resultSql, err = d.handleSelectSqlReplacement(inputSql)
86+
t.AssertNil(err)
87+
t.Assert(resultSql, expectedSql)
88+
89+
// LIMIT query with only rows
90+
inputSql = "SELECT * FROM User LIMIT 50"
91+
expectedSql = "SELECT * FROM User OFFSET 0 ROWS FETCH NEXT 50 ROWS ONLY"
92+
resultSql, err = d.handleSelectSqlReplacement(inputSql)
93+
t.AssertNil(err)
94+
t.Assert(resultSql, expectedSql)
95+
96+
// LIMIT query without ORDER BY
97+
inputSql = "SELECT * FROM User LIMIT 30"
98+
expectedSql = "SELECT * FROM User OFFSET 0 ROWS FETCH NEXT 30 ROWS ONLY"
99+
resultSql, err = d.handleSelectSqlReplacement(inputSql)
100+
t.AssertNil(err)
101+
t.Assert(resultSql, expectedSql)
102+
103+
// Complex query with ORDER BY and LIMIT
104+
inputSql = "SELECT name, age FROM User WHERE age > 18 ORDER BY age ASC LIMIT 10, 5"
105+
expectedSql = "SELECT name, age FROM User WHERE age > 18 ORDER BY age ASC OFFSET 10 ROWS FETCH NEXT 5 ROWS ONLY"
106+
resultSql, err = d.handleSelectSqlReplacement(inputSql)
107+
t.AssertNil(err)
108+
t.Assert(resultSql, expectedSql)
109+
110+
// Complex conditional queries have limits
111+
inputSql = "SELECT * FROM User WHERE age > 18 AND status = 'active' LIMIT 100, 50"
112+
expectedSql = "SELECT * FROM User WHERE age > 18 AND status = 'active' OFFSET 100 ROWS FETCH NEXT 50 ROWS ONLY"
113+
resultSql, err = d.handleSelectSqlReplacement(inputSql)
114+
t.AssertNil(err)
115+
t.Assert(resultSql, expectedSql)
116+
117+
// A LIMIT query that contains subquery
118+
inputSql = "SELECT * FROM (SELECT * FROM User WHERE age > 18) AS subquery LIMIT 10"
119+
expectedSql = "SELECT * FROM (SELECT * FROM User WHERE age > 18) AS subquery OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY"
120+
resultSql, err = d.handleSelectSqlReplacement(inputSql)
121+
t.AssertNil(err)
122+
t.Assert(resultSql, expectedSql)
123+
124+
// Queries with complex ORDER BY and LIMIT
125+
inputSql = "SELECT name, age FROM User WHERE age > 18 ORDER BY age DESC, name ASC LIMIT 20, 10"
126+
expectedSql = "SELECT name, age FROM User WHERE age > 18 ORDER BY age DESC, name ASC OFFSET 20 ROWS FETCH NEXT 10 ROWS ONLY"
127+
resultSql, err = d.handleSelectSqlReplacement(inputSql)
128+
t.AssertNil(err)
129+
t.Assert(resultSql, expectedSql)
130+
131+
})
132+
}

0 commit comments

Comments
 (0)