Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

offset and limit working unexpectedly, resulting limit 0 in compiled SQL #6188

Closed
darrenqc opened this issue Mar 27, 2023 · 7 comments
Closed
Assignees
Labels
type:with reproduction steps with reproduction steps

Comments

@darrenqc
Copy link

Environment

  • golang version: 1.18
  • gorm version: v1.24.3

Problem

We've been using gorm in our production environment. Recently, I noticed a bad case when setting limit(0).

  • Prior to v1.24.0: gorm ignores limit 0
  • Since v1.24.0: gorm adds limit 0 to the compiled SQL if we set the query condition to db.Offset(0).Limit(0)

Below are some cases I use to explain to problem.

package main

import (
	"fmt"
	"sync"

	"gorm.io/gorm"
	"gorm.io/gorm/clause"
	"gorm.io/gorm/schema"
	"gorm.io/gorm/utils/tests"
)

var db, _ = gorm.Open(tests.DummyDialector{}, nil)

type User struct {
	ID  int `gorm:"column:id" json:"id"`
	Age int `gorm:"column:age" json:"age"`
}

func main() {
	db.DryRun = true
	user, _ := schema.Parse(&User{}, &sync.Map{}, db.NamingStrategy)
	stmt := gorm.Statement{DB: db, Table: user.Table, Schema: user, Clauses: map[string]clause.Clause{}}
	db.Statement = &stmt
	s1 := db.Where("id=?", 1).Offset(0).Limit(0).Find(&User{})
	fmt.Printf("sql is %+v\n", s1.Statement.SQL.String())
	s2 := db.Where("id=?", 1).Limit(0).Offset(0).Find(&User{})
	fmt.Printf("sql is %+v\n", s2.Statement.SQL.String())
        s3 := db.Where("id=?", 1).Limit(0).Find(&User{})
	fmt.Printf("sql is %+v\n", s3.Statement.SQL.String())
}
  • s1 ouputs: sql is SELECT * FROM `users` WHERE id=? LIMIT 0
  • s2 outputs: sql is SELECT * FROM `users` WHERE id=?
  • s3 outputs: sql is SELECT * FROM `users` WHERE id=? LIMIT 0

Which change caused the problem

This commit changed the behavior in clause/limit.go, resulting in the problem.

Possible solution

Since I'm not familiar with the design priciples of gorm, I decide to report an issue instead of submitting a pull request.

I believe the expected behavior of offset and limit would be:

  • no condition: no extra clauses
  • limit(0): no extra clauses
  • offset(0): offset 0 or no extra clauses
  • limit(a): limit a
  • offset(b): offset b
  • limit(0).offset(0): offset 0 or no extra clauses
  • limit(a).offset(0): offset 0 limit a or limit a
  • limit(0).offset(b): offset b
  • limit(a).offset(b): offset b limit a

To conclude:

  • set limit when limit is not zero
  • set offset when offset is not zero
@github-actions github-actions bot added the type:missing reproduction steps missing reproduction steps label Mar 27, 2023
@github-actions
Copy link

The issue has been automatically marked as stale as it missing playground pull request link, which is important to help others understand your issue effectively and make sure the issue hasn't been fixed on latest master, checkout https://github.com/go-gorm/playground for details. it will be closed in 30 days if no further activity occurs. if you are asking question, please use the Question template, most likely your question already answered https://github.com/go-gorm/gorm/issues or described in the document https://gorm.ioSearch Before Asking

@Hanwn
Copy link
Contributor

Hanwn commented Mar 27, 2023

Yes, case mentioned by this issue (#5735 (comment) ) is :

Specifying a limit of 0 results in all rows being returned instead of none

When I execute Limit(0) firstly, then Offset(0) , it will result in sql generted without limit and offset suffix, which means all data satified condition will be returned.

I can try to make a pr to fix it.

@github-actions
Copy link

The issue has been automatically marked as stale as it missing playground pull request link, which is important to help others understand your issue effectively and make sure the issue hasn't been fixed on latest master, checkout https://github.com/go-gorm/playground for details. it will be closed in 30 days if no further activity occurs. if you are asking question, please use the Question template, most likely your question already answered https://github.com/go-gorm/gorm/issues or described in the document https://gorm.ioSearch Before Asking

@darrenqc
Copy link
Author

commit

@darrenqc
Copy link
Author

commit

@darrenqc darrenqc reopened this Mar 27, 2023
@darrenqc
Copy link
Author

Yes, case mentioned by this issue (#5735 (comment) ) is :

Specifying a limit of 0 results in all rows being returned instead of none

When I execute Limit(0) firstly, then Offset(0) , it will result in sql generted without limit and offset suffix, which means all data satified condition will be returned.

I can try to make a pr to fix it.

Thanks for attaching the related issue. Do you mean that you will try to solve the difference between Limit(0).Offset(0) and Offset(0).Limit(0) but the behavior for zero values would remain what it is now?

@github-actions
Copy link

The issue has been automatically marked as stale as it missing playground pull request link, which is important to help others understand your issue effectively and make sure the issue hasn't been fixed on latest master, checkout https://github.com/go-gorm/playground for details. it will be closed in 30 days if no further activity occurs. if you are asking question, please use the Question template, most likely your question already answered https://github.com/go-gorm/gorm/issues or described in the document https://gorm.ioSearch Before Asking

@a631807682 a631807682 added type:with reproduction steps with reproduction steps and removed type:missing reproduction steps missing reproduction steps status:stale labels Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:with reproduction steps with reproduction steps
Projects
None yet
Development

No branches or pull requests

4 participants