fukamachi / cl-dbi

Database independent interface for Common Lisp
202 stars 28 forks source link

Begin/ End transaction - fails on the commit #79

Open beberman opened 1 year ago

beberman commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.