From bbdd4429544a580d804c87018e77442ebc533e33 Mon Sep 17 00:00:00 2001 From: Jack Ling <34231795+lingcoder@users.noreply.github.com> Date: Fri, 27 Feb 2026 16:00:53 +0800 Subject: [PATCH] fix(database/gdb): treat negative Limit/Page/Offset values as zero (#4702) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 #4699 Ref #4703 (discovered during pagination test development) --- database/gdb/gdb_model_select.go | 18 +++++++++ database/gdb/gdb_z_unit_issue_test.go | 54 +++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 database/gdb/gdb_z_unit_issue_test.go diff --git a/database/gdb/gdb_model_select.go b/database/gdb/gdb_model_select.go index 7d362ac1c..c4cfd3135 100644 --- a/database/gdb/gdb_model_select.go +++ b/database/gdb/gdb_model_select.go @@ -615,12 +615,22 @@ func (m *Model) UnionAll(unions ...*Model) *Model { // The parameter `limit` can be either one or two number, if passed two number is passed, // it then sets "LIMIT limit[0],limit[1]" statement for the model, or else it sets "LIMIT limit[0]" // statement. +// Note: Negative values are treated as zero. func (m *Model) Limit(limit ...int) *Model { model := m.getModel() switch len(limit) { case 1: + if limit[0] < 0 { + limit[0] = 0 + } model.limit = limit[0] case 2: + if limit[0] < 0 { + limit[0] = 0 + } + if limit[1] < 0 { + limit[1] = 0 + } model.start = limit[0] model.limit = limit[1] } @@ -629,8 +639,12 @@ func (m *Model) Limit(limit ...int) *Model { // Offset sets the "OFFSET" statement for the model. // It only makes sense for some databases like SQLServer, PostgreSQL, etc. +// Note: Negative values are treated as zero. func (m *Model) Offset(offset int) *Model { model := m.getModel() + if offset < 0 { + offset = 0 + } model.offset = offset return model } @@ -645,11 +659,15 @@ func (m *Model) Distinct() *Model { // Page sets the paging number for the model. // The parameter `page` is started from 1 for paging. // Note that, it differs that the Limit function starts from 0 for "LIMIT" statement. +// Note: Negative limit values are treated as zero. func (m *Model) Page(page, limit int) *Model { model := m.getModel() if page <= 0 { page = 1 } + if limit < 0 { + limit = 0 + } model.start = (page - 1) * limit model.limit = limit return model diff --git a/database/gdb/gdb_z_unit_issue_test.go b/database/gdb/gdb_z_unit_issue_test.go new file mode 100644 index 000000000..bdd6e0355 --- /dev/null +++ b/database/gdb/gdb_z_unit_issue_test.go @@ -0,0 +1,54 @@ +// 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 gdb + +import ( + "testing" + + "github.com/gogf/gf/v2/test/gtest" +) + +// Test_Issue4699 tests negative values for Limit/Page/Offset should be treated as zero. +// See https://github.com/gogf/gf/issues/4699 +func Test_Issue4699(t *testing.T) { + gtest.C(t, func(t *gtest.T) { + // Create a base model for testing + m := &Model{} + + // Test Limit with single negative parameter + m1 := m.Limit(-1) + t.AssertEQ(m1.limit, 0) + + // Test Limit with two parameters (start, limit) where both are negative + m2 := m.Limit(-10, -5) + t.AssertEQ(m2.start, 0) + t.AssertEQ(m2.limit, 0) + + // Test Limit with mixed parameters (negative start, positive limit) + m3 := m.Limit(-10, 5) + t.AssertEQ(m3.start, 0) + t.AssertEQ(m3.limit, 5) + + // Test Page with negative limit + m4 := m.Page(1, -10) + t.AssertEQ(m4.start, 0) + t.AssertEQ(m4.limit, 0) + + // Test Page with negative limit on page 2 + m5 := m.Page(2, -10) + t.AssertEQ(m5.start, 0) // (2-1) * 0 = 0 + t.AssertEQ(m5.limit, 0) + + // Test Offset with negative value + m6 := m.Offset(-5) + t.AssertEQ(m6.offset, 0) + + // Test Offset with positive value (sanity check) + m7 := m.Offset(10) + t.AssertEQ(m7.offset, 10) + }) +}