Skip to content

fix ScheduledAt functionality with InsertManyTx#121

Merged
bgentry merged 2 commits intomasterfrom
bg-fix-insertmanytx-scheduled-jobs
Dec 20, 2023
Merged

fix ScheduledAt functionality with InsertManyTx#121
bgentry merged 2 commits intomasterfrom
bg-fix-insertmanytx-scheduled-jobs

Conversation

@bgentry
Copy link
Contributor

@bgentry bgentry commented Dec 19, 2023

I stumbled upon this fairly egregious bug in my own test app. It turns out when you insert with InsertManyTx, ScheduledAt is not respected and we never even added it to the underlying query. Instead, the job is enqueued immediately as available and worked right away.

This fixes the bug by respecting ScheduledAt the same on bulk inserts as for individual inserts. I added a test to verify the broken behavior and then fixed it.

I also pulled in a sqlc upgrade commit from #117 to avoid needing to work around having a newer version locally.

@bgentry bgentry added the bug Something isn't working label Dec 19, 2023
@bgentry bgentry requested a review from brandur December 19, 2023 10:59
I stumbled upon this fairly egregious bug in my own test app. It turns
out when you insert with `InsertManyTx`, `ScheduledAt` is not respected
and we never even added it to the underlying query. Instead, the job is
enqueued immediately as available and worked right away.

This fixes the bug by respecting `ScheduledAt` the same on bulk inserts
as for individual inserts. I added a test to verify the broken behavior
and then fixed it.
@bgentry bgentry force-pushed the bg-fix-insertmanytx-scheduled-jobs branch from ed630ef to 3a3093d Compare December 19, 2023 10:59
Copy link
Contributor

@brandur brandur left a comment

Choose a reason for hiding this comment

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

lgtm!

@bgentry bgentry merged commit be3a091 into master Dec 20, 2023
@bgentry bgentry deleted the bg-fix-insertmanytx-scheduled-jobs branch December 20, 2023 13:58
brandur added a commit that referenced this pull request Dec 22, 2023
Seems like a good idea to cut a version that includes #121. Here, add a
banner for 0.0.15 to the changelog so we can cut a release.
brandur added a commit that referenced this pull request Dec 22, 2023
Seems like a good idea to cut a version that includes #121. Here, add a
banner for 0.0.15 to the changelog so we can cut a release.
brandur added a commit that referenced this pull request Dec 22, 2023
Seems like a good idea to cut a version that includes #121. Here, add a
banner for 0.0.15 to the changelog so we can cut a release.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants