Skip to content

Commit

Permalink
fix: limit=0 results (#5735) (#5736)
Browse files Browse the repository at this point in the history
  • Loading branch information
robhafner committed Oct 7, 2022
1 parent 4b22a55 commit e8f48b5
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 14 deletions.
2 changes: 1 addition & 1 deletion chainable_api.go
Expand Up @@ -244,7 +244,7 @@ func (db *DB) Order(value interface{}) (tx *DB) {
// Limit specify the number of records to be retrieved
func (db *DB) Limit(limit int) (tx *DB) {
tx = db.getInstance()
tx.Statement.AddClause(clause.Limit{Limit: limit})
tx.Statement.AddClause(clause.Limit{Limit: &limit})
return
}

Expand Down
3 changes: 2 additions & 1 deletion clause/benchmarks_test.go
Expand Up @@ -29,6 +29,7 @@ func BenchmarkSelect(b *testing.B) {
func BenchmarkComplexSelect(b *testing.B) {
user, _ := schema.Parse(&tests.User{}, &sync.Map{}, db.NamingStrategy)

limit10 := 10
for i := 0; i < b.N; i++ {
stmt := gorm.Statement{DB: db, Table: user.Table, Schema: user, Clauses: map[string]clause.Clause{}}
clauses := []clause.Interface{
Expand All @@ -43,7 +44,7 @@ func BenchmarkComplexSelect(b *testing.B) {
clause.Or(clause.Gt{Column: "score", Value: 100}, clause.Like{Column: "name", Value: "%linus%"}),
}},
clause.GroupBy{Columns: []clause.Column{{Name: "role"}}, Having: []clause.Expression{clause.Eq{"role", "admin"}}},
clause.Limit{Limit: 10, Offset: 20},
clause.Limit{Limit: &limit10, Offset: 20},
clause.OrderBy{Columns: []clause.OrderByColumn{{Column: clause.PrimaryColumn, Desc: true}}},
}

Expand Down
10 changes: 5 additions & 5 deletions clause/limit.go
Expand Up @@ -4,7 +4,7 @@ import "strconv"

// Limit limit clause
type Limit struct {
Limit int
Limit *int
Offset int
}

Expand All @@ -15,12 +15,12 @@ func (limit Limit) Name() string {

// Build build where clause
func (limit Limit) Build(builder Builder) {
if limit.Limit > 0 {
if limit.Limit != nil && *limit.Limit >= 0 {
builder.WriteString("LIMIT ")
builder.WriteString(strconv.Itoa(limit.Limit))
builder.WriteString(strconv.Itoa(*limit.Limit))
}
if limit.Offset > 0 {
if limit.Limit > 0 {
if limit.Limit != nil && *limit.Limit >= 0 {
builder.WriteByte(' ')
}
builder.WriteString("OFFSET ")
Expand All @@ -33,7 +33,7 @@ func (limit Limit) MergeClause(clause *Clause) {
clause.Name = ""

if v, ok := clause.Expression.(Limit); ok {
if limit.Limit == 0 && v.Limit != 0 {
if (limit.Limit == nil || *limit.Limit == 0) && (v.Limit != nil && *v.Limit != 0) {
limit.Limit = v.Limit
}

Expand Down
20 changes: 14 additions & 6 deletions clause/limit_test.go
Expand Up @@ -8,18 +8,26 @@ import (
)

func TestLimit(t *testing.T) {
limit0 := 0
limit10 := 10
limit50 := 50
limitNeg10 := -10
results := []struct {
Clauses []clause.Interface
Result string
Vars []interface{}
}{
{
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{
Limit: 10,
Limit: &limit10,
Offset: 20,
}},
"SELECT * FROM `users` LIMIT 10 OFFSET 20", nil,
},
{
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Limit: &limit0}},
"SELECT * FROM `users` LIMIT 0", nil,
},
{
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Offset: 20}},
"SELECT * FROM `users` OFFSET 20", nil,
Expand All @@ -29,23 +37,23 @@ func TestLimit(t *testing.T) {
"SELECT * FROM `users` OFFSET 30", nil,
},
{
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Offset: 20}, clause.Limit{Limit: 10}},
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Offset: 20}, clause.Limit{Limit: &limit10}},
"SELECT * FROM `users` LIMIT 10 OFFSET 20", nil,
},
{
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Limit: 10, Offset: 20}, clause.Limit{Offset: 30}},
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Limit: &limit10, Offset: 20}, clause.Limit{Offset: 30}},
"SELECT * FROM `users` LIMIT 10 OFFSET 30", nil,
},
{
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Limit: 10, Offset: 20}, clause.Limit{Offset: 30}, clause.Limit{Offset: -10}},
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Limit: &limit10, Offset: 20}, clause.Limit{Offset: 30}, clause.Limit{Offset: -10}},
"SELECT * FROM `users` LIMIT 10", nil,
},
{
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Limit: 10, Offset: 20}, clause.Limit{Offset: 30}, clause.Limit{Limit: -10}},
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Limit: &limit10, Offset: 20}, clause.Limit{Offset: 30}, clause.Limit{Limit: &limitNeg10}},
"SELECT * FROM `users` OFFSET 30", nil,
},
{
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Limit: 10, Offset: 20}, clause.Limit{Offset: 30}, clause.Limit{Limit: 50}},
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Limit: &limit10, Offset: 20}, clause.Limit{Offset: 30}, clause.Limit{Limit: &limit50}},
"SELECT * FROM `users` LIMIT 50 OFFSET 30", nil,
},
}
Expand Down
4 changes: 3 additions & 1 deletion finisher_api.go
Expand Up @@ -185,7 +185,9 @@ func (db *DB) FindInBatches(dest interface{}, batchSize int, fc func(tx *DB, bat
var totalSize int
if c, ok := tx.Statement.Clauses["LIMIT"]; ok {
if limit, ok := c.Expression.(clause.Limit); ok {
totalSize = limit.Limit
if limit.Limit != nil {
totalSize = *limit.Limit
}

if totalSize > 0 && batchSize > totalSize {
batchSize = totalSize
Expand Down

0 comments on commit e8f48b5

Please sign in to comment.