-
Notifications
You must be signed in to change notification settings - Fork 11.8k
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
Annotations: Improve updating annotation tags queries #71201
Annotations: Improve updating annotation tags queries #71201
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great fixing this ❤️
// WithTransactionalDbSession creates a new SQL transaction to ensure consistency | ||
// for the database operations done within the [sqlstore.DBTransactionFunc]. | ||
// It's better to combine InTransaction and WithDbSession instead, as the context | ||
// variable is not updated when using this method. | ||
WithTransactionalDbSession(ctx context.Context, callback sqlstore.DBTransactionFunc) error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could deprecate this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about that, but I think we have a linter that wouldn't be too happy with that 🤔
WithTransactionalDbSession(ctx context.Context, callback sqlstore.DBTransactionFunc) error | ||
// WithDbSession runs database operations either in an existing transaction available | ||
// through [context.Context] or if that's not present, as non-transactional database |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe it would be good mentioning that runs inside transaction if it's under the scope of sqlstore.InTransaction()
Annotations: Improve annotation tag updates
Annotations: Improve annotation tag updates
What is this feature?
While working on #71199, I noticed that we're currently deleting all tags related to an annotation and then recreating them one by one when updating annotations with new tags. This provides an attempt at unifying that creation so that only the set of differing tags will be created/deleted.
Why do we need this feature?
🤷 We don't really need it, it took me a while to figure out what the original problem was, and I had already done this refactoring at that time.
It should be more performant given enough operations, but I have not validated that.
Who is this feature for?
Heavy users of annotations.
Which issue(s) does this PR fix?:
Fixes #
Special notes for your reviewer:
Please check that: