From debbugs-submit-bounces@debbugs.gnu.org Sun Nov 12 19:48:18 2023 Received: (at submit) by debbugs.gnu.org; 13 Nov 2023 00:48:18 +0000 Received: from localhost ([127.0.0.1]:57248 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r2L7h-0001XJ-AS for submit@debbugs.gnu.org; Sun, 12 Nov 2023 19:48:18 -0500 Received: from lists.gnu.org ([2001:470:142::17]:43908) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r2L7c-0001Wv-6u for submit@debbugs.gnu.org; Sun, 12 Nov 2023 19:48:15 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1r2L6j-0001jx-Kx for bug-gnu-emacs@gnu.org; Sun, 12 Nov 2023 19:47:19 -0500 Received: from mout-p-202.mailbox.org ([2001:67c:2050:0:465::202]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_CHACHA20_POLY1305:256) (Exim 4.90_1) (envelope-from ) id 1r2L6d-0005wQ-BU for bug-gnu-emacs@gnu.org; Sun, 12 Nov 2023 19:47:15 -0500 Received: from smtp1.mailbox.org (smtp1.mailbox.org [IPv6:2001:67c:2050:b231:465::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-202.mailbox.org (Postfix) with ESMTPS id 4ST9jy22xzz9sTt for ; Mon, 13 Nov 2023 01:47:02 +0100 (CET) Date: Mon, 13 Nov 2023 01:47:01 +0100 From: Vasilij Schneidermann To: bug-gnu-emacs@gnu.org Subject: 29.1; with-sqlite-transaction commits on exception rather than rolling back Message-ID: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="mkoGy538Gt7qXCzN" Content-Disposition: inline X-Rspamd-Queue-Id: 4ST9jy22xzz9sTt Received-SPF: pass client-ip=2001:67c:2050:0:465::202; envelope-from=mail@vasilij.de; helo=mout-p-202.mailbox.org X-Spam_score_int: -25 X-Spam_score: -2.6 X-Spam_bar: -- X-Spam_report: (-2.6 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: 0.7 (/) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.3 (/) --mkoGy538Gt7qXCzN Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable During experimentation with built-in SQLite support, I've noticed that if I use `with-sqlite-transaction' and there is a mistake in the body, `sqlite-commit' is executed rather than `sqlite-rollback', thereby committing the changes done up to the error instead of backing out. This appears to be unusual behavior. From my quick research, the following SQLite bindings in other languages have an equivalent to `with-sqlite-transaction' and back out changes on error: - CHICKEN Scheme: https://api.call-cc.org/5/doc/sql-de-lite#def:with-transaction - Go: https://pkg.go.dev/database/sql#Conn.BeginTx - Racket: https://docs.racket-lang.org/db/query-api.html#%28def._%28%28lib.= _db%2Fbase..rkt%29._call-with-transaction%29%29 - Ruby: https://www.rubydoc.info/gems/sqlite3/1.4.2/SQLite3/Database#transa= ction-instance_method - Rust: https://docs.rs/rusqlite/latest/rusqlite/struct.Connection.html#method.tr= ansaction In languages not offering an equivalent API, it was instead recommended to manually issue BEGIN/COMMIT/ROLLBACK statements. But given that `with-sqlite-transaction' exists, it would be less confusing to make it back out on exception (and re-raise the original error). In GNU Emacs 29.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.38, cairo version 1.17.8) Windowing system distributor 'The X.Org Foundation', version 11.0.12101009 System Description: Arch Linux Configured using: 'configure --sysconfdir=3D/etc --prefix=3D/usr --libexecdir=3D/usr/lib --with-tree-sitter --localstatedir=3D/var --with-cairo --disable-build-details --with-harfbuzz --with-libsystemd --with-modules --with-x-toolkit=3Dgtk3 'CFLAGS=3D-march=3Dx86-64 -mtune=3Dgeneric -O2 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=3D2 -Wformat -Werror=3Dformat-security -fstack-clash-protection -fcf-protection -g -ffile-prefix-map=3D/build/emacs/src=3D/usr/src/debug/emacs -flto=3Dauto' 'LDFLAGS=3D-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now -flto=3Dauto'' Configured features: ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG JSON LCMS2 LIBOTF LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB Important settings: value of $LC_MESSAGES:=20 value of $LANG: en_US.UTF-8 locale-coding-system: utf-8-unix Major mode: Lisp Interaction Minor modes in effect: tooltip-mode: t global-eldoc-mode: t eldoc-mode: t show-paren-mode: t electric-indent-mode: t mouse-wheel-mode: t tool-bar-mode: t menu-bar-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t blink-cursor-mode: t line-number-mode: t indent-tabs-mode: t transient-mark-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t Load-path shadows: None found. Features: (shadow sort mail-extr emacsbug message mailcap yank-media puny dired dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068 epg-config gnus-util text-property-search time-date subr-x mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils rmc iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic indonesian philippine cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese composite emoji-zwj charscript charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp files window text-properties overlay sha1 md5 base64 format env code-pages mule custom widget keymap hashtable-print-readable backquote threads dbusbind inotify lcms2 dynamic-setting system-font-setting font-render-setting cairo move-toolbar gtk x-toolkit xinput2 x multi-tty make-network-process emacs) Memory information: ((conses 16 36631 11549) (symbols 48 5153 0) (strings 32 13273 1917) (string-bytes 1 378652) (vectors 16 9346) (vector-slots 8 149327 16481) (floats 8 21 19) (intervals 56 235 0) (buffers 984 10)) --mkoGy538Gt7qXCzN Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEE0dAcySl3bqM8O17WFmfJg6zCifoFAmVRcgEACgkQFmfJg6zC ifrTOQf7Biv192lViw12PHt8qpZAgVwnh3LtORjzVWTZaf8oQ9mo0NcCmTp87CDt dDZcX0mooXOsCuwNndVNcfFyne03JUQcLYVYO7qDns6pblZ4wZ31CKONT6wCrrq9 AT4QChUp9ykPnuBGtt1+QJkpDqE+wpOjV+ukGTv5lqRPNvrQuRMGztgwG24yekBs x2MQEjSarkEhJnrlj7L3efvPOFgSQVd7Zuw9Xu1DsBNTSY+l/1HmpDONtsA2DReL b52L9nMjVIUnAxJTWTobt4gCDnJc59xEMnWRZNFjf2Pg64ObKOp1MTz2Kml0yVBH 7kflgHLalZyuXKHGdNPsC3qXNTD8ZQ== =i3bl -----END PGP SIGNATURE----- --mkoGy538Gt7qXCzN-- From debbugs-submit-bounces@debbugs.gnu.org Thu Nov 16 09:12:23 2023 Received: (at 67142) by debbugs.gnu.org; 16 Nov 2023 14:12:23 +0000 Received: from localhost ([127.0.0.1]:54823 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r3d6V-0004ge-62 for submit@debbugs.gnu.org; Thu, 16 Nov 2023 09:12:23 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:58110) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r3d6T-0004gS-Br for 67142@debbugs.gnu.org; Thu, 16 Nov 2023 09:12:22 -0500 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1r3d6N-0004rw-CD; Thu, 16 Nov 2023 09:12:15 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=luG99csJbJaUCKbXmVlk8zC3EZNSxx/k1sb9+cpaHYc=; b=qgV9X55+vO3z 05yImpXcU+e0Nxx4otpRnhKvt/h9WjOLBWh4w11kYWr+Sje9HfFO3pIFGNCasOiuIwt29c7Al1ivx LDBzf89WWTaWgU6Is9FdxjteNAg+9mUpC24iDLvcW5glvnFXDLGsjTcN05gnpbNhgOpoQjEY1b/UB gUKaJmSAJDU4c9sEivEOOHLWg+keSOu8/BojLoA2yz3v6LF7F3Ads0bCVjKF9zNBVelRBKuq4qFm4 J8YnyH3Fy51DI8e/XjtQRJHxWNpNP5S7c0nJv078zxUkNL3oJ1SsPzpCliEoSBbAQTGSa4ZbrtjuI 2apL2sus6M2vBdLV9FNWdQ==; Date: Thu, 16 Nov 2023 16:12:00 +0200 Message-Id: <83pm09pz2n.fsf@gnu.org> From: Eli Zaretskii To: Vasilij Schneidermann In-Reply-To: (message from Vasilij Schneidermann on Mon, 13 Nov 2023 01:47:01 +0100) Subject: Re: bug#67142: 29.1; with-sqlite-transaction commits on exception rather than rolling back References: X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 67142 Cc: 67142@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > Date: Mon, 13 Nov 2023 01:47:01 +0100 > From: Vasilij Schneidermann > > During experimentation with built-in SQLite support, I've noticed that > if I use `with-sqlite-transaction' and there is a mistake in the body, > `sqlite-commit' is executed rather than `sqlite-rollback', thereby > committing the changes done up to the error instead of backing out. > > This appears to be unusual behavior. From my quick research, the > following SQLite bindings in other languages have an equivalent to > `with-sqlite-transaction' and back out changes on error: Thanks. Does the change below look correct? (I'm not an expert on SQLite or DB programming in general.) diff --git a/lisp/sqlite.el b/lisp/sqlite.el index aad0aa4..df06647 100644 --- a/lisp/sqlite.el +++ b/lisp/sqlite.el @@ -24,7 +24,11 @@ ;;; 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))) @@ -32,10 +36,13 @@ with-sqlite-transaction (,func-var (lambda () ,@body))) (if (sqlite-available-p) (unwind-protect - (progn + (let (result) (sqlite-transaction ,db-var) - (funcall ,func-var)) - (sqlite-commit ,db-var)) + (setq result (funcall ,func-var)) + (or (sqlite-commit ,db-var) + (signal 'sqlite-error (list "SQLite commit failed"))) + result) + (sqlite-rollback ,db-var)) (funcall ,func-var))))) (provide 'sqlite) From debbugs-submit-bounces@debbugs.gnu.org Thu Nov 16 15:26:09 2023 Received: (at 67142) by debbugs.gnu.org; 16 Nov 2023 20:26:10 +0000 Received: from localhost ([127.0.0.1]:44673 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r3iwC-0005bk-1F for submit@debbugs.gnu.org; Thu, 16 Nov 2023 15:26:09 -0500 Received: from mout-p-102.mailbox.org ([2001:67c:2050:0:465::102]:37368) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r3iw6-0005bA-3d for 67142@debbugs.gnu.org; Thu, 16 Nov 2023 15:26:06 -0500 Received: from smtp1.mailbox.org (smtp1.mailbox.org [10.196.197.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-102.mailbox.org (Postfix) with ESMTPS id 4SWWkq0xkyz9smX; Thu, 16 Nov 2023 21:25:55 +0100 (CET) Date: Thu, 16 Nov 2023 21:25:53 +0100 From: Vasilij Schneidermann To: Eli Zaretskii Subject: Re: bug#67142: 29.1; with-sqlite-transaction commits on exception rather than rolling back Message-ID: References: <83pm09pz2n.fsf@gnu.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="T26XW0Y7c2mGn2Wj" Content-Disposition: inline In-Reply-To: <83pm09pz2n.fsf@gnu.org> X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 67142 Cc: 67142@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) --T26XW0Y7c2mGn2Wj Content-Type: text/plain; charset=utf-8 Content-Disposition: inline > Does the change below look correct? (I'm not an expert on SQLite or > DB programming in general.) The `unwind-protect` part does not look correct since the rollback is always performed, even after the body form completed successfully and a commit was done. Either a commit or a rollback should be done, not both. Perhaps `condition-case` would be the better option, but then the error would need to be re-raised after doing the rollback... A minor mistake is the result variable not using an uninterned symbol (or alternatively, `prog1` could replace the use of the result variable). --T26XW0Y7c2mGn2Wj Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEE0dAcySl3bqM8O17WFmfJg6zCifoFAmVWes4ACgkQFmfJg6zC ifoPbgf/Z+EmTqOy+FoMC6Hcz75MH0sBr8dZUBCbVsyEO8paEk9lxwxmbKv9VX8P LCj1pBQFeCVN1bL5olSkKUhU6g9fk9jyW5e1aYqXwwKTzYq84EUELJpvSGbdDA4I eR8brQhfyuDUaE5J3mkEVh9O8rfyuO0a+xqYbeyTk16fFNUPVvOhIW/sezogORuw pjFK30fKZfgZmwVbLdMUeW99YAEgD4rh1OYiE7kVxKWn/9/O6sIBWM7PnQbhISAt CZdyMERHrsQkgnh7VTYxRmVZC8fVjxnRKyOiCuoP4Nxdie55rFa95nmejzPcmbF8 0v7p1WzOklaiHusMn7Q7EJM99++16Q== =2uS4 -----END PGP SIGNATURE----- --T26XW0Y7c2mGn2Wj-- From debbugs-submit-bounces@debbugs.gnu.org Fri Nov 17 02:48:58 2023 Received: (at 67142) by debbugs.gnu.org; 17 Nov 2023 07:48:59 +0000 Received: from localhost ([127.0.0.1]:45034 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r3tb0-0002Xt-IZ for submit@debbugs.gnu.org; Fri, 17 Nov 2023 02:48:58 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:40200) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r3taw-0002XT-7o for 67142@debbugs.gnu.org; Fri, 17 Nov 2023 02:48:57 -0500 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1r3taq-000730-7K; Fri, 17 Nov 2023 02:48:48 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=cI367GYTOCUX6mWFnbs5ZMlULqhr8UBdS5fPmnMC0Us=; b=Oi3mPP6u/KHI GwVawa2Wagrp7mxIevRaYFTOi7e0hycZsQN5IX+7N/NSJd5BrMq8WI+RtSKZdbq/xPZr5YInc8znk eU2LoTPNO0J0vugmRHem3ZM2Q8AUPphFT8Nv0cjUHSmsDc/MCqUnnKesUFBSZwyGcos8OJ/ZtvLAj H9jI0m9MhTsXi/iNwiW99R4C1OWe7G3h2dKa9sxJk/aW1g4a9VDnzSiDAWn1bzEEmOUj8qM1FfqVD 0BZeeD0XGGaSumDnuc5Qh4icraj0NrMy36s93U+8A2Wl/mtbfRMehDv5A3Rk1AKeDjeZEKIEVq95P 00ACxjvpGl/v/SFsBXOezQ==; Date: Fri, 17 Nov 2023 09:48:41 +0200 Message-Id: <831qcoq0py.fsf@gnu.org> From: Eli Zaretskii To: Vasilij Schneidermann In-Reply-To: (message from Vasilij Schneidermann on Thu, 16 Nov 2023 21:25:53 +0100) Subject: Re: bug#67142: 29.1; with-sqlite-transaction commits on exception rather than rolling back References: <83pm09pz2n.fsf@gnu.org> X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 67142 Cc: 67142@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > Date: Thu, 16 Nov 2023 21:25:53 +0100 > From: Vasilij Schneidermann > Cc: 67142@debbugs.gnu.org > > > Does the change below look correct? (I'm not an expert on SQLite or > > DB programming in general.) > > The `unwind-protect` part does not look correct since the rollback is > always performed, even after the body form completed successfully and a > commit was done. Either a commit or a rollback should be done, not both. Oops. Is the below better? > 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? 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) + (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) From debbugs-submit-bounces@debbugs.gnu.org Fri Nov 17 03:38:17 2023 Received: (at 67142) by debbugs.gnu.org; 17 Nov 2023 08:38:17 +0000 Received: from localhost ([127.0.0.1]:45091 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r3uMj-00049R-DO for submit@debbugs.gnu.org; Fri, 17 Nov 2023 03:38:17 -0500 Received: from mail-pf1-x443.google.com ([2607:f8b0:4864:20::443]:61752) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r3uMg-000497-9o for 67142@debbugs.gnu.org; Fri, 17 Nov 2023 03:38:16 -0500 Received: by mail-pf1-x443.google.com with SMTP id d2e1a72fcca58-6c34e87b571so1604982b3a.3 for <67142@debbugs.gnu.org>; Fri, 17 Nov 2023 00:38:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1700210288; x=1700815088; darn=debbugs.gnu.org; h=content-transfer-encoding:mime-version:user-agent:message-id:date :references:in-reply-to:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=K7c7Ryo9c9W/wvl5U9zlP1HDinyfKZxWaufHxBGcLcU=; b=grY2fdqLh8Uc/ulJnvOk73wFilDEApx1HaXUqyvEkw5gLS83TEkYeylX1WuR7d/k8Z cseStD5kU0/gjG+qx/6iQL6141fDjpaAfSjZirQQun6JZKWe6iX59KqsBzJq5cvTZjat rGeLyQZ1btlqaFGm0zvGwmH+wQ9qOEazcYWOtclPe6e8b6zKeYt82GNMWdluFNxXRRJ+ Duz5TCkckxhU6uHAzbN+5GbRqk8IWajGi/hTqrYljez5vlfpWqdzRa6NY+v+Y7WrE+k9 u65MLRgT/3jUdLyVucbBBaPQzw0uQa1PwOdiZ2iVECkBP0EnmSgokBXp84HNw1XwYe6D FV4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700210288; x=1700815088; h=content-transfer-encoding:mime-version:user-agent:message-id:date :references:in-reply-to:subject:cc:to:from:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=K7c7Ryo9c9W/wvl5U9zlP1HDinyfKZxWaufHxBGcLcU=; b=ksfM59M3zyqgh1HgqqpGZDBd/dfcC6OOEIWNZrqbvm+AWdcvPVrOfe/0EPRi7P8x8I VtXBENO7e+kqmUdcmFfZeTaoSkv/NN2vcYeleAkRsVil9MwPTIlO0E9NMU9RWrTHly6C 3Vl9bkRB65Uva//CoBWjKSKfUTHFF7qhTMBAGeooamN1cnENVsbTAUOKMYM3MZmcnSqt cstyy5f1YsRj3DYMH/2cvOYKlRWZ7e2dbbRrJLK7XEdINcA17Or7Y5BcvE2Y6GbEDEXS W5P2+3cKPdCyLaf76x27USGzuLtjjIZwldOutefLhqqBSlLzirSbvAme8Zwy0CwiiScL dIWA== X-Gm-Message-State: AOJu0YxdG/Wdz5XutNtDtXdI9ROWg4xnsS2dvvW6hGiY2PpVmxpro/wm 2ZvD56ZXYSMgn9RGxxLz2M75VQyjB2FP2g== X-Google-Smtp-Source: AGHT+IHEJDtqog7n58wsvUhtaXybjM6k40VM597EIWTd9EdLX6qCH9oVjzJggUNM9ePlzx3ULEv7vw== X-Received: by 2002:a05:6a20:7d90:b0:187:a2ca:40a4 with SMTP id v16-20020a056a207d9000b00187a2ca40a4mr5769774pzj.50.1700210287947; Fri, 17 Nov 2023 00:38:07 -0800 (PST) Received: from localhost ([115.240.90.130]) by smtp.gmail.com with ESMTPSA id b7-20020a17090a800700b00274de7c8aacsm2726501pjn.45.2023.11.17.00.38.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 17 Nov 2023 00:38:07 -0800 (PST) From: Visuwesh To: Eli Zaretskii Subject: Re: bug#67142: 29.1; with-sqlite-transaction commits on exception rather than rolling back In-Reply-To: <831qcoq0py.fsf@gnu.org> (Eli Zaretskii's message of "Fri, 17 Nov 2023 09:48:41 +0200") References: <83pm09pz2n.fsf@gnu.org> <831qcoq0py.fsf@gnu.org> Date: Fri, 17 Nov 2023 14:08:05 +0530 Message-ID: <87a5rc4vwy.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: 67142 Cc: 67142@debbugs.gnu.org, Vasilij Schneidermann X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) [=E0=AE=B5=E0=AF=86=E0=AE=B3=E0=AF=8D=E0=AE=B3=E0=AE=BF =E0=AE=A8=E0=AE=B5= =E0=AE=AE=E0=AF=8D=E0=AE=AA=E0=AE=B0=E0=AF=8D 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: >=20=20 > (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))))) >=20=20 > (provide 'sqlite) From debbugs-submit-bounces@debbugs.gnu.org Fri Nov 17 04:08:48 2023 Received: (at 67142) by debbugs.gnu.org; 17 Nov 2023 09:08:48 +0000 Received: from localhost ([127.0.0.1]:45122 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r3uqG-0007av-AY for submit@debbugs.gnu.org; Fri, 17 Nov 2023 04:08:48 -0500 Received: from mout-p-201.mailbox.org ([2001:67c:2050:0:465::201]:40614) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r3uqD-0007ah-0m for 67142@debbugs.gnu.org; Fri, 17 Nov 2023 04:08:47 -0500 Received: from smtp102.mailbox.org (smtp102.mailbox.org [10.196.197.102]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-201.mailbox.org (Postfix) with ESMTPS id 4SWrfs56gJz9sTq; Fri, 17 Nov 2023 10:08:37 +0100 (CET) Date: Fri, 17 Nov 2023 10:08:36 +0100 From: Vasilij Schneidermann To: Visuwesh Subject: Re: bug#67142: 29.1; with-sqlite-transaction commits on exception rather than rolling back Message-ID: References: <83pm09pz2n.fsf@gnu.org> <831qcoq0py.fsf@gnu.org> <87a5rc4vwy.fsf@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="1HWvJaU75tn5gcrD" Content-Disposition: inline In-Reply-To: <87a5rc4vwy.fsf@gmail.com> X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 67142 Cc: Eli Zaretskii , 67142@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) --1HWvJaU75tn5gcrD Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable > > I don't understand this part, sorry. Why do we need a symbol to > > return the result of the body? >=20 > 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? Correct, this is what I was hinting at. As annoying as it is, when writing unhygienic macros one should use uninterned symbols to avoid exposing additional variables to the BODY argument. Alternatively, the pattern of `(let ((return (...))) ... return)` can be replaced with `(prog1 (...) ...)`. @Eli: The new patch looks better and closer to how the issue is solved in the Ruby sqlite3 gem. I'm still not sure about this use of `unwind-protect` being correct, but it does preserve the backtrace in case of an error better than when I used `(condition-case e (...) (error (apply #'signal e)))`. --1HWvJaU75tn5gcrD Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEE0dAcySl3bqM8O17WFmfJg6zCifoFAmVXLZAACgkQFmfJg6zC ifpX1wf/aFM0fRM0VZ85x6iwgzKW7uFNA8cDavSYGzLomB/lDwQohRvesO68h/eN I4HCXepLs5J2vmX4zg9D0vjWbawdJmo40oryDvbP3zdtQybf3ZK2UhZZT1f9Wpwd 0gnl9ToDUYW8YFJEmtukUAurU9tpqbJrS+WnRIExmYroS9Cy/QwlTbHVAc4bdMsF v2hAoYb9MpqNKZDfYOgJvyGQVYLHB6ANXmMP9VFOoKpERUQOTCp2UgFNgsT4upRt cSfXzLv84Wi3yB3m3j++Eg1PiGyFiNDYG1Uqkshqd1L55dDyp5DG8yopfSxulTLx ABJp5jbweNmLzT5oeTAv6G7WiuEVRw== =Gii1 -----END PGP SIGNATURE----- --1HWvJaU75tn5gcrD-- From debbugs-submit-bounces@debbugs.gnu.org Sat Nov 18 05:37:55 2023 Received: (at 67142) by debbugs.gnu.org; 18 Nov 2023 10:37:55 +0000 Received: from localhost ([127.0.0.1]:47875 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r4Ii2-00011B-Oi for submit@debbugs.gnu.org; Sat, 18 Nov 2023 05:37:55 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:41608) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r4Ihz-00010y-Sl for 67142@debbugs.gnu.org; Sat, 18 Nov 2023 05:37:53 -0500 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1r4Iht-0007eP-4I; Sat, 18 Nov 2023 05:37:45 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=Ca03YZmrn9LZqS2lkSwu4ZQSzah3zs9jT51shF03Nbc=; b=Nz12ZZV0tGKz eU4Fq3+7C6f8lJMobQsv83t0soOTp9ha0Bge2rkHkM59XEXxOQsLQvsr3+1o3AQxiIj5xVwgISho2 k79cK9LM35Ovr80gfwf4KUT6M0BmdJWxB+VtRh/UqT1Dal55Y1okTazPL/VwOo1ppoWzmApHw1TpZ jcaZGsRwATZWIbrPiDJw1ZOabg+XA2UGnh6YXIApKJIZjsUtahIv4O7fU6iCkh5dwmNg+jorgNexm 2Ah2eycDJwZoyC6HU45XG8o2gjPR/Lqj/oM7qRyEDbUqjMKNXprRUq9IsbSQeu6vHtXofdV3JG1jp 6EldDjlW/x84Q//CHPEABw==; Date: Sat, 18 Nov 2023 12:37:41 +0200 Message-Id: <83leavmjnu.fsf@gnu.org> From: Eli Zaretskii To: Vasilij Schneidermann In-Reply-To: (message from Vasilij Schneidermann on Fri, 17 Nov 2023 10:08:36 +0100) Subject: Re: bug#67142: 29.1; with-sqlite-transaction commits on exception rather than rolling back References: <83pm09pz2n.fsf@gnu.org> <831qcoq0py.fsf@gnu.org> <87a5rc4vwy.fsf@gmail.com> X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 67142 Cc: 67142@debbugs.gnu.org, visuweshm@gmail.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > Date: Fri, 17 Nov 2023 10:08:36 +0100 > From: Vasilij Schneidermann > Cc: Eli Zaretskii , 67142@debbugs.gnu.org > > > 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? > > Correct, this is what I was hinting at. As annoying as it is, when > writing unhygienic macros one should use uninterned symbols to avoid > exposing additional variables to the BODY argument. > > Alternatively, the pattern of `(let ((return (...))) ... return)` can be > replaced with `(prog1 (...) ...)`. > > @Eli: The new patch looks better and closer to how the issue is solved > in the Ruby sqlite3 gem. I'm still not sure about this use of > `unwind-protect` being correct, but it does preserve the backtrace in > case of an error better than when I used `(condition-case e (...) (error > (apply #'signal e)))`. OK, how about the below? diff --git a/lisp/sqlite.el b/lisp/sqlite.el index aad0aa4..8a52573 100644 --- a/lisp/sqlite.el +++ b/lisp/sqlite.el @@ -24,19 +24,28 @@ ;;; 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))) + (func-var (gensym)) + (res-var (gensym)) + (commit-var (gensym))) `(let ((,db-var ,db) - (,func-var (lambda () ,@body))) + (,func-var (lambda () ,@body)) + ,res-var ,commit-var) (if (sqlite-available-p) (unwind-protect (progn (sqlite-transaction ,db-var) - (funcall ,func-var)) - (sqlite-commit ,db-var)) - (funcall ,func-var))))) + (setq ,res-var (funcall ,func-var)) + (setq ,commit-var (sqlite-commit ,db-var)) + ,res-var) + (or ,commit-var (sqlite-rollback ,db-var)))) + (funcall ,func-var)))) (provide 'sqlite) From debbugs-submit-bounces@debbugs.gnu.org Mon Nov 20 16:14:18 2023 Received: (at 67142) by debbugs.gnu.org; 20 Nov 2023 21:14:18 +0000 Received: from localhost ([127.0.0.1]:54764 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r5Baz-0007dZ-U5 for submit@debbugs.gnu.org; Mon, 20 Nov 2023 16:14:18 -0500 Received: from mout-p-202.mailbox.org ([2001:67c:2050:0:465::202]:38950) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r5Bav-0007dH-SP for 67142@debbugs.gnu.org; Mon, 20 Nov 2023 16:14:16 -0500 Received: from smtp2.mailbox.org (smtp2.mailbox.org [IPv6:2001:67c:2050:b231:465::2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-202.mailbox.org (Postfix) with ESMTPS id 4SZ0cX36GCz9stW; Mon, 20 Nov 2023 22:14:04 +0100 (CET) Date: Mon, 20 Nov 2023 22:14:00 +0100 From: Vasilij Schneidermann To: Eli Zaretskii Subject: Re: bug#67142: 29.1; with-sqlite-transaction commits on exception rather than rolling back Message-ID: References: <83pm09pz2n.fsf@gnu.org> <831qcoq0py.fsf@gnu.org> <87a5rc4vwy.fsf@gmail.com> <83leavmjnu.fsf@gnu.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="euL6N/Z18P9gtxFJ" Content-Disposition: inline In-Reply-To: <83leavmjnu.fsf@gnu.org> X-Rspamd-Queue-Id: 4SZ0cX36GCz9stW X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 67142 Cc: 67142@debbugs.gnu.org, visuweshm@gmail.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) --euL6N/Z18P9gtxFJ Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable > OK, how about the below? >=20 > diff --git a/lisp/sqlite.el b/lisp/sqlite.el > index aad0aa4..8a52573 100644 > --- a/lisp/sqlite.el > +++ b/lisp/sqlite.el > @@ -24,19 +24,28 @@ > ;;; Code: > =20 > (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))) > + (func-var (gensym)) > + (res-var (gensym)) > + (commit-var (gensym))) > `(let ((,db-var ,db) > - (,func-var (lambda () ,@body))) > + (,func-var (lambda () ,@body)) > + ,res-var ,commit-var) > (if (sqlite-available-p) > (unwind-protect > (progn > (sqlite-transaction ,db-var) > - (funcall ,func-var)) > - (sqlite-commit ,db-var)) > - (funcall ,func-var))))) > + (setq ,res-var (funcall ,func-var)) > + (setq ,commit-var (sqlite-commit ,db-var)) > + ,res-var) > + (or ,commit-var (sqlite-rollback ,db-var)))) > + (funcall ,func-var)))) > =20 > (provide 'sqlite) Thank you, this looks very good. I've tested it on my own code using the SQLite support and there don't appear to be any (obvious issues). --euL6N/Z18P9gtxFJ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEE0dAcySl3bqM8O17WFmfJg6zCifoFAmVbzBUACgkQFmfJg6zC ifpoMggAgXuZ1jMQWyaGV+p97ui4SwIFoVh1fV5FX1n6Ql5GXLWHUjTbdOPMsp5D ldV9MIe5+QEvYB2woaro+LjxEE7P5GjLhJhC6Seb+YzD7N7L/ydNQarNCWHFz1ln kIUfLSHRPwJC00P5/gV64wxMbPz4QzAMLNaZC3tJcaSI3uUZUaXNhh36r96Y33Q4 yrswlxMCXvoyeoDWOs9pQwVHqpDgWxDbtDJI9KnUkfE/S7ptaV43MG61REBetEHz LKVYfsEqVr/vZmfNit1OCsFxwe+k+byrq4aTq628a9kYHWDEb2DOa08OYR297UBG NV3iO16BWK0RDpxfEKFEjg4sOCluSQ== =uG0y -----END PGP SIGNATURE----- --euL6N/Z18P9gtxFJ-- From debbugs-submit-bounces@debbugs.gnu.org Tue Nov 21 08:40:17 2023 Received: (at 67142-done) by debbugs.gnu.org; 21 Nov 2023 13:40:17 +0000 Received: from localhost ([127.0.0.1]:55509 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r5QzA-0008L5-LB for submit@debbugs.gnu.org; Tue, 21 Nov 2023 08:40:16 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:60686) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r5Qz8-0008Kk-Lh for 67142-done@debbugs.gnu.org; Tue, 21 Nov 2023 08:40:15 -0500 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1r5Qz0-0008Fe-2l; Tue, 21 Nov 2023 08:40:06 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=B0lVT9FpX+qjY6Ehv5Oj7Qdik8vxV0/sD59AgREXc7U=; b=KQQZn9Fumo4F OGnixUwhtEZoSRLwqvR1D/CvWm4sCI9B58daLvBSiAwJN04wd7tzeY7HumlY591idRKLE08qvDrUz ixS388yoiOPr2wuhGljHdMVKA4EQnlm46o5rqDpB3BG71epnIfYJtpY22OFunpBheaheMwqL/p7Ey VCY1jYQ4YgJC+0kRAdFazGJljlejWN8rYBkau1OLV2WJOE/eQ/wpQzEhf2PkjWnq+xAzcorxX9JHO DsgaKsiX5DQsiCv7/u9qIuSJSNqKoDZoiNT9TJvfZsJ3gmnXmaKFbxR9QbtjZkJp0rKotrzFBupyD wR08fWe/dsPo+CMI5ZZy4w==; Date: Tue, 21 Nov 2023 15:39:52 +0200 Message-Id: <831qcji5sn.fsf@gnu.org> From: Eli Zaretskii To: Vasilij Schneidermann In-Reply-To: (message from Vasilij Schneidermann on Mon, 20 Nov 2023 22:14:00 +0100) Subject: Re: bug#67142: 29.1; with-sqlite-transaction commits on exception rather than rolling back References: <83pm09pz2n.fsf@gnu.org> <831qcoq0py.fsf@gnu.org> <87a5rc4vwy.fsf@gmail.com> <83leavmjnu.fsf@gnu.org> X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 67142-done Cc: 67142-done@debbugs.gnu.org, visuweshm@gmail.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > Date: Mon, 20 Nov 2023 22:14:00 +0100 > From: Vasilij Schneidermann > Cc: visuweshm@gmail.com, 67142@debbugs.gnu.org > > Thank you, this looks very good. I've tested it on my own code using the > SQLite support and there don't appear to be any (obvious issues). Thanks for testing and for the valuable feedback, I've now installed this on the emacs-29 branch, and I'm therefore closing this bug. From unknown Sat Jun 21 05:18:42 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Wed, 20 Dec 2023 12:24:06 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator From debbugs-submit-bounces@debbugs.gnu.org Sun Mar 03 08:42:02 2024 Received: (at control) by debbugs.gnu.org; 3 Mar 2024 13:42:02 +0000 Received: from localhost ([127.0.0.1]:39896 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rgm6M-0006UN-D2 for submit@debbugs.gnu.org; Sun, 03 Mar 2024 08:42:02 -0500 Received: from mail-108-mta22.mxroute.com ([136.175.108.22]:33601) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rgm6I-0006U1-Nw for control@debbugs.gnu.org; Sun, 03 Mar 2024 08:42:00 -0500 Received: from filter006.mxroute.com ([136.175.111.2] filter006.mxroute.com) (Authenticated sender: mN4UYu2MZsgR) by mail-108-mta22.mxroute.com (ZoneMTA) with ESMTPSA id 18e048c410b0003bea.001 for (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384); Sun, 03 Mar 2024 13:41:24 +0000 X-Zone-Loop: 658e17e947c63a45f8fada3dc44991dadc161c76ff84 X-Originating-IP: [136.175.111.2] DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=neverwas.me ; s=x; h=Content-Type:MIME-Version:Message-ID:Date:Subject:To:From:Sender: Reply-To:Cc:Content-Transfer-Encoding:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: In-Reply-To:References:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=tWfFDjA/Be4YICHe+h0toxu8f9U7xTUJtPESKNbaQzc=; b=IF9HOhztn75YpwCO07GAKz2Aoo mrT4WxQVBjZcGbDGoAHVQnat6DA2ZFx6vxp/jjQBCEuo9v0JxkiFarKIq9/OwAfJXvaV5bzWBaQ0D xPi01uHr4POTFTC98P3JOxSAty9Re+KfQ2tANiUG3V7Gj0g5JYkGeN3PPFlpfrWLdOF1tqVfwx2KY NuJKUGSsCS7ftjJqwKXTggdiGggxqR8bZRgUJL2W1+n9fk/2Wc9p29/c7hyEJAKLq0C3eqKc80c4w dQWTMcdqEZ0ZHVd3ws039MHejMMTFltnq6sPZX/LrRCFTWF4leWj+Ib+FYFIa2LxRtnxudZzjbcIL cjTubi1A==; From: "J.P." To: control@debbugs.gnu.org Subject: control message for bug #67142 Date: Sun, 03 Mar 2024 05:41:19 -0800 Message-ID: <87v863v3eo.fsf@neverwas.me> MIME-Version: 1.0 Content-Type: text/plain X-Authenticated-Id: masked@neverwas.me X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: control X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) unarchive 67142 quit From debbugs-submit-bounces@debbugs.gnu.org Sun Mar 03 09:05:27 2024 Received: (at 67142) by debbugs.gnu.org; 3 Mar 2024 14:05:27 +0000 Received: from localhost ([127.0.0.1]:39904 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rgmT0-0001SP-Li for submit@debbugs.gnu.org; Sun, 03 Mar 2024 09:05:26 -0500 Received: from mail-108-mta204.mxroute.com ([136.175.108.204]:36317) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rgmSx-0001SC-Ts for 67142@debbugs.gnu.org; Sun, 03 Mar 2024 09:05:25 -0500 Received: from filter006.mxroute.com ([136.175.111.2] filter006.mxroute.com) (Authenticated sender: mN4UYu2MZsgR) by mail-108-mta204.mxroute.com (ZoneMTA) with ESMTPSA id 18e04a1be470003bea.001 for <67142@debbugs.gnu.org> (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384); Sun, 03 Mar 2024 14:04:52 +0000 X-Zone-Loop: 6f1c5b7fb94913e9ac9d75f27cb41dec63d9da7eb8fa X-Originating-IP: [136.175.111.2] DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=neverwas.me ; s=x; h=Content-Type:MIME-Version:Message-ID:Date:References:In-Reply-To: Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=kuTbdzEIjvjA6NpwoDB/+GJdweoL3ukyDgq8DO++nNE=; b=Vum4R7Ys57KGCHMDiYsap6tV+A 2eKweP7VkBQuENxz9R4nzWd8/b25AhzQgZI3wnwJw1aBQebt9qVgZH95MTs5/LPxj3GC93N4Avc4R svGukks0CHPFXwCyeeMAjuFmzzHs6oTxZ8bh2uQH0DkHsub0gUqou8zESqglrGCDpJXT4Yhy7TAwG sPO15TZuxd9J2om8ItbWQtOgR2sYD98Cl/e+HNDJfxQV7IjbrQDzaPjpUqn07DjtRcQ6QKnj1z5vU FhPup/KRSBi6XULpPHyxlZpeErcx/sQIjv3onWeE58e9V9UREcJxvaGOx2SRQSHxMSJaskkkl/pN5 alEcbfGw==; From: "J.P." To: Vasilij Schneidermann Subject: Re: bug#67142: 29.1; with-sqlite-transaction commits on exception rather than rolling back In-Reply-To: (Vasilij Schneidermann's message of "Mon, 20 Nov 2023 22:14:00 +0100") References: <83pm09pz2n.fsf@gnu.org> <831qcoq0py.fsf@gnu.org> <87a5rc4vwy.fsf@gmail.com> <83leavmjnu.fsf@gnu.org> Date: Sun, 03 Mar 2024 06:04:48 -0800 Message-ID: <87wmqjtnr3.fsf@neverwas.me> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-Authenticated-Id: masked@neverwas.me X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: 67142 Cc: Eli Zaretskii , visuweshm@gmail.com, 67142@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Vasilij Schneidermann writes: >> OK, how about the below? >> >> diff --git a/lisp/sqlite.el b/lisp/sqlite.el >> index aad0aa4..8a52573 100644 >> --- a/lisp/sqlite.el >> +++ b/lisp/sqlite.el >> @@ -24,19 +24,28 @@ >> ;;; 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))) >> + (func-var (gensym)) >> + (res-var (gensym)) >> + (commit-var (gensym))) >> `(let ((,db-var ,db) >> - (,func-var (lambda () ,@body))) >> + (,func-var (lambda () ,@body)) >> + ,res-var ,commit-var) >> (if (sqlite-available-p) >> (unwind-protect >> (progn >> (sqlite-transaction ,db-var) >> - (funcall ,func-var)) >> - (sqlite-commit ,db-var)) >> - (funcall ,func-var))))) >> + (setq ,res-var (funcall ,func-var)) >> + (setq ,commit-var (sqlite-commit ,db-var)) >> + ,res-var) >> + (or ,commit-var (sqlite-rollback ,db-var)))) >> + (funcall ,func-var)))) >> >> (provide 'sqlite) > > Thank you, this looks very good. I've tested it on my own code using the > SQLite support and there don't appear to be any (obvious issues). Not obvious on account of the indentation, but it seems the second (funcall ,func-var) snuck outside the `if' form with that fix, making body run twice if you have SQLite. From debbugs-submit-bounces@debbugs.gnu.org Sun Mar 10 23:43:56 2024 Received: (at 67142) by debbugs.gnu.org; 11 Mar 2024 03:43:56 +0000 Received: from localhost ([127.0.0.1]:38251 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rjWZv-00077P-RQ for submit@debbugs.gnu.org; Sun, 10 Mar 2024 23:43:56 -0400 Received: from mail-108-mta213.mxroute.com ([136.175.108.213]:33393) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rjWZs-00077G-Kq for 67142@debbugs.gnu.org; Sun, 10 Mar 2024 23:43:54 -0400 Received: from filter006.mxroute.com ([136.175.111.2] filter006.mxroute.com) (Authenticated sender: mN4UYu2MZsgR) by mail-108-mta213.mxroute.com (ZoneMTA) with ESMTPSA id 18e2b9b801e0003bea.001 for <67142@debbugs.gnu.org> (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384); Mon, 11 Mar 2024 03:43:14 +0000 X-Zone-Loop: bd352ef7cd39230672d8953fa5785ea06139564e63d3 X-Originating-IP: [136.175.111.2] DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=neverwas.me ; s=x; h=Content-Type:MIME-Version:Message-ID:Date:References:In-Reply-To: Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=cR16cKNSXgGjCIQxLq0zIlwrk3975hQUZELFrlf8RhU=; b=SlChtjKIGEXmZ6vB2qLBJwGQH4 SXqghjz7WqeFs4DSdEv7CIpEiOdzyN66dVEkCC6QsimRsokPB1e0d7MbtPVTiuY/a0R8DEDbvmXy7 yV8OvwdaC1JlCYPd9uXxmD9bglUcV3kMXvGNLKXNrST+UkMOWact3P0T66kcNB5HCDcIbakrkbAR2 su0ND3bnrjBheyklmmKIJD34PdKLwDATS2HgLYetKd/OKb+gOt8Xmf2Y0Pt1lHOdMDBltHIycITCN 6BHiJPTx2+dCO5gdEF+eg9DS8WyURSpUWE3kMyceErPDtPLsk2qE/iQO1/QWlhX/+t43gacVJCsEX deVDhSog==; From: "J.P." To: 67142@debbugs.gnu.org Subject: Re: bug#67142: 29.1; with-sqlite-transaction commits on exception rather than rolling back In-Reply-To: <87wmqjtnr3.fsf@neverwas.me> (J. P.'s message of "Sun, 03 Mar 2024 06:04:48 -0800") References: <83pm09pz2n.fsf@gnu.org> <831qcoq0py.fsf@gnu.org> <87a5rc4vwy.fsf@gmail.com> <83leavmjnu.fsf@gnu.org> <87wmqjtnr3.fsf@neverwas.me> Date: Sun, 10 Mar 2024 20:43:04 -0700 Message-ID: <87o7bl77sn.fsf@neverwas.me> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Authenticated-Id: masked@neverwas.me X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: 67142 Cc: eliz@gnu.org, mail@vasilij.de, visuweshm@gmail.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) --=-=-= Content-Type: text/plain "J.P." writes: > Not obvious on account of the indentation, but it seems the second > > (funcall ,func-var) > > snuck outside the `if' form with that fix, making body run twice if you > have SQLite. I guess folks are too busy to get excited about fixing this. Here's a patch in case that ever changes. Thanks. --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0001-Run-successful-body-once-in-with-sqlite-transaction.patch >From e9319447a1403736cd8aac766d29ba2d98591eff Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Sun, 10 Mar 2024 20:13:42 -0700 Subject: [PATCH] Run successful body once in with-sqlite-transaction * lisp/sqlite.el (with-sqlite-transaction): Tuck misplaced body of else form back into feature-test control structure whence it escaped. * test/lisp/sqlite-tests.el: New file to accompany test/src/sqlite-tests.el. (Bug#67142) --- lisp/sqlite.el | 7 +++--- test/lisp/sqlite-tests.el | 51 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 test/lisp/sqlite-tests.el diff --git a/lisp/sqlite.el b/lisp/sqlite.el index 46e35ac18d8..efc5997fb5c 100644 --- a/lisp/sqlite.el +++ b/lisp/sqlite.el @@ -32,7 +32,8 @@ with-sqlite-transaction 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." +back the transaction changes before allowing the signal to +propagate." (declare (indent 1) (debug (form body))) (let ((db-var (gensym)) (func-var (gensym)) @@ -48,8 +49,8 @@ with-sqlite-transaction (setq ,res-var (funcall ,func-var)) (setq ,commit-var (sqlite-commit ,db-var)) ,res-var) - (or ,commit-var (sqlite-rollback ,db-var)))) - (funcall ,func-var)))) + (or ,commit-var (sqlite-rollback ,db-var))) + (funcall ,func-var))))) (provide 'sqlite) diff --git a/test/lisp/sqlite-tests.el b/test/lisp/sqlite-tests.el new file mode 100644 index 00000000000..d4892a27efc --- /dev/null +++ b/test/lisp/sqlite-tests.el @@ -0,0 +1,51 @@ +;;; sqlite-tests.el --- Tests for sqlite.el -*- lexical-binding: t; -*- + +;; Copyright (C) 2024 Free Software Foundation, Inc. + +;; This file is part of GNU Emacs. + +;; GNU Emacs is free software: you can redistribute it and/or modify +;; it under the terms of the GNU General Public License as published by +;; the Free Software Foundation, either version 3 of the License, or +;; (at your option) any later version. + +;; GNU Emacs is distributed in the hope that it will be useful, +;; but WITHOUT ANY WARRANTY; without even the implied warranty of +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;; GNU General Public License for more details. + +;; You should have received a copy of the GNU General Public License +;; along with GNU Emacs. If not, see . + +;;; Commentary: + +;;; Code: +(require 'sqlite) + +(ert-deftest with-sqlite-transaction () + (skip-unless (sqlite-available-p)) + (let ((db (sqlite-open))) + (sqlite-execute db "create table test (a)") + (should + (eql 42 (with-sqlite-transaction db + (sqlite-execute db "insert into test values (1)") + (should (equal '((1)) (sqlite-select db "select * from test"))) + 42))) + ;; Body runs exactly once. + (should (equal '((1)) (sqlite-select db "select * from test"))))) + +(ert-deftest with-sqlite-transaction/rollback () + (skip-unless (sqlite-available-p)) + (let ((db (sqlite-open))) + (sqlite-execute db "create table test (a)") + (should (equal '(sqlite-error + ("SQL logic error" "no such function: fake" 1 1)) + (should-error + (with-sqlite-transaction db + (sqlite-execute db "insert into test values (1)") + (sqlite-execute db "insert into test values (fake(2))") + 42)))) + ;; First insertion (a=1) rolled back. + (should-not (sqlite-select db "select * from test")))) + +;;; sqlite-tests.el ends here -- 2.44.0 --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Mon Mar 11 09:26:08 2024 Received: (at 67142) by debbugs.gnu.org; 11 Mar 2024 13:26:08 +0000 Received: from localhost ([127.0.0.1]:39244 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rjffL-0000wF-Ua for submit@debbugs.gnu.org; Mon, 11 Mar 2024 09:26:08 -0400 Received: from eggs.gnu.org ([209.51.188.92]:48774) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rjffK-0000vn-Vx for 67142@debbugs.gnu.org; Mon, 11 Mar 2024 09:26:07 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rjfef-0005P4-ML; Mon, 11 Mar 2024 09:25:25 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=hpsGF2SeeEEyZ/yDqNQdM7Qjfzz+sjVnNo84Zw/HGuY=; b=Bf8p0C+6zjpp S5fUGt7fbAeFjxJWWSI0L7c9cTFmCFBjK9RrR/ivNJYR4P3Zde0e7qRIMw4uEARC+FEJci0L3MOYH gisZBJjyvqk9tbWE36bIdMomWXZVypKNMuFXhYjKXFDw2cLHCTo56mB9OU109dGZbhUk+31PtBpno T0QHmr6mMz7RAD6jqg10Hyav7V+Rph4Gj2U3yQB3KHeLtN+I5Woavr+wWXOSc3Oq5YM7HGEocIRwL enx2j+/R5VFaHpCiTKJjV2Ic/Lzzy+cQtEJrK0tcb4yxDdFdAZKrrwlK2REVcUtvQP4FQdAxvR0Dq fyA1HySEIzdkSFnBWmllUQ==; Date: Mon, 11 Mar 2024 15:25:11 +0200 Message-Id: <86a5n4vr2g.fsf@gnu.org> From: Eli Zaretskii To: "J.P." In-Reply-To: <87o7bl77sn.fsf@neverwas.me> (jp@neverwas.me) Subject: Re: bug#67142: 29.1; with-sqlite-transaction commits on exception rather than rolling back References: <83pm09pz2n.fsf@gnu.org> <831qcoq0py.fsf@gnu.org> <87a5rc4vwy.fsf@gmail.com> <83leavmjnu.fsf@gnu.org> <87wmqjtnr3.fsf@neverwas.me> <87o7bl77sn.fsf@neverwas.me> X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 67142 Cc: visuweshm@gmail.com, mail@vasilij.de, 67142@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > From: "J.P." > Cc: eliz@gnu.org, visuweshm@gmail.com, mail@vasilij.de > Date: Sun, 10 Mar 2024 20:43:04 -0700 > > "J.P." writes: > > > Not obvious on account of the indentation, but it seems the second > > > > (funcall ,func-var) > > > > snuck outside the `if' form with that fix, making body run twice if you > > have SQLite. > > I guess folks are too busy to get excited about fixing this. Here's a > patch in case that ever changes. Thanks. Thanks, installed on the emacs-29 branch. From unknown Sat Jun 21 05:18:42 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Tue, 09 Apr 2024 11:24:06 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator