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: association concurrently appending #6044

Merged
merged 3 commits into from
Feb 18, 2023
Merged

Conversation

black-06
Copy link
Contributor

@black-06 black-06 commented Feb 9, 2023

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

What did this pull request do?

Fix bug when association concurrently appending.

Association in each goroutine will modify the user.Languages by reflect.Append, which method grows the slice.
If it grows successfully, languages will be a new slice, that's ok.
But If cap is enough, these goroutines will write the same slice, that's wrong.

If we only call DB.FirstOrCreate(&user), user.Languages len is 0 and cap is 0, that's ok.
if we only call DB.Preload("Languages").FirstOrCreate(&user), still len 0 and cap 0.
But we call DB.FirstOrCreate(&user) first, then DB.Preload("Languages").FirstOrCreate(&user), user.Languages is len 0 and cap 10, this error will occur #5801

User Case Description

DB.FirstOrCreate(&user)
DB.Preload("Languages").FirstOrCreate(&user)

var wg sync.WaitGroup
for i := 0; i < count; i++ {
wg.Add(1)
go func(user User, language Language) {
	err := DB.Model(&user).Association("Languages").Append(&language)
	AssertEqual(t, err, nil)

	wg.Done()
}(user, languages[i])
}
wg.Wait()

@black-06
Copy link
Contributor Author

black-06 commented Feb 9, 2023

When I added a new test, the old test failed. Did I miss anything?

Copy link
Member

@a631807682 a631807682 left a comment

Choose a reason for hiding this comment

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

When we use goroutine, another connection is created, causing PRAGMA foreign_keys to become invalid.
https://www.sqlite.org/foreignkeys.html#fk_enable

We can create a db instance specifically for it

	tx, err := OpenTestConnection()
	// your test

@jinzhu jinzhu merged commit 42fc75c into go-gorm:master Feb 18, 2023
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.

4 participants