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 transactions for non-autocommitting handles #2492

Merged
merged 1 commit into from
Sep 21, 2023

Conversation

hgschmie
Copy link
Contributor

If a handle is by default using autocommit == false, then transactions
could be commited (or rolled back) without having to call
Handle#begin() first. This broke in 3.41.1.

Restore the old behavior. Fixes #2491

If a handle is by default using autocommit == false, then transactions
could be commited (or rolled back) without having to call
Handle#begin() first. This broke in 3.41.1.

Restore the old behavior. Fixes jdbi#2491
@sonarcloud
Copy link

sonarcloud bot commented Sep 20, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@stevenschlansker stevenschlansker left a comment

Choose a reason for hiding this comment

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

Is this expected to work? It seems a little weird to hand off an in-flight transaction to Jdbi. But I suppose breaking user code sucks too

@hgschmie
Copy link
Contributor Author

well, not sure how to answer that. But up until 3.41.1, you could turn autocommit off on a connection, pile a bunch of statements onto it and then call Handle#commit(). You probably shouldn't but it still worked.

What the change does is basically looking at the commit state of the connection when the transaction handler is created and if it has autocommit already turned off, it moves the state machine "one state further". So the handler is in the same state if you opened it with a connection that has auto-commit turned off as with a regular (autocommit on) + Handle#begin().

I still think it is a cornercase that the user stumbled upon and if it hadn't worked in the past, I would not make that change. I also think it is benign for "regular" use one can call begin multiple times.

@hgschmie hgschmie merged commit c9ed4ee into jdbi:master Sep 21, 2023
42 checks passed
@hgschmie hgschmie deleted the fix-2491 branch September 21, 2023 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Handle.begin() must be explicitly called in transaction
2 participants