cl-dbi:commit should make a non-local exit

[svetlyak40wt]

This behavior is implicit, unexpected and leads to many WTFs.

This is a short illustration:

CL-USER> (dbi:with-transaction mito.connection:*connection*
           (format t "BEFORE COMMIT~%")
           (cl-dbi:commit mito:*connection*)
           (format t "AFTER COMMIT~%"))
BEFORE COMMIT
T

Code after cl-dbi:commit have no change to be executed! This is because cl-dbi:commit signals dbi.driver::transaction-done-condition and it is catched in dbi:with-transaction by handler-case:

(let ((#:conn-var1322 mito.connection:*connection*))
  (if (find #:conn-var1322 dbi.driver:*in-transaction* :test #'eq)
      (dbi:with-savepoint #:conn-var1322
        (format t "BEFORE COMMIT~%")
        (dbi.driver:commit mito.connection:*connection*)
        (format t "AFTER COMMIT~%"))
      (let* (#:transaction-done1323
             #:transaction-ok1324
             (#:conn-var1325 #:conn-var1322)
             (dbi.driver:*in-transaction*
              (cons #:conn-var1325 dbi.driver:*in-transaction*)))
        (dbi.driver:begin-transaction #:conn-var1325)
        (unwind-protect
            (handler-case
             (multiple-value-prog1
                 (progn
                  (format t "BEFORE COMMIT~%")
                  (dbi.driver:commit mito.connection:*connection*)
                  (format t "AFTER COMMIT~%"))
               (setf #:transaction-ok1324 t))
             (dbi.driver::transaction-done-condition nil
              (setf #:transaction-done1323 t)))
          (unless #:transaction-done1323
            (handler-case
             (if #:transaction-ok1324
                 (dbi.driver:commit #:conn-var1325)
                 (dbi.driver:rollback #:conn-var1325))
             (dbi.driver::transaction-done-condition nil)))))))

Probably, handler-bind could be used here to keep track if transaction was commited.

[svetlyak40wt]

I've experimented with this code a little bit. Probably the better way will be to refactor handling transactions and savepoints and to keep a transaction's state in *in-transaction* list like:

(defclass transaction-state ()
    ((conn :documentation "Connection where transaction was started")
     (state :documentation "Either transaction in progress or not"
                :type (member :in-progress :commited :rolled-back)))

[svetlyak40wt]

@fukamachi if you lack of time, I could make a pull request.

[fukamachi]

This change is intended not to run more SQLs after commit/rollback because I think that those operations are manual explicit terminations of with-transaction.

2019年10月4日(金) 15:59 Alexander Artemenko notifications@github.com:

@fukamachi https://github.com/fukamachi if you lack of time, I could make a pull request.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/fukamachi/cl-dbi/issues/49?email_source=notifications&email_token=AAAWDSRULNXGKFRWDFIQGYLQM3SVPA5CNFSM4I5L3QZKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAKVSLY#issuecomment-538269999, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAWDSVBQTQ3NCFTW3YMNV3QM3SVPANCNFSM4I5L3QZA .

[fukamachi]

It could be done with handler-bind, as you mentioned, and raise an error for following execute-with-connection or do-sql.

[svetlyak40wt]

My case was like that:

I have a web request handler which has around method with with-transaction call. Here is simplified pseudocode describing my problem:

(define-condition immediate-response ()
  ((html :initarg :html)
   (code :initarg :code))
  (:documentation "Raise this to return response from any stage of the request."))

(defgeneric handle-request (app)
  (:documentation "This function should return a string with HTML response."))

(defmethod handle-request :around ((app t))
  (dbi:with-transaction (dbi:*connection*)
    (handler-case (call-next-method)
      (immediate-response (condition)
        (when (< (get-code condition)
                 500)
          ;; when code was OK, I want to save the state to the database
          (dbi:commit))
        ;; And in any case to return HTML, but right now
        ;; cl-dbi:commit prevent this line's execution because
        ;; it throws transaction-done-condition
        (get-html condition)))))

(defclass my-app ()
  ())

(defun find-the-object ()
  (let ((results (execute "SELECT * FROM objects")))
    (unless results
      (signal 'immediate-response
              :code 404
              :html "Object not found"))))

(defmethod handle-request ((app my-app))
  (let ((object (find-the-object)))
    (do-the-job-with object)
    (render-html object)))

[svetlyak40wt]

I'll try to make a pull if you don't mind.

[fukamachi]

Now I see a problem you have and I admit that it’s an unpleasant behavior.

I'll try to make a pull if you don't mind.

Of course not. I appreciate your contribution.

[svetlyak40wt]

@fukamachi please, review this pull-request:

https://github.com/fukamachi/cl-dbi/pull/50

[fukamachi]

Probably fixed by #50. Thanks a lot.