Subject: bug#67142: 29.1; with-sqlite-transaction commits on exception rather than rolling back

From: Visuwesh <[email-masked]>
Date: Fri, 17 Nov 2023 14:08:05 +0530

[வெள்ளி நவம்பர் 17, 2023] Eli Zaretskii wrote:

>> A minor mistake is the result variable not using an uninterned symbol
>> (or alternatively, `prog1` could replace the use of the result variable).
>
> I don't understand this part, sorry.  Why do we need a symbol to
> return the result of the body?

db-var and func-var are uninterned symbols in the macro expansion but
the variables 'result' and 'commit' marked below aren't.  If the intent
is not to expose these variables to the macro's BODY, the inner let
should also uninterned symbols right?

> diff --git a/lisp/sqlite.el b/lisp/sqlite.el
> index aad0aa4..4488896 100644
> --- a/lisp/sqlite.el
> +++ b/lisp/sqlite.el
> @@ -24,18 +24,25 @@
>  ;;; Code:
>  
>  (defmacro with-sqlite-transaction (db &rest body)
> -  "Execute BODY while holding a transaction for DB."
> +  "Execute BODY while holding a transaction for DB.
> +If BODY completes normally, commit the changes and return
> +the value of BODY.
> +If BODY signals an error, or transaction commit fails, roll
> +back the transaction changes."
>    (declare (indent 1) (debug (form body)))
>    (let ((db-var (gensym))
>          (func-var (gensym)))
>      `(let ((,db-var ,db)
>             (,func-var (lambda () ,@body)))
               --------

>         (if (sqlite-available-p)
> -           (unwind-protect
> -               (progn
> -                 (sqlite-transaction ,db-var)
> -                 (funcall ,func-var))
> -             (sqlite-commit ,db-var))
> +           (let (result commit)
                    ^^^^^^^^^^^^^
                    Use uninterned symbols here like above (marked with -).

> +             (unwind-protect
> +                 (progn
> +                   (sqlite-transaction ,db-var)
> +                   (setq result (funcall ,func-var))
> +                   (setq commit (sqlite-commit ,db-var))
> +                   result)
> +             (or commit (sqlite-rollback ,db-var))))
>           (funcall ,func-var)))))
>  
>  (provide 'sqlite)