Skip to content

Commit bbdd442

Browse files
authored
fix(database/gdb): treat negative Limit/Page/Offset values as zero (gogf#4702)
## Summary Fix bug where negative values in `Limit()`, `Page()`, and `Offset()` methods generate invalid SQL causing database errors. ## Root Cause The methods don't validate negative input: - `Limit(-1)` generates `LIMIT -1` → SQL error - `Page(1, -10)` generates `LIMIT -10` → SQL error - `Offset(-5)` generates `OFFSET -5` → SQL error ## Fix Treat all negative values as zero (safe default): **Limit() method**: ```go case 1: if limit[0] < 0 { limit[0] = 0 } case 2: if limit[0] < 0 { limit[0] = 0 } if limit[1] < 0 { limit[1] = 0 } ``` **Page() method**: ```go if limit < 0 { limit = 0 } ``` **Offset() method**: ```go if offset < 0 { offset = 0 } ``` ## Behavior Changes - `Limit(-1)` → `Limit(0)` (no limit) - `Limit(-10, -5)` → `Limit(0, 0)` (no offset, no limit) - `Page(1, -10)` → `Page(1, 0)` (no results) - `Offset(-5)` → `Offset(0)` (no offset) ## Documentation Added "Note: Negative values are treated as zero" to all three methods. ## Tests Added `Test_Issue4699` in `database/gdb/gdb_z_unit_issue_test.go` with 7 test cases: 1. Limit with single negative parameter 2. Limit with two negative parameters 3. Limit with mixed parameters (negative start, positive limit) 4. Page with negative limit 5. Page with negative limit on page 2 6. Offset with negative value 7. Offset with positive value (sanity check) ## Related Fixes gogf#4699 Ref gogf#4703 (discovered during pagination test development)
1 parent 6686bd6 commit bbdd442

2 files changed

Lines changed: 72 additions & 0 deletions

File tree

database/gdb/gdb_model_select.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,12 +615,22 @@ func (m *Model) UnionAll(unions ...*Model) *Model {
615615
// The parameter `limit` can be either one or two number, if passed two number is passed,
616616
// it then sets "LIMIT limit[0],limit[1]" statement for the model, or else it sets "LIMIT limit[0]"
617617
// statement.
618+
// Note: Negative values are treated as zero.
618619
func (m *Model) Limit(limit ...int) *Model {
619620
model := m.getModel()
620621
switch len(limit) {
621622
case 1:
623+
if limit[0] < 0 {
624+
limit[0] = 0
625+
}
622626
model.limit = limit[0]
623627
case 2:
628+
if limit[0] < 0 {
629+
limit[0] = 0
630+
}
631+
if limit[1] < 0 {
632+
limit[1] = 0
633+
}
624634
model.start = limit[0]
625635
model.limit = limit[1]
626636
}
@@ -629,8 +639,12 @@ func (m *Model) Limit(limit ...int) *Model {
629639

630640
// Offset sets the "OFFSET" statement for the model.
631641
// It only makes sense for some databases like SQLServer, PostgreSQL, etc.
642+
// Note: Negative values are treated as zero.
632643
func (m *Model) Offset(offset int) *Model {
633644
model := m.getModel()
645+
if offset < 0 {
646+
offset = 0
647+
}
634648
model.offset = offset
635649
return model
636650
}
@@ -645,11 +659,15 @@ func (m *Model) Distinct() *Model {
645659
// Page sets the paging number for the model.
646660
// The parameter `page` is started from 1 for paging.
647661
// Note that, it differs that the Limit function starts from 0 for "LIMIT" statement.
662+
// Note: Negative limit values are treated as zero.
648663
func (m *Model) Page(page, limit int) *Model {
649664
model := m.getModel()
650665
if page <= 0 {
651666
page = 1
652667
}
668+
if limit < 0 {
669+
limit = 0
670+
}
653671
model.start = (page - 1) * limit
654672
model.limit = limit
655673
return model
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
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 gdb
8+
9+
import (
10+
"testing"
11+
12+
"github.com/gogf/gf/v2/test/gtest"
13+
)
14+
15+
// Test_Issue4699 tests negative values for Limit/Page/Offset should be treated as zero.
16+
// See https://github.com/gogf/gf/issues/4699
17+
func Test_Issue4699(t *testing.T) {
18+
gtest.C(t, func(t *gtest.T) {
19+
// Create a base model for testing
20+
m := &Model{}
21+
22+
// Test Limit with single negative parameter
23+
m1 := m.Limit(-1)
24+
t.AssertEQ(m1.limit, 0)
25+
26+
// Test Limit with two parameters (start, limit) where both are negative
27+
m2 := m.Limit(-10, -5)
28+
t.AssertEQ(m2.start, 0)
29+
t.AssertEQ(m2.limit, 0)
30+
31+
// Test Limit with mixed parameters (negative start, positive limit)
32+
m3 := m.Limit(-10, 5)
33+
t.AssertEQ(m3.start, 0)
34+
t.AssertEQ(m3.limit, 5)
35+
36+
// Test Page with negative limit
37+
m4 := m.Page(1, -10)
38+
t.AssertEQ(m4.start, 0)
39+
t.AssertEQ(m4.limit, 0)
40+
41+
// Test Page with negative limit on page 2
42+
m5 := m.Page(2, -10)
43+
t.AssertEQ(m5.start, 0) // (2-1) * 0 = 0
44+
t.AssertEQ(m5.limit, 0)
45+
46+
// Test Offset with negative value
47+
m6 := m.Offset(-5)
48+
t.AssertEQ(m6.offset, 0)
49+
50+
// Test Offset with positive value (sanity check)
51+
m7 := m.Offset(10)
52+
t.AssertEQ(m7.offset, 10)
53+
})
54+
}

0 commit comments

Comments
 (0)