Repository: go-gorm/gorm
Pull request #5736 (2022-10-03 19:13:33)

fix: limit=0 results (#5735)
<!--
Make sure these boxes checked before submitting your pull request.

For significant changes, please open an issue to make an agreement on an implementation design/plan first before starting it.
-->

- [] Do only one thing
- [] Non breaking API changes
- [] Tested

### What did this pull request do?

<!--
provide a general description of the code changes in your pull request
-->

### User Case Description

<!-- Your use case -->


robhafner: Here is the link for the pull request to fix the sqlite module.

https://github.com/go-gorm/sqlite/pull/114
robhafner: Here is the link for the pull request to fix the sqlserver module.

https://github.com/go-gorm/sqlserver/pull/70
robhafner: 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 move as to how to make the build pass. 



Patch:
diff --git a/chainable_api.go b/chainable_api.go
index 68b4d1a..ab3a1a3 100644
--- a/chainable_api.go
+++ b/chainable_api.go
@@ -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
 }
 
diff --git a/clause/limit.go b/clause/limit.go
index 184f602..3ede738 100644
--- a/clause/limit.go
+++ b/clause/limit.go
@@ -4,7 +4,7 @@ import "strconv"
 
 // Limit limit clause
 type Limit struct {
-	Limit  int
+	Limit  *int
 	Offset int
 }
 
@@ -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 ")
@@ -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
 		}
 
diff --git a/clause/on_conflict.go b/clause/on_conflict.go
index 309c5fc..032bf4a 100644
--- a/clause/on_conflict.go
+++ b/clause/on_conflict.go
@@ -16,27 +16,27 @@ func (OnConflict) Name() string {
 
 // Build build onConflict clause
 func (onConflict OnConflict) Build(builder Builder) {
-	if len(onConflict.Columns) > 0 {
-		builder.WriteByte('(')
-		for idx, column := range onConflict.Columns {
-			if idx > 0 {
-				builder.WriteByte(',')
-			}
-			builder.WriteQuoted(column)
-		}
-		builder.WriteString(`) `)
-	}
-
-	if len(onConflict.TargetWhere.Exprs) > 0 {
-		builder.WriteString(" WHERE ")
-		onConflict.TargetWhere.Build(builder)
-		builder.WriteByte(' ')
-	}
-
 	if onConflict.OnConstraint != "" {
 		builder.WriteString("ON CONSTRAINT ")
 		builder.WriteString(onConflict.OnConstraint)
 		builder.WriteByte(' ')
+	} else {
+		if len(onConflict.Columns) > 0 {
+			builder.WriteByte('(')
+			for idx, column := range onConflict.Columns {
+				if idx > 0 {
+					builder.WriteByte(',')
+				}
+				builder.WriteQuoted(column)
+			}
+			builder.WriteString(`) `)
+		}
+
+		if len(onConflict.TargetWhere.Exprs) > 0 {
+			builder.WriteString(" WHERE ")
+			onConflict.TargetWhere.Build(builder)
+			builder.WriteByte(' ')
+		}
 	}
 
 	if onConflict.DoNothing {
diff --git a/finisher_api.go b/finisher_api.go
index 835a698..5516c0a 100644
--- a/finisher_api.go
+++ b/finisher_api.go
@@ -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
diff --git a/schema/relationship.go b/schema/relationship.go
index bb8aeb6..9436f28 100644
--- a/schema/relationship.go
+++ b/schema/relationship.go
@@ -403,33 +403,30 @@ func (schema *Schema) guessRelation(relation *Relationship, field *Field, cgl gu
 	case guessBelongs:
 		primarySchema, foreignSchema = relation.FieldSchema, schema
 	case guessEmbeddedBelongs:
-		if field.OwnerSchema != nil {
-			primarySchema, foreignSchema = relation.FieldSchema, field.OwnerSchema
-		} else {
+		if field.OwnerSchema == nil {
 			reguessOrErr()
 			return
 		}
+		primarySchema, foreignSchema = relation.FieldSchema, field.OwnerSchema
 	case guessHas:
 	case guessEmbeddedHas:
-		if field.OwnerSchema != nil {
-			primarySchema, foreignSchema = field.OwnerSchema, relation.FieldSchema
-		} else {
+		if field.OwnerSchema == nil {
 			reguessOrErr()
 			return
 		}
+		primarySchema, foreignSchema = field.OwnerSchema, relation.FieldSchema
 	}
 
 	if len(relation.foreignKeys) > 0 {
 		for _, foreignKey := range relation.foreignKeys {
-			if f := foreignSchema.LookUpField(foreignKey); f != nil {
-				foreignFields = append(foreignFields, f)
-			} else {
+			f := foreignSchema.LookUpField(foreignKey)
+			if f == nil {
 				reguessOrErr()
 				return
 			}
+			foreignFields = append(foreignFields, f)
 		}
 	} else {
-		var primaryFields []*Field
 		var primarySchemaName = primarySchema.Name
 		if primarySchemaName == "" {
 			primarySchemaName = relation.FieldSchema.Name
@@ -466,10 +463,11 @@ func (schema *Schema) guessRelation(relation *Relationship, field *Field, cgl gu
 		}
 	}
 
-	if len(foreignFields) == 0 {
+	switch {
+	case len(foreignFields) == 0:
 		reguessOrErr()
 		return
-	} else if len(relation.primaryKeys) > 0 {
+	case len(relation.primaryKeys) > 0:
 		for idx, primaryKey := range relation.primaryKeys {
 			if f := primarySchema.LookUpField(primaryKey); f != nil {
 				if len(primaryFields) < idx+1 {
@@ -483,7 +481,7 @@ func (schema *Schema) guessRelation(relation *Relationship, field *Field, cgl gu
 				return
 			}
 		}
-	} else if len(primaryFields) == 0 {
+	case len(primaryFields) == 0:
 		if len(foreignFields) == 1 && primarySchema.PrioritizedPrimaryField != nil {
 			primaryFields = append(primaryFields, primarySchema.PrioritizedPrimaryField)
 		} else if len(primarySchema.PrimaryFields) == len(foreignFields) {


Test patch:
diff --git a/clause/benchmarks_test.go b/clause/benchmarks_test.go
index e08677a..34d5df4 100644
--- a/clause/benchmarks_test.go
+++ b/clause/benchmarks_test.go
@@ -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{
@@ -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}}},
 		}
 
diff --git a/clause/limit_test.go b/clause/limit_test.go
index c26294a..79065ab 100644
--- a/clause/limit_test.go
+++ b/clause/limit_test.go
@@ -8,6 +8,10 @@ import (
 )
 
 func TestLimit(t *testing.T) {
+	limit0 := 0
+	limit10 := 10
+	limit50 := 50
+	limitNeg10 := -10
 	results := []struct {
 		Clauses []clause.Interface
 		Result  string
@@ -15,11 +19,15 @@ func TestLimit(t *testing.T) {
 	}{
 		{
 			[]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,
@@ -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,
 		},
 	}
diff --git a/tests/postgres_test.go b/tests/postgres_test.go
index b5b672a..f45b261 100644
--- a/tests/postgres_test.go
+++ b/tests/postgres_test.go
@@ -7,6 +7,7 @@ import (
 	"github.com/google/uuid"
 	"github.com/lib/pq"
 	"gorm.io/gorm"
+	"gorm.io/gorm/clause"
 )
 
 func TestPostgresReturningIDWhichHasStringType(t *testing.T) {
@@ -148,3 +149,53 @@ func TestMany2ManyWithDefaultValueUUID(t *testing.T) {
 		t.Errorf("Failed, got error: %v", err)
 	}
 }
+
+func TestPostgresOnConstraint(t *testing.T) {
+	if DB.Dialector.Name() != "postgres" {
+		t.Skip()
+	}
+
+	type Thing struct {
+		gorm.Model
+		SomeID  string
+		OtherID string
+		Data    string
+	}
+
+	DB.Migrator().DropTable(&Thing{})
+	DB.Migrator().CreateTable(&Thing{})
+	if err := DB.Exec("ALTER TABLE things ADD CONSTRAINT some_id_other_id_unique UNIQUE (some_id, other_id)").Error; err != nil {
+		t.Error(err)
+	}
+
+	thing := Thing{
+		SomeID:  "1234",
+		OtherID: "1234",
+		Data:    "something",
+	}
+
+	DB.Create(&thing)
+
+	thing2 := Thing{
+		SomeID:  "1234",
+		OtherID: "1234",
+		Data:    "something else",
+	}
+
+	result := DB.Clauses(clause.OnConflict{
+		OnConstraint: "some_id_other_id_unique",
+		UpdateAll:    true,
+	}).Create(&thing2)
+	if result.Error != nil {
+		t.Errorf("creating second thing: %v", result.Error)
+	}
+
+	var things []Thing
+	if err := DB.Find(&things).Error; err != nil {
+		t.Errorf("Failed, got error: %v", err)
+	}
+
+	if len(things) > 1 {
+		t.Errorf("expected 1 thing got more")
+	}
+}