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

Handle.begin() must be explicitly called in transaction #2491

Closed
grigorem opened this issue Sep 19, 2023 · 2 comments · Fixed by #2492
Closed

Handle.begin() must be explicitly called in transaction #2491

grigorem opened this issue Sep 19, 2023 · 2 comments · Fixed by #2492
Assignees
Labels

Comments

@grigorem
Copy link

grigorem commented Sep 19, 2023

On the latest version (3.41.1), if autocommit is disabled, you need to explicitly call handle.begin() before any insert/update/delete to make any changes. If not, it silently exits without making any changes.

Not sure if it was an intended change or if the previous behaviour was wrong, but it wasn't clear from the release notes or documentation that this would be needed.

Example:

// ... jdbi - postgres connection with autocommit set to false
int result = jdbi.inTransaction(handle -> {
      int inserted = handle.execute("INSERT INTO example_table (example_column) VALUES ('example_value')");
      handle.commit();
      return inserted;
    });

Running the above code on 3.41.0, will insert into DB.
Running the above code on 3.41.1, will not insert into DB.

To fix the above on 3.41.1 you would need to add handle.begin():

// ... jdbi - postgres connection with autocommit set to false
int result = jdbi.inTransaction(handle -> {
      handle.begin();
      int inserted = handle.execute("INSERT INTO example_table (example_column) VALUES ('example_value')");
      handle.commit();
      return inserted;
    });

From what I found, this might be caused by the new logic around handlerState property added in the BoundLocalTransactionHandler implementation (#2479).

@stevenschlansker
Copy link
Member

It is not necessary to mix inTransaction with begin and commit. The inTransaction call will call both for you.
The first example has imbalanced transaction handling - it effectively calls begin(); begin(); commit() which is not correct.
The second example will work, but has unnecessarily nested transactions - remove begin and commit and it works just the same.

@hgschmie
Copy link
Contributor

it is a good call that we should not silently error out if someone calls commit or rollback without a begin first.

The main purpose of calling "begin" is to turn off autocommit if it was set. I will put a test together from the example code above that test the pre and post 3.41.1 behavior.

@hgschmie hgschmie self-assigned this Sep 19, 2023
hgschmie added a commit to hgschmie/jdbi that referenced this issue Sep 20, 2023
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
hgschmie added a commit to hgschmie/jdbi that referenced this issue Sep 20, 2023
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

3 participants