fix(limit): set default limit to math.MaxInt when only offset is provided

- Ensures MySQL compatibility by always providing a LIMIT when OFFSET is used
- Updates tests to reflect new behavior
- Improves code organization and documentation
This commit is contained in:
aydinomer00 2025-01-04 22:03:13 +03:00
parent 48f9924d13
commit 3eac3cd8cb
2 changed files with 93 additions and 29 deletions

View File

@ -17,17 +17,16 @@ func (limit Limit) Name() string {
// Build constructs the LIMIT clause // Build constructs the LIMIT clause
func (limit Limit) Build(builder Builder) { func (limit Limit) Build(builder Builder) {
// If only offset is defined and limit is nil, set limit to math.MaxInt // NOT: We don't auto-set limit here. We only rely on the final struct's Limit and Offset.
if limit.Limit == nil && limit.Offset > 0 { // Any "auto offset => limit" logic is handled in MergeClause.
maxInt := math.MaxInt
limit.Limit = &maxInt
}
if limit.Limit != nil && *limit.Limit >= 0 { if limit.Limit != nil && *limit.Limit >= 0 {
builder.WriteString("LIMIT ") builder.WriteString("LIMIT ")
builder.AddVar(builder, *limit.Limit) builder.AddVar(builder, *limit.Limit)
} }
if limit.Offset > 0 { if limit.Offset > 0 {
// Add space if LIMIT was set
if limit.Limit != nil && *limit.Limit >= 0 { if limit.Limit != nil && *limit.Limit >= 0 {
builder.WriteByte(' ') builder.WriteByte(' ')
} }
@ -41,16 +40,30 @@ func (limit Limit) MergeClause(clause *Clause) {
clause.Name = "" clause.Name = ""
if v, ok := clause.Expression.(Limit); ok { if v, ok := clause.Expression.(Limit); ok {
// 1) Merge offset
if limit.Offset == 0 && v.Offset > 0 {
limit.Offset = v.Offset
} else if limit.Offset < 0 {
// Negative offset => 0
limit.Offset = 0
}
// 2) Merge limit
if (limit.Limit == nil || *limit.Limit == 0) && v.Limit != nil { if (limit.Limit == nil || *limit.Limit == 0) && v.Limit != nil {
limit.Limit = v.Limit limit.Limit = v.Limit
} }
if limit.Offset == 0 && v.Offset > 0 { // 3) If final limit is negative => treat it as nil (meaning "no limit")
limit.Offset = v.Offset if limit.Limit != nil && *limit.Limit < 0 {
} else if limit.Offset < 0 { limit.Limit = nil
limit.Offset = 0
} }
} }
// 4) If offset > 0 but limit is still nil, set limit to math.MaxInt
if limit.Offset > 0 && limit.Limit == nil {
maxInt := math.MaxInt
limit.Limit = &maxInt
}
clause.Expression = limit clause.Expression = limit
} }

View File

@ -13,68 +13,119 @@ func TestLimit(t *testing.T) {
limit10 := 10 limit10 := 10
limit50 := 50 limit50 := 50
limitNeg10 := -10 limitNeg10 := -10
results := []struct { results := []struct {
Clauses []clause.Interface Clauses []clause.Interface
Result string Result string
Vars []interface{} Vars []interface{}
}{ }{
// case #0 - limit10 offset20
{ {
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{ []clause.Interface{
Limit: &limit10, clause.Select{}, clause.From{},
Offset: 20, clause.Limit{Limit: &limit10, Offset: 20},
}}, },
"SELECT * FROM `users` LIMIT ? OFFSET ?", "SELECT * FROM `users` LIMIT ? OFFSET ?",
[]interface{}{limit10, 20}, []interface{}{limit10, 20},
}, },
// case #1 - limit0
{ {
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Limit: &limit0}}, []clause.Interface{
clause.Select{}, clause.From{},
clause.Limit{Limit: &limit0},
},
"SELECT * FROM `users` LIMIT ?", "SELECT * FROM `users` LIMIT ?",
[]interface{}{limit0}, []interface{}{limit0},
}, },
// case #2 - limit0 offset0 => offset0 is effectively ignored
{ {
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Limit: &limit0}, clause.Limit{Offset: 0}}, []clause.Interface{
clause.Select{}, clause.From{},
clause.Limit{Limit: &limit0}, clause.Limit{Offset: 0},
},
"SELECT * FROM `users` LIMIT ?", "SELECT * FROM `users` LIMIT ?",
[]interface{}{limit0}, []interface{}{limit0},
}, },
// case #3 - only offset=20
// MySQL demands limit if offset>0 => math.MaxInt
{ {
// Updated test case: only Offset is given, so now we expect math.MaxInt as LIMIT []clause.Interface{
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Offset: 20}}, clause.Select{}, clause.From{},
clause.Limit{Offset: 20},
},
"SELECT * FROM `users` LIMIT ? OFFSET ?", "SELECT * FROM `users` LIMIT ? OFFSET ?",
[]interface{}{math.MaxInt, 20}, []interface{}{math.MaxInt, 20},
}, },
// case #4 - multiple offsets (20 -> 30)
{ {
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Offset: 20}, clause.Limit{Offset: 30}}, []clause.Interface{
"SELECT * FROM `users` OFFSET ?", clause.Select{}, clause.From{},
[]interface{}{30}, clause.Limit{Offset: 20}, clause.Limit{Offset: 30},
}, },
"SELECT * FROM `users` LIMIT ? OFFSET ?",
[]interface{}{math.MaxInt, 30},
},
// case #5 - merge offset20 with limit10
{ {
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Offset: 20}, clause.Limit{Limit: &limit10}}, []clause.Interface{
clause.Select{}, clause.From{},
clause.Limit{Offset: 20}, clause.Limit{Limit: &limit10},
},
"SELECT * FROM `users` LIMIT ? OFFSET ?", "SELECT * FROM `users` LIMIT ? OFFSET ?",
[]interface{}{limit10, 20}, []interface{}{limit10, 20},
}, },
// case #6 - merge offset20->30 with limit10
{ {
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Limit: &limit10, 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 ? OFFSET ?", "SELECT * FROM `users` LIMIT ? OFFSET ?",
[]interface{}{limit10, 30}, []interface{}{limit10, 30},
}, },
// case #7 - negative offset => offset=0 => "SELECT * FROM `users` LIMIT 10"
{ {
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Limit: &limit10, 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 ?", "SELECT * FROM `users` LIMIT ?",
[]interface{}{limit10}, []interface{}{limit10},
}, },
// case #8 - negative limit => treat as nil => offset=30 => => "LIMIT ? OFFSET ?"
{ {
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Limit: &limit10, Offset: 20}, clause.Limit{Offset: 30}, clause.Limit{Limit: &limitNeg10}}, []clause.Interface{
"SELECT * FROM `users` OFFSET ?", clause.Select{}, clause.From{},
[]interface{}{30}, // Start with limit10 offset20
clause.Limit{Limit: &limit10, Offset: 20},
// Then change offset to 30
clause.Limit{Offset: 30},
// Then set limit to negative => remove limit => offset>0 => limit=math.MaxInt
clause.Limit{Limit: &limitNeg10},
}, },
"SELECT * FROM `users` LIMIT ? OFFSET ?",
[]interface{}{math.MaxInt, 30},
},
// case #9 - changing limit from 10 -> 50, offset=30
{ {
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Limit: &limit10, Offset: 20}, clause.Limit{Offset: 30}, clause.Limit{Limit: &limit50}}, []clause.Interface{
clause.Select{}, clause.From{},
clause.Limit{Limit: &limit10, Offset: 20},
clause.Limit{Offset: 30},
clause.Limit{Limit: &limit50},
},
"SELECT * FROM `users` LIMIT ? OFFSET ?", "SELECT * FROM `users` LIMIT ? OFFSET ?",
[]interface{}{limit50, 30}, []interface{}{limit50, 30},
}, },
// New test example: if only Offset is defined, Limit should become math.MaxInt // case #10 - only offset=100 => "LIMIT ? OFFSET ?", math.MaxInt, 100
{ {
[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Offset: 100}}, []clause.Interface{
clause.Select{}, clause.From{},
clause.Limit{Offset: 100},
},
"SELECT * FROM `users` LIMIT ? OFFSET ?", "SELECT * FROM `users` LIMIT ? OFFSET ?",
[]interface{}{math.MaxInt, 100}, []interface{}{math.MaxInt, 100},
}, },