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

fix: limit=0 results (#5735) #5736

Merged
merged 1 commit into from Oct 7, 2022
Merged

fix: limit=0 results (#5735) #5736

merged 1 commit into from Oct 7, 2022

Conversation

robhafner
Copy link
Contributor

  • [] Do only one thing
  • [] Non breaking API changes
  • [] Tested

What did this pull request do?

User Case Description

@robhafner
Copy link
Contributor Author

Here is the link for the pull request to fix the sqlite module.

go-gorm/sqlite#114

@robhafner
Copy link
Contributor Author

Here is the link for the pull request to fix the sqlserver module.

go-gorm/sqlserver#70

@robhafner
Copy link
Contributor Author

robhafner commented Oct 3, 2022

Please let me know if there is anything else that you need from me to move this pull request forward as it is preventing some of our services from upgrading to gorm 2.

Also, given that these fixes reside in different modules it's unclear to me as to how to make the build pass.

@jinzhu jinzhu merged commit e8f48b5 into go-gorm:master Oct 7, 2022
@@ -4,7 +4,7 @@ import "strconv"

// Limit limit clause
type Limit struct {
Limit int

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change.

@@ -8,18 +8,26 @@ import (
)

func TestLimit(t *testing.T) {
limit0 := 0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test cases should not be changed unless for a reason that's good enough, this is not good enough.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

bgtill added a commit to tillfinancial/gorm-snowflake that referenced this pull request Oct 12, 2023
GORM Introduced a breaking change here: go-gorm/gorm#5736
This PR makes changes to mitigate that issue. A simmilar aproach was
taken here:

go-gorm/sqlserver#70
go-gorm/sqlite#114
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants