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

Begin/ End transaction - fails on the commit #79

Open
beberman opened this issue Jul 24, 2023 · 4 comments
Open

Begin/ End transaction - fails on the commit #79

beberman opened this issue Jul 24, 2023 · 4 comments

Comments

@beberman
Copy link

beberman commented Jul 24, 2023

I believe I've found a base issues on the code for transactions. I"m working on MySQL 8 and the following sequence:
(begin-transaction conn)
(query conn "insert...")
(commit conn)

Ends up with MySQL holding an open transaction on the connection thread.

I believe the bug is in driver.lisp in the begin-transaction code. The code for commit checks the variable transaction-state using the method get-transaction-state . This is returning _n_il so the when clause simply falls through and the call-next-method is never called (which would actually issue the commit).

The reason appears to begin-transaction never adds a cons to this variable which does happen in %with-transaction. This could be set with with-savepoint. Is that the required process? If so the documentation should be updated. Otherwise this is a bug.

Rollback has the same issue as Commit.

@fukamachi
Copy link
Owner

I don't understand what's exactly the problem you have.
Where is the end-transaction symbol from? (you mean commit?)
It'd be helpful if you add a reproducible code, its result, and expected behavior to a bug report.

@beberman
Copy link
Author

beberman commented Jul 29, 2023

Sure happy to.
Assume the following table is in a test database

create table facts (
       id int not null primary key auto_increment,
       text varchar(255) not null
);

Then run this test code:

(defun test-fn ()
  (let ((conn nil)
        (stmt nil)
        (data '("baz")))
    (setf conn (dbi:connect 'mysql :host "localhost" :username "bseberman" :password ""))
    (dbi:do-sql conn "use test")
    (dbi:begin-transaction conn)
    (setf stmt (dbi:prepare conn "insert into facts (text) values (?)"))
    (dbi:execute stmt data)
    (dbi:commit conn)
    ))

If begin and commit where pairs, then I would expect that the following query run on MySQL would return the new id and data.
Select * from facts;
However it does not show the new values. The reason is the commit on MySQL was never called, because of the logic I described above.

Instead looking at the outstanding transactions shows that there is still an open transaction:
SELECT count(*) FROM information_schema.innodb_trx;
This returns 1 which shows that the transaction is still open.

The with-transaction approach does work

(defun test-fn-with-trans ()
    (let ((conn nil)
        (stmt nil)
        (data '("baz")))
      (setf conn (dbi:connect 'mysql :host "localhost" :username "bseberman" :password ""))
      (dbi:do-sql conn "use test")
      (dbi:with-transaction conn
        (setf stmt (dbi:prepare conn "insert into facts (text) values (?)"))
        (dbi:execute stmt data)
        )))

The query directly on MySQL shows the new pair inserted into the table as expected.

this is because with-transaction uses the with-savepoint code.

This code of course leaks connections. A fuller implementation would hand the connection to the functions and maintain them in a cache.

@fukamachi
Copy link
Owner

Thank you for your explanation.

I never thought of using dbi:commit without dbi:with-transaction.

It'd be better to make it work since it's not an intuitive behavior.
However, are there any reasons that you don't use dbi:with-transaction all time?
I'm asking this because I'd like to judge the priority.

@beberman
Copy link
Author

I ran into this because other packages I've used in other languages typically work the way I used dbi. Its also consistent with how the SQL is designed within the databases.

That said dbi:with-transaction is very convenient and works correctly. I would suggest that the simplest solution is to either document that begin-transaction, commit, and rollback only work within the context of a save-point with an example or remove all of these functions from the API. MySQL does not support nested transactions but it can be implemented with save-points so that is the one case where controlling the save points is useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants