Repository: go-gorm/gorm
Pull request #6206 (2023-03-31 23:08:20)

fix: many2many association with duplicate belongs to elem
<!--
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.
-->

- [x] Do only one thing
- [x] Non breaking API changes
- [x] Tested

### What did this pull request do?
Extends https://github.com/go-gorm/gorm/pull/5473 to `many2many` models with `belongs to` associations.
<!--
provide a general description of the code changes in your pull request
-->

### User Case Description
Using the following models that have `many2many` and `belongs to` associations:
```
type Application struct {
	ID        string `gorm:"primarykey"`
	Name      string
	Resources []Resource 
}

type Resource struct {
	ID       string `gorm:"primarykey"`
	Name     string
	OwnerId  string
	Owner    Owner 
}

type Owner struct {
	ID   string `gorm:"primarykey"`
	Name string
}
```
In cases where two resources share the same `Owner`, attempting to update an `Application` using `FullSaveAssociations: true` will result in an error. GORM tries to insert the same `Owner` multiple times in a single INSERT command: 
```
INSERT INTO "owner" ("id","name") VALUES ('1','owner-1'),('1','owner-1') ON CONFLICT ("id") DO UPDATE SET "name"="excluded"."name"
ERROR: ON CONFLICT DO UPDATE command cannot affect row a second time (SQLSTATE 21000)
```
After the fix, GORM will identify and insert only the unique owner(s):
```
INSERT INTO "owner" ("id","name") VALUES ('1','owner-1') ON CONFLICT ("id") DO UPDATE SET "name"="excluded"."name"
```
<!-- Your use case -->



Patch:
diff --git a/.github/workflows/invalid_question.yml b/.github/workflows/invalid_question.yml
index 77b26ab..fbebfc1 100644
--- a/.github/workflows/invalid_question.yml
+++ b/.github/workflows/invalid_question.yml
@@ -16,7 +16,7 @@ jobs:
       ACTIONS_STEP_DEBUG: true
     steps:
     - name: Close Stale Issues
-      uses: actions/stale@v7
+      uses: actions/stale@v8
       with:
         repo-token: ${{ secrets.GITHUB_TOKEN }}
         stale-issue-message: "This issue has been marked as invalid question, please give more information by following the `Question` template, if you believe there is a bug of GORM, please create a pull request that could reproduce the issue on [https://github.com/go-gorm/playground](https://github.com/go-gorm/playground), the issue will be closed in 30 days if no further activity occurs. most likely your question already answered https://github.com/go-gorm/gorm/issues or described in the document https://gorm.io ✨ [Search Before Asking](https://stackoverflow.com/help/how-to-ask) ✨"
diff --git a/.github/workflows/missing_playground.yml b/.github/workflows/missing_playground.yml
index 1efa361..b23a5bf 100644
--- a/.github/workflows/missing_playground.yml
+++ b/.github/workflows/missing_playground.yml
@@ -16,7 +16,7 @@ jobs:
       ACTIONS_STEP_DEBUG: true
     steps:
     - name: Close Stale Issues
-      uses: actions/stale@v7
+      uses: actions/stale@v8
       with:
         repo-token: ${{ secrets.GITHUB_TOKEN }}
         stale-issue-message: "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](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.io ✨ [Search Before Asking](https://stackoverflow.com/help/how-to-ask) ✨"
diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml
index 43f2f73..c975288 100644
--- a/.github/workflows/stale.yml
+++ b/.github/workflows/stale.yml
@@ -16,7 +16,7 @@ jobs:
       ACTIONS_STEP_DEBUG: true
     steps:
     - name: Close Stale Issues
-      uses: actions/stale@v7
+      uses: actions/stale@v8
       with:
         repo-token: ${{ secrets.GITHUB_TOKEN }}
         stale-issue-message: "This issue has been automatically marked as stale because it has been open 360 days with no activity. Remove stale label or comment or this will be closed in 180 days"
diff --git a/callbacks/associations.go b/callbacks/associations.go
index 9d7c141..f3cd464 100644
--- a/callbacks/associations.go
+++ b/callbacks/associations.go
@@ -51,25 +51,40 @@ func SaveBeforeAssociations(create bool) func(db *gorm.DB) {
 					}
 
 					elems := reflect.MakeSlice(reflect.SliceOf(fieldType), 0, 10)
+					distinctElems := reflect.MakeSlice(reflect.SliceOf(fieldType), 0, 10)
+					identityMap := map[string]bool{}
 					for i := 0; i < rValLen; i++ {
 						obj := db.Statement.ReflectValue.Index(i)
 						if reflect.Indirect(obj).Kind() != reflect.Struct {
 							break
 						}
-
 						if _, zero := rel.Field.ValueOf(db.Statement.Context, obj); !zero { // check belongs to relation value
 							rv := rel.Field.ReflectValueOf(db.Statement.Context, obj) // relation reflect value
+							if !isPtr {
+								rv = rv.Addr()
+							}
 							objs = append(objs, obj)
-							if isPtr {
-								elems = reflect.Append(elems, rv)
-							} else {
-								elems = reflect.Append(elems, rv.Addr())
+							elems = reflect.Append(elems, rv)
+
+							relPrimaryValues := make([]interface{}, 0, len(rel.FieldSchema.PrimaryFields))
+							for _, pf := range rel.FieldSchema.PrimaryFields {
+								if pfv, ok := pf.ValueOf(db.Statement.Context, rv); !ok {
+									relPrimaryValues = append(relPrimaryValues, pfv)
+								}
+							}
+							cacheKey := utils.ToStringKey(relPrimaryValues...)
+							if len(relPrimaryValues) != len(rel.FieldSchema.PrimaryFields) || !identityMap[cacheKey] {
+								if cacheKey != "" { // has primary fields
+									identityMap[cacheKey] = true
+								}
+
+								distinctElems = reflect.Append(distinctElems, rv)
 							}
 						}
 					}
 
 					if elems.Len() > 0 {
-						if saveAssociations(db, rel, elems, selectColumns, restricted, nil) == nil {
+						if saveAssociations(db, rel, distinctElems, selectColumns, restricted, nil) == nil {
 							for i := 0; i < elems.Len(); i++ {
 								setupReferences(objs[i], elems.Index(i))
 							}
diff --git a/clause/limit.go b/clause/limit.go
index 3ede738..abda005 100644
--- a/clause/limit.go
+++ b/clause/limit.go
@@ -33,7 +33,7 @@ func (limit Limit) MergeClause(clause *Clause) {
 	clause.Name = ""
 
 	if v, ok := clause.Expression.(Limit); ok {
-		if (limit.Limit == nil || *limit.Limit == 0) && (v.Limit != nil && *v.Limit != 0) {
+		if (limit.Limit == nil || *limit.Limit == 0) && v.Limit != nil {
 			limit.Limit = v.Limit
 		}
 
diff --git a/migrator/index.go b/migrator/index.go
index fe686e5..8845da9 100644
--- a/migrator/index.go
+++ b/migrator/index.go
@@ -17,12 +17,12 @@ func (idx Index) Table() string {
 	return idx.TableName
 }
 
-// Name return the name  of the index.
+// Name return the name of the index.
 func (idx Index) Name() string {
 	return idx.NameValue
 }
 
-// Columns return the columns fo the index
+// Columns return the columns of the index
 func (idx Index) Columns() []string {
 	return idx.ColumnList
 }
@@ -37,7 +37,7 @@ func (idx Index) Unique() (unique bool, ok bool) {
 	return idx.UniqueValue.Bool, idx.UniqueValue.Valid
 }
 
-// Option return the optional attribute fo the index
+// Option return the optional attribute of the index
 func (idx Index) Option() string {
 	return idx.OptionValue
 }
diff --git a/migrator/migrator.go b/migrator/migrator.go
index 389ce00..32c6a05 100644
--- a/migrator/migrator.go
+++ b/migrator/migrator.go
@@ -113,7 +113,7 @@ func (m Migrator) AutoMigrate(values ...interface{}) error {
 				return err
 			}
 		} else {
-			if err := m.RunWithValue(value, func(stmt *gorm.Statement) (errr error) {
+			if err := m.RunWithValue(value, func(stmt *gorm.Statement) error {
 				columnTypes, err := queryTx.Migrator().ColumnTypes(value)
 				if err != nil {
 					return err
@@ -123,7 +123,6 @@ func (m Migrator) AutoMigrate(values ...interface{}) error {
 					parseCheckConstraints = stmt.Schema.ParseCheckConstraints()
 				)
 				for _, dbName := range stmt.Schema.DBNames {
-					field := stmt.Schema.FieldsByDBName[dbName]
 					var foundColumn gorm.ColumnType
 
 					for _, columnType := range columnTypes {
@@ -135,12 +134,15 @@ func (m Migrator) AutoMigrate(values ...interface{}) error {
 
 					if foundColumn == nil {
 						// not found, add column
-						if err := execTx.Migrator().AddColumn(value, dbName); err != nil {
+						if err = execTx.Migrator().AddColumn(value, dbName); err != nil {
+							return err
+						}
+					} else {
+						// found, smartly migrate
+						field := stmt.Schema.FieldsByDBName[dbName]
+						if err = execTx.Migrator().MigrateColumn(value, field, foundColumn); err != nil {
 							return err
 						}
-					} else if err := execTx.Migrator().MigrateColumn(value, field, foundColumn); err != nil {
-						// found, smart migrate
-						return err
 					}
 				}
 
@@ -195,7 +197,7 @@ func (m Migrator) GetTables() (tableList []string, err error) {
 func (m Migrator) CreateTable(values ...interface{}) error {
 	for _, value := range m.ReorderModels(values, false) {
 		tx := m.DB.Session(&gorm.Session{})
-		if err := m.RunWithValue(value, func(stmt *gorm.Statement) (errr error) {
+		if err := m.RunWithValue(value, func(stmt *gorm.Statement) (err error) {
 			var (
 				createTableSQL          = "CREATE TABLE ? ("
 				values                  = []interface{}{m.CurrentTable(stmt)}
@@ -214,7 +216,7 @@ func (m Migrator) CreateTable(values ...interface{}) error {
 
 			if !hasPrimaryKeyInDataType && len(stmt.Schema.PrimaryFields) > 0 {
 				createTableSQL += "PRIMARY KEY ?,"
-				primaryKeys := []interface{}{}
+				primaryKeys := make([]interface{}, 0, len(stmt.Schema.PrimaryFields))
 				for _, field := range stmt.Schema.PrimaryFields {
 					primaryKeys = append(primaryKeys, clause.Column{Name: field.DBName})
 				}
@@ -225,8 +227,8 @@ func (m Migrator) CreateTable(values ...interface{}) error {
 			for _, idx := range stmt.Schema.ParseIndexes() {
 				if m.CreateIndexAfterCreateTable {
 					defer func(value interface{}, name string) {
-						if errr == nil {
-							errr = tx.Migrator().CreateIndex(value, name)
+						if err == nil {
+							err = tx.Migrator().CreateIndex(value, name)
 						}
 					}(value, idx.Name)
 				} else {
@@ -276,8 +278,8 @@ func (m Migrator) CreateTable(values ...interface{}) error {
 				createTableSQL += fmt.Sprint(tableOption)
 			}
 
-			errr = tx.Exec(createTableSQL, values...).Error
-			return errr
+			err = tx.Exec(createTableSQL, values...).Error
+			return err
 		}); err != nil {
 			return err
 		}
@@ -498,7 +500,7 @@ func (m Migrator) MigrateColumn(value interface{}, field *schema.Field, columnTy
 		currentDefaultNotNull := field.HasDefaultValue && (field.DefaultValueInterface != nil || !strings.EqualFold(field.DefaultValue, "NULL"))
 		dv, dvNotNull := columnType.DefaultValue()
 		if dvNotNull && !currentDefaultNotNull {
-			// defalut value -> null
+			// default value -> null
 			alterColumn = true
 		} else if !dvNotNull && currentDefaultNotNull {
 			// null -> default value
diff --git a/schema/field.go b/schema/field.go
index 00beb06..15edab9 100644
--- a/schema/field.go
+++ b/schema/field.go
@@ -580,8 +580,6 @@ func (field *Field) setupValuerAndSetter() {
 			case **bool:
 				if data != nil && *data != nil {
 					field.ReflectValueOf(ctx, value).SetBool(**data)
-				} else {
-					field.ReflectValueOf(ctx, value).SetBool(false)
 				}
 			case bool:
 				field.ReflectValueOf(ctx, value).SetBool(data)
@@ -601,8 +599,6 @@ func (field *Field) setupValuerAndSetter() {
 			case **int64:
 				if data != nil && *data != nil {
 					field.ReflectValueOf(ctx, value).SetInt(**data)
-				} else {
-					field.ReflectValueOf(ctx, value).SetInt(0)
 				}
 			case int64:
 				field.ReflectValueOf(ctx, value).SetInt(data)
@@ -667,8 +663,6 @@ func (field *Field) setupValuerAndSetter() {
 			case **uint64:
 				if data != nil && *data != nil {
 					field.ReflectValueOf(ctx, value).SetUint(**data)
-				} else {
-					field.ReflectValueOf(ctx, value).SetUint(0)
 				}
 			case uint64:
 				field.ReflectValueOf(ctx, value).SetUint(data)
@@ -721,8 +715,6 @@ func (field *Field) setupValuerAndSetter() {
 			case **float64:
 				if data != nil && *data != nil {
 					field.ReflectValueOf(ctx, value).SetFloat(**data)
-				} else {
-					field.ReflectValueOf(ctx, value).SetFloat(0)
 				}
 			case float64:
 				field.ReflectValueOf(ctx, value).SetFloat(data)
@@ -767,8 +759,6 @@ func (field *Field) setupValuerAndSetter() {
 			case **string:
 				if data != nil && *data != nil {
 					field.ReflectValueOf(ctx, value).SetString(**data)
-				} else {
-					field.ReflectValueOf(ctx, value).SetString("")
 				}
 			case string:
 				field.ReflectValueOf(ctx, value).SetString(data)


Test patch:
diff --git a/clause/limit_test.go b/clause/limit_test.go
index 79065ab..a9fd4e2 100644
--- a/clause/limit_test.go
+++ b/clause/limit_test.go
@@ -28,6 +28,10 @@ func TestLimit(t *testing.T) {
 			[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Limit: &limit0}},
 			"SELECT * FROM `users` LIMIT 0", nil,
 		},
+		{
+			[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Limit: &limit0}, clause.Limit{Offset: 0}},
+			"SELECT * FROM `users` LIMIT 0", nil,
+		},
 		{
 			[]clause.Interface{clause.Select{}, clause.From{}, clause.Limit{Offset: 20}},
 			"SELECT * FROM `users` OFFSET 20", nil,
diff --git a/tests/associations_many2many_test.go b/tests/associations_many2many_test.go
index 845c16a..b69d668 100644
--- a/tests/associations_many2many_test.go
+++ b/tests/associations_many2many_test.go
@@ -393,3 +393,33 @@ func TestConcurrentMany2ManyAssociation(t *testing.T) {
 	AssertEqual(t, err, nil)
 	AssertAssociationCount(t, find, "Languages", int64(count), "after concurrent append")
 }
+
+func TestMany2ManyDuplicateBelongsToAssociation(t *testing.T) {
+	user1 := User{Name: "TestMany2ManyDuplicateBelongsToAssociation-1", Friends: []*User{
+		{Name: "TestMany2ManyDuplicateBelongsToAssociation-friend-1", Company: Company{
+			ID:   1,
+			Name: "Test-company-1",
+		}},
+	}}
+
+	user2 := User{Name: "TestMany2ManyDuplicateBelongsToAssociation-2", Friends: []*User{
+		{Name: "TestMany2ManyDuplicateBelongsToAssociation-friend-2", Company: Company{
+			ID:   1,
+			Name: "Test-company-1",
+		}},
+	}}
+	users := []*User{&user1, &user2}
+	var err error
+	err = DB.Session(&gorm.Session{FullSaveAssociations: true}).Save(users).Error
+	AssertEqual(t, nil, err)
+
+	var findUser1 User
+	err = DB.Preload("Friends.Company").Where("id = ?", user1.ID).First(&findUser1).Error
+	AssertEqual(t, nil, err)
+	AssertEqual(t, user1, findUser1)
+
+	var findUser2 User
+	err = DB.Preload("Friends.Company").Where("id = ?", user2.ID).First(&findUser2).Error
+	AssertEqual(t, nil, err)
+	AssertEqual(t, user2, findUser2)
+}
diff --git a/tests/embedded_struct_test.go b/tests/embedded_struct_test.go
index 63ec53e..0d240fd 100644
--- a/tests/embedded_struct_test.go
+++ b/tests/embedded_struct_test.go
@@ -103,9 +103,16 @@ func TestEmbeddedPointerTypeStruct(t *testing.T) {
 		URL   string
 	}
 
+	type Author struct {
+		ID    string
+		Name  string
+		Email string
+	}
+
 	type HNPost struct {
 		*BasePost
 		Upvotes int32
+		*Author `gorm:"EmbeddedPrefix:user_"` // Embedded struct
 	}
 
 	DB.Migrator().DropTable(&HNPost{})
@@ -123,6 +130,10 @@ func TestEmbeddedPointerTypeStruct(t *testing.T) {
 	if hnPost.Title != "embedded_pointer_type" {
 		t.Errorf("Should find correct value for embedded pointer type")
 	}
+
+	if hnPost.Author != nil {
+		t.Errorf("Expected to get back a nil Author but got: %v", hnPost.Author)
+	}
 }
 
 type Content struct {