Fix set state if running query bug #560
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merged
bgentry
force-pushed
the
bg-fix-set-state-if-running-query-bug
branch
from
20ba899
to
06bbfdc
Compare
brandur
approved these changes
Aug 24, 2024
While doing some work on a related query, I noticed that the `unique_key` is being cleared here specifically if the new state is explicitly `cancelled`, but _not_ if the state is being set to cancel because of a cancellation attempt that didn't get picked up before the job completed. I didn't yet add test coverage for this but wanted to at least commit it before I forgot.
bgentry
force-pushed
the
bg-fix-set-state-if-running-query-bug
branch
from
06bbfdc
to
11c3b12
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While doing some work on a related query, I noticed that the
unique_keyis being cleared here specifically if the new state is explicitlycancelled, but not if the state is being set to cancel because of a cancellation attempt that didn't get picked up before the job completed.I didn't yet add test coverage for this but wanted to at least commit it before I forgot. Probably worth it given that this is super nuanced and could easily break in the future?
I also took the occasion to upgrade to sqlc v1.27 in this PR because I picked it up from Homebrew, but I'm opening a separate PR for that too.