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
next prev parent 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
List information: https://guix.gnu.org/
* 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 public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).