Repository: go-gorm/gorm
Pull request #5503 (2022-07-10 13:10:01)

fix empty QueryClauses in association (#5502)
<!--
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?

<!--
provide a general description of the code changes in your pull request
-->

See #5502

### User Case Description

<!-- Your use case -->


a631807682: @Goxiaoy Can you add some tests? We need to make sure it won't be broken by other changes.
Goxiaoy: @a631807682 Added
Goxiaoy: @a631807682 formated

Patch:
diff --git a/.gitignore b/.gitignore
index 45505cc..7273332 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,4 +3,5 @@ documents
 coverage.txt
 _book
 .idea
-vendor
\ No newline at end of file
+vendor
+.vscode
diff --git a/association.go b/association.go
index 35e10dd..06229ca 100644
--- a/association.go
+++ b/association.go
@@ -507,7 +507,9 @@ func (association *Association) buildCondition() *DB {
 				joinStmt.AddClause(queryClause)
 			}
 			joinStmt.Build("WHERE")
-			tx.Clauses(clause.Expr{SQL: strings.Replace(joinStmt.SQL.String(), "WHERE ", "", 1), Vars: joinStmt.Vars})
+			if len(joinStmt.SQL.String()) > 0 {
+				tx.Clauses(clause.Expr{SQL: strings.Replace(joinStmt.SQL.String(), "WHERE ", "", 1), Vars: joinStmt.Vars})
+			}
 		}
 
 		tx = tx.Session(&Session{QueryFields: true}).Clauses(clause.From{Joins: []clause.Join{{
diff --git a/callbacks/associations.go b/callbacks/associations.go
index 4a50e6c..00e00fc 100644
--- a/callbacks/associations.go
+++ b/callbacks/associations.go
@@ -206,7 +206,7 @@ func SaveAfterAssociations(create bool) func(db *gorm.DB) {
 								}
 							}
 
-							cacheKey := utils.ToStringKey(relPrimaryValues)
+							cacheKey := utils.ToStringKey(relPrimaryValues...)
 							if len(relPrimaryValues) != len(rel.FieldSchema.PrimaryFields) || !identityMap[cacheKey] {
 								identityMap[cacheKey] = true
 								if isPtr {
@@ -292,7 +292,7 @@ func SaveAfterAssociations(create bool) func(db *gorm.DB) {
 								}
 							}
 
-							cacheKey := utils.ToStringKey(relPrimaryValues)
+							cacheKey := utils.ToStringKey(relPrimaryValues...)
 							if len(relPrimaryValues) != len(rel.FieldSchema.PrimaryFields) || !identityMap[cacheKey] {
 								identityMap[cacheKey] = true
 								distinctElems = reflect.Append(distinctElems, elem)
diff --git a/statement.go b/statement.go
index 850af6c..1268781 100644
--- a/statement.go
+++ b/statement.go
@@ -650,7 +650,7 @@ func (stmt *Statement) Changed(fields ...string) bool {
 	return false
 }
 
-var nameMatcher = regexp.MustCompile(`^[\W]?(?:[a-z_0-9]+?)[\W]?\.[\W]?([a-z_0-9]+?)[\W]?$`)
+var nameMatcher = regexp.MustCompile(`^(?:[\W]?([A-Za-z_0-9]+?)[\W]?\.)?[\W]?([A-Za-z_0-9]+?)[\W]?$`)
 
 // SelectAndOmitColumns get select and omit columns, select -> true, omit -> false
 func (stmt *Statement) SelectAndOmitColumns(requireCreate, requireUpdate bool) (map[string]bool, bool) {
@@ -672,8 +672,8 @@ func (stmt *Statement) SelectAndOmitColumns(requireCreate, requireUpdate bool) (
 			}
 		} else if field := stmt.Schema.LookUpField(column); field != nil && field.DBName != "" {
 			results[field.DBName] = true
-		} else if matches := nameMatcher.FindStringSubmatch(column); len(matches) == 2 {
-			results[matches[1]] = true
+		} else if matches := nameMatcher.FindStringSubmatch(column); len(matches) == 3 && (matches[1] == stmt.Table || matches[1] == "") {
+			results[matches[2]] = true
 		} else {
 			results[column] = true
 		}


Test patch:
diff --git a/statement_test.go b/statement_test.go
index a89cc7d..a537c7b 100644
--- a/statement_test.go
+++ b/statement_test.go
@@ -36,17 +36,21 @@ func TestWhereCloneCorruption(t *testing.T) {
 }
 
 func TestNameMatcher(t *testing.T) {
-	for k, v := range map[string]string{
-		"table.name":         "name",
-		"`table`.`name`":     "name",
-		"'table'.'name'":     "name",
-		"'table'.name":       "name",
-		"table1.name_23":     "name_23",
-		"`table_1`.`name23`": "name23",
-		"'table23'.'name_1'": "name_1",
-		"'table23'.name1":    "name1",
+	for k, v := range map[string][]string{
+		"table.name":         []string{"table", "name"},
+		"`table`.`name`":     []string{"table", "name"},
+		"'table'.'name'":     []string{"table", "name"},
+		"'table'.name":       []string{"table", "name"},
+		"table1.name_23":     []string{"table1", "name_23"},
+		"`table_1`.`name23`": []string{"table_1", "name23"},
+		"'table23'.'name_1'": []string{"table23", "name_1"},
+		"'table23'.name1":    []string{"table23", "name1"},
+		"'name1'":            []string{"", "name1"},
+		"`name_1`":           []string{"", "name_1"},
+		"`Name_1`":           []string{"", "Name_1"},
+		"`Table`.`nAme`":     []string{"Table", "nAme"},
 	} {
-		if matches := nameMatcher.FindStringSubmatch(k); len(matches) < 2 || matches[1] != v {
+		if matches := nameMatcher.FindStringSubmatch(k); len(matches) < 3 || matches[1] != v[0] || matches[2] != v[1] {
 			t.Errorf("failed to match value: %v, got %v, expect: %v", k, matches, v)
 		}
 	}
diff --git a/tests/associations_test.go b/tests/associations_test.go
index e729e97..42b32af 100644
--- a/tests/associations_test.go
+++ b/tests/associations_test.go
@@ -4,6 +4,8 @@ import (
 	"testing"
 
 	"gorm.io/gorm"
+	"gorm.io/gorm/clause"
+	"gorm.io/gorm/schema"
 	. "gorm.io/gorm/utils/tests"
 )
 
@@ -284,3 +286,65 @@ func TestAssociationError(t *testing.T) {
 	err = DB.Model(&emptyUser).Association("Languages").Delete(&user1.Languages)
 	AssertEqual(t, err, gorm.ErrPrimaryKeyRequired)
 }
+
+type (
+	myType           string
+	emptyQueryClause struct {
+		Field *schema.Field
+	}
+)
+
+func (myType) QueryClauses(f *schema.Field) []clause.Interface {
+	return []clause.Interface{emptyQueryClause{Field: f}}
+}
+
+func (sd emptyQueryClause) Name() string {
+	return "empty"
+}
+
+func (sd emptyQueryClause) Build(clause.Builder) {
+}
+
+func (sd emptyQueryClause) MergeClause(*clause.Clause) {
+}
+
+func (sd emptyQueryClause) ModifyStatement(stmt *gorm.Statement) {
+	// do nothing
+}
+
+func TestAssociationEmptyQueryClause(t *testing.T) {
+	type Organization struct {
+		gorm.Model
+		Name string
+	}
+	type Region struct {
+		gorm.Model
+		Name          string
+		Organizations []Organization `gorm:"many2many:region_orgs;"`
+	}
+	type RegionOrg struct {
+		RegionId       uint
+		OrganizationId uint
+		Empty          myType
+	}
+	if err := DB.SetupJoinTable(&Region{}, "Organizations", &RegionOrg{}); err != nil {
+		t.Fatalf("Failed to set up join table, got error: %s", err)
+	}
+	if err := DB.Migrator().DropTable(&Organization{}, &Region{}); err != nil {
+		t.Fatalf("Failed to migrate, got error: %s", err)
+	}
+	if err := DB.AutoMigrate(&Organization{}, &Region{}); err != nil {
+		t.Fatalf("Failed to migrate, got error: %v", err)
+	}
+	region := &Region{Name: "Region1"}
+	if err := DB.Create(region).Error; err != nil {
+		t.Fatalf("fail to create region %v", err)
+	}
+	var orgs []Organization
+
+	if err := DB.Model(&Region{}).Association("Organizations").Find(&orgs); err != nil {
+		t.Fatalf("fail to find region organizations %v", err)
+	} else {
+		AssertEqual(t, len(orgs), 0)
+	}
+}
diff --git a/tests/helper_test.go b/tests/helper_test.go
index 7ee2a57..d1af073 100644
--- a/tests/helper_test.go
+++ b/tests/helper_test.go
@@ -80,6 +80,7 @@ func CheckPet(t *testing.T, pet Pet, expect Pet) {
 			t.Fatalf("errors happened when query: %v", err)
 		} else {
 			AssertObjEqual(t, newPet, pet, "ID", "CreatedAt", "UpdatedAt", "DeletedAt", "UserID", "Name")
+			AssertObjEqual(t, newPet, expect, "ID", "CreatedAt", "UpdatedAt", "DeletedAt", "UserID", "Name")
 		}
 	}
 
@@ -174,6 +175,7 @@ func CheckUser(t *testing.T, user User, expect User) {
 				var manager User
 				DB.First(&manager, "id = ?", *user.ManagerID)
 				AssertObjEqual(t, manager, user.Manager, "ID", "CreatedAt", "UpdatedAt", "DeletedAt", "Name", "Age", "Birthday", "CompanyID", "ManagerID", "Active")
+				AssertObjEqual(t, manager, expect.Manager, "ID", "CreatedAt", "UpdatedAt", "DeletedAt", "Name", "Age", "Birthday", "CompanyID", "ManagerID", "Active")
 			}
 		} else if user.ManagerID != nil {
 			t.Errorf("Manager should not be created for zero value, got: %+v", user.ManagerID)
diff --git a/tests/update_belongs_to_test.go b/tests/update_belongs_to_test.go
index 8fe0f28..4e94cfd 100644
--- a/tests/update_belongs_to_test.go
+++ b/tests/update_belongs_to_test.go
@@ -41,4 +41,19 @@ func TestUpdateBelongsTo(t *testing.T) {
 	var user4 User
 	DB.Preload("Company").Preload("Manager").Find(&user4, "id = ?", user.ID)
 	CheckUser(t, user4, user)
+
+	user.Company.Name += "new2"
+	user.Manager.Name += "new2"
+	if err := DB.Session(&gorm.Session{FullSaveAssociations: true}).Select("`Company`").Save(&user).Error; err != nil {
+		t.Fatalf("errors happened when update: %v", err)
+	}
+
+	var user5 User
+	DB.Preload("Company").Preload("Manager").Find(&user5, "id = ?", user.ID)
+	if user5.Manager.Name != user4.Manager.Name {
+		t.Errorf("should not update user's manager")
+	} else {
+		user.Manager.Name = user4.Manager.Name
+	}
+	CheckUser(t, user, user5)
 }
diff --git a/tests/update_has_one_test.go b/tests/update_has_one_test.go
index c926fbc..40af6ae 100644
--- a/tests/update_has_one_test.go
+++ b/tests/update_has_one_test.go
@@ -90,8 +90,9 @@ func TestUpdateHasOne(t *testing.T) {
 	t.Run("Restriction", func(t *testing.T) {
 		type CustomizeAccount struct {
 			gorm.Model
-			UserID sql.NullInt64
-			Number string `gorm:"<-:create"`
+			UserID  sql.NullInt64
+			Number  string `gorm:"<-:create"`
+			Number2 string
 		}
 
 		type CustomizeUser struct {
@@ -114,7 +115,8 @@ func TestUpdateHasOne(t *testing.T) {
 		cusUser := CustomizeUser{
 			Name: "update-has-one-associations",
 			Account: CustomizeAccount{
-				Number: number,
+				Number:  number,
+				Number2: number,
 			},
 		}
 
@@ -122,6 +124,7 @@ func TestUpdateHasOne(t *testing.T) {
 			t.Fatalf("errors happened when create: %v", err)
 		}
 		cusUser.Account.Number += "-update"
+		cusUser.Account.Number2 += "-update"
 		if err := DB.Session(&gorm.Session{FullSaveAssociations: true}).Updates(&cusUser).Error; err != nil {
 			t.Fatalf("errors happened when create: %v", err)
 		}
@@ -129,5 +132,6 @@ func TestUpdateHasOne(t *testing.T) {
 		var account2 CustomizeAccount
 		DB.Find(&account2, "user_id = ?", cusUser.ID)
 		AssertEqual(t, account2.Number, number)
+		AssertEqual(t, account2.Number2, cusUser.Account.Number2)
 	})
 }
diff --git a/tests/update_test.go b/tests/update_test.go
index 0fc89a9..d763458 100644
--- a/tests/update_test.go
+++ b/tests/update_test.go
@@ -307,6 +307,8 @@ func TestSelectWithUpdate(t *testing.T) {
 	if utils.AssertEqual(result.UpdatedAt, user.UpdatedAt) {
 		t.Fatalf("Update struct should update UpdatedAt, was %+v, got %+v", result.UpdatedAt, user.UpdatedAt)
 	}
+
+	AssertObjEqual(t, result, User{Name: "update_with_select"}, "Name", "Age")
 }
 
 func TestSelectWithUpdateWithMap(t *testing.T) {
diff --git a/utils/utils_test.go b/utils/utils_test.go
index 5737c51..27dfee1 100644
--- a/utils/utils_test.go
+++ b/utils/utils_test.go
@@ -12,3 +12,20 @@ func TestIsValidDBNameChar(t *testing.T) {
 		}
 	}
 }
+
+func TestToStringKey(t *testing.T) {
+	cases := []struct {
+		values []interface{}
+		key    string
+	}{
+		{[]interface{}{"a"}, "a"},
+		{[]interface{}{1, 2, 3}, "1_2_3"},
+		{[]interface{}{[]interface{}{1, 2, 3}}, "[1 2 3]"},
+		{[]interface{}{[]interface{}{"1", "2", "3"}}, "[1 2 3]"},
+	}
+	for _, c := range cases {
+		if key := ToStringKey(c.values...); key != c.key {
+			t.Errorf("%v: expected %v, got %v", c.values, c.key, key)
+		}
+	}
+}