Skip to content

Conversation

@phroggyy
Copy link
Contributor

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

What did this pull request do?

This test changes SAVEPOINT names from relying on the memory address of the callback (which can cause unexpected issues with partial rollbacks), to using a random integer.

The integer is generated using maphash.Hash, which is used as that internally relies on runtime.fastrand, which is the most performant way to get a random integer.

I've ensured the test fails without my code changes, and passes with them applied.

Use Case Description

Covered in #7173

Copy link
Contributor

@daheige daheige left a comment

Choose a reason for hiding this comment

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

LGTM

@jinzhu jinzhu merged commit 7f75b12 into go-gorm:master Sep 14, 2024
@Asarew Asarew mentioned this pull request Nov 28, 2024
// nested transaction
if !db.DisableNestedTransaction {
err = db.SavePoint(fmt.Sprintf("sp%p", fc)).Error
spID := new(maphash.Hash).Sum64()

Choose a reason for hiding this comment

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

instead of sprintf two times for the same id we could just do:

spID := fmt.Sprintf("sp%d", new(maphash.Hash).Sum64())

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.

6 participants