Skip to content

Commit 217e6cf

Browse files
committed
fix: treat negative Limit/Page/Offset values as zero
Fixes gogf#4699 ## Changes - `Limit()`: Convert negative values to 0 for both start and limit parameters - `Page()`: Convert negative limit values to 0 (returns empty result) - `Offset()`: Convert negative offsets to 0 ## Behavior - `Limit(-1)` → `LIMIT 0` (returns empty result) - `Page(1, -10)` → `LIMIT 0` (returns empty result) - `Offset(-5)` → `OFFSET 0` (equivalent to no offset) This approach maintains API friendliness by avoiding errors while ensuring SQL validity across all database drivers. ## Tests - Added `Test_Issue4699` in `gdb_z_unit_issue_test.go` covering: - Single/dual parameter Limit with negative values - Page with negative limit parameter - Offset with negative value - Mixed positive/negative parameter combinations
1 parent c8a11f7 commit 217e6cf

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)