all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Christopher Baines <mail@cbaines.net>
To: 69292@debbugs.gnu.org
Cc: "Christopher Baines" <guix@cbaines.net>,
	"Josselin Poiret" <dev@jpoiret.xyz>,
	"Ludovic Courtès" <ludo@gnu.org>,
	"Mathieu Othacehe" <othacehe@gnu.org>,
	"Ricardo Wurmus" <rekado@elephly.net>,
	"Simon Tournier" <zimon.toutoune@gmail.com>,
	"Tobias Geerinckx-Rice" <me@tobias.gr>
Subject: [bug#69292] [PATCH 2/6] store: database: Remove with-statement and associated code.
Date: Tue, 20 Feb 2024 19:39:02 +0000	[thread overview]
Message-ID: <b7360abb08559073653effea98a99332fc1f8071.1708457946.git.mail@cbaines.net> (raw)
In-Reply-To: <4b6a268daab5e0b307dff2229d551a47c9fe1ebc.1708457946.git.mail@cbaines.net>

I think using dynamic-wind to finalize all statements is the wrong
approach. Firstly it would be good to allow reseting statements rather than
finalizing them. Then for the problem of handling errors, the approach I've
settled on in the build coordinator is to close the database connection, since
that'll trigger guile-sqlite3 to finalize all the cached statements.

This reverts commit 5d6e2255286e591def122ec2f4a3cbda497fea21.

* .dir-locals.el (scheme-mode): Remove with-statement.
* guix/store/database.scm (call-with-statement): Remove procedure.
(with-statement): Remove syntax rule.
(call-with-transaction, last-insert-row-id, path-id, update-or-insert,
add-references): Don't use with-statement.

Change-Id: I2fd976b3f12ec8105cc56350933a953cf53647e8
---
 .dir-locals.el          |  1 -
 guix/store/database.scm | 62 ++++++++++++++++++-----------------------
 2 files changed, 27 insertions(+), 36 deletions(-)

diff --git a/.dir-locals.el b/.dir-locals.el
index f135eb69a5..2d1a03c313 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -131,7 +131,6 @@
    (eval . (put 'with-database 'scheme-indent-function 2))
    (eval . (put 'call-with-database 'scheme-indent-function 1))
    (eval . (put 'call-with-transaction 'scheme-indent-function 1))
-   (eval . (put 'with-statement 'scheme-indent-function 3))
    (eval . (put 'call-with-retrying-transaction 'scheme-indent-function 1))
 
    (eval . (put 'call-with-container 'scheme-indent-function 1))
diff --git a/guix/store/database.scm b/guix/store/database.scm
index 3093fd816a..de72b79860 100644
--- a/guix/store/database.scm
+++ b/guix/store/database.scm
@@ -130,25 +130,22 @@ (define* (call-with-transaction db proc #:key restartable?)
 the transaction, otherwise commit the transaction after it finishes.
 RESTARTABLE? may be set to a non-#f value when it is safe to run PROC multiple
 times.  This may reduce contention for the database somewhat."
-  (define (exec sql)
-    (with-statement db sql stmt
-      (sqlite-fold cons '() stmt)))
   ;; We might use begin immediate here so that if we need to retry, we figure
   ;; that out immediately rather than because some SQLITE_BUSY exception gets
   ;; thrown partway through PROC - in which case the part already executed
   ;; (which may contain side-effects!) might have to be executed again for
   ;; every retry.
-  (exec (if restartable? "begin;" "begin immediate;"))
+  (sqlite-exec db (if restartable? "begin;" "begin immediate;"))
   (catch #t
     (lambda ()
       (let-values ((result (proc)))
-        (exec "commit;")
+        (sqlite-exec db "commit;")
         (apply values result)))
     (lambda args
       ;; The roll back may or may not have occurred automatically when the
       ;; error was generated. If it has occurred, this does nothing but signal
       ;; an error. If it hasn't occurred, this needs to be done.
-      (false-if-exception (exec "rollback;"))
+      (false-if-exception (sqlite-exec db "rollback;"))
       (apply throw args))))
 
 (define* (call-with-retrying-transaction db proc #:key restartable?)
@@ -170,26 +167,14 @@ (define-syntax with-database
     ((_ file db exp ...)
      (call-with-database file (lambda (db) exp ...)))))
 
-(define (call-with-statement db sql proc)
-  (let ((stmt (sqlite-prepare db sql #:cache? #t)))
-    (dynamic-wind
-      (const #t)
-      (lambda ()
-        (proc stmt))
-      (lambda ()
-        (sqlite-finalize stmt)))))
-
-(define-syntax-rule (with-statement db sql stmt exp ...)
-  "Run EXP... with STMT bound to a prepared statement corresponding to the sql
-string SQL for DB."
-  (call-with-statement db sql
-                       (lambda (stmt) exp ...)))
-
 (define (last-insert-row-id db)
   ;; XXX: (sqlite3) currently lacks bindings for 'sqlite3_last_insert_rowid'.
   ;; Work around that.
-  (with-statement db "SELECT last_insert_rowid();" stmt
-    (match (sqlite-fold cons '() stmt)
+  (let* ((stmt   (sqlite-prepare db "SELECT last_insert_rowid();"
+                                 #:cache? #t))
+         (result (sqlite-fold cons '() stmt)))
+    (sqlite-finalize stmt)
+    (match result
       ((#(id)) id)
       (_ #f))))
 
@@ -199,11 +184,13 @@ (define path-id-sql
 (define* (path-id db path)
   "If PATH exists in the 'ValidPaths' table, return its numerical
 identifier.  Otherwise, return #f."
-  (with-statement db path-id-sql stmt
+  (let ((stmt (sqlite-prepare db path-id-sql #:cache? #t)))
     (sqlite-bind-arguments stmt #:path path)
-    (match (sqlite-fold cons '() stmt)
-      ((#(id) . _) id)
-      (_ #f))))
+    (let ((result (sqlite-fold cons '() stmt)))
+      (sqlite-finalize stmt)
+      (match result
+        ((#(id) . _) id)
+        (_ #f)))))
 
 (define update-sql
   "UPDATE ValidPaths SET hash = :hash, registrationTime = :time, deriver =
@@ -235,17 +222,20 @@ (define* (update-or-insert db #:key path deriver hash nar-size time)
 
   (let ((id (path-id db path)))
     (if id
-        (with-statement db update-sql stmt
+        (let ((stmt (sqlite-prepare db update-sql #:cache? #t)))
           (sqlite-bind-arguments stmt #:id id
                                  #:deriver deriver
                                  #:hash hash #:size nar-size #:time time)
-          (sqlite-fold cons '() stmt))
-        (with-statement db insert-sql stmt
+          (sqlite-fold cons '() stmt)
+          (sqlite-finalize stmt)
+          (last-insert-row-id db))
+        (let ((stmt (sqlite-prepare db insert-sql #:cache? #t)))
           (sqlite-bind-arguments stmt
                                  #:path path #:deriver deriver
                                  #:hash hash #:size nar-size #:time time)
-          (sqlite-fold cons '() stmt)))
-    (last-insert-row-id db)))
+          (sqlite-fold cons '() stmt)             ;execute it
+          (sqlite-finalize stmt)
+          (last-insert-row-id db)))))
 
 (define add-reference-sql
   "INSERT OR REPLACE INTO Refs (referrer, reference) VALUES (:referrer, :reference);")
@@ -253,13 +243,15 @@ (define add-reference-sql
 (define (add-references db referrer references)
   "REFERRER is the id of the referring store item, REFERENCES is a list
 ids of items referred to."
-  (with-statement db add-reference-sql stmt
+  (let ((stmt (sqlite-prepare db add-reference-sql #:cache? #t)))
     (for-each (lambda (reference)
                 (sqlite-reset stmt)
                 (sqlite-bind-arguments stmt #:referrer referrer
                                        #:reference reference)
-                (sqlite-fold cons '() stmt))
-              references)))
+                (sqlite-fold cons '() stmt)       ;execute it
+                (last-insert-row-id db))
+              references)
+    (sqlite-finalize stmt)))
 
 (define (timestamp)
   "Return a timestamp, either the current time of SOURCE_DATE_EPOCH."
-- 
2.41.0





  reply	other threads:[~2024-02-20 19:40 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-20 19:20 [bug#69292] [PATCH 0/6] Prepare the database code for use in the daemon Christopher Baines
2024-02-20 19:39 ` [bug#69292] [PATCH 1/6] store: database: Remove call-with-savepoint and associated code Christopher Baines
2024-02-20 19:39   ` Christopher Baines [this message]
2024-02-23 16:35     ` [bug#69292] [PATCH 2/6] store: database: Remove with-statement " Ludovic Courtès
2024-02-23 18:32       ` Reepca Russelstein via Guix-patches via
2024-03-05 11:05         ` Ludovic Courtès
2024-02-20 19:39   ` [bug#69292] [PATCH 3/6] store: database: Inline SQL to where it's used Christopher Baines
2024-02-23 16:40     ` Ludovic Courtès
2024-02-20 19:39   ` [bug#69292] [PATCH 4/6] store: database: Stop finalizing prepared statements Christopher Baines
2024-02-22 12:39     ` reepca--- via Guix-patches via
2024-02-26 10:50       ` Christopher Baines
2024-02-20 19:39   ` [bug#69292] [PATCH 5/6] store: database: Refactor sqlite-register Christopher Baines
2024-02-23 16:33     ` Ludovic Courtès
2024-02-20 19:39   ` [bug#69292] [PATCH 6/6] store: database: Rename a couple of procedures Christopher Baines
2024-02-23 16:43     ` Ludovic Courtès
2024-02-26 11:03       ` Christopher Baines
2024-02-27  9:24         ` Ludovic Courtès
2024-04-03 17:35           ` Christopher Baines
2024-02-23 16:31   ` [bug#69292] [PATCH 1/6] store: database: Remove call-with-savepoint and associated code Ludovic Courtès
2024-04-03 17:36 ` bug#69292: [PATCH 0/6] Prepare the database code for use in the daemon Christopher Baines

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b7360abb08559073653effea98a99332fc1f8071.1708457946.git.mail@cbaines.net \
    --to=mail@cbaines.net \
    --cc=69292@debbugs.gnu.org \
    --cc=dev@jpoiret.xyz \
    --cc=guix@cbaines.net \
    --cc=ludo@gnu.org \
    --cc=me@tobias.gr \
    --cc=othacehe@gnu.org \
    --cc=rekado@elephly.net \
    --cc=zimon.toutoune@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.