GNU bug report logs - #69292
[PATCH 0/6] Prepare the database code for use in the daemon

Previous Next

Full log

Message #41 received at 69292 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Christopher Baines <mail <at> cbaines.net>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 69292 <at> debbugs.gnu.org, Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#69292] [PATCH 3/6] store: database: Inline SQL to where
 it's used.
Date: Fri, 23 Feb 2024 17:40:29 +0100

Christopher Baines <mail <at> cbaines.net> skribis:

> This makes the code easier to read, as you don't have to keep jumping between
> the two places.
>
> * guix/store/database.scm (path-id-sql, update-sql, insert-sql,
> add-reference-sql): Remove variables.
> (path-id, update-or-insert, add-references): Include SQL.
>
> Change-Id: I53b4ab973be8d0cd10a0f35ba25972f1c9680353

LGTM.

>    (let ((id (path-id db path)))
>      (if id
> -        (let ((stmt (sqlite-prepare db update-sql #:cache? #t)))
> +        (let ((stmt (sqlite-prepare
> +                     db
> +                     "
> +UPDATE ValidPaths
> +SET hash = :hash,
> +    registrationTime = :time,
> +    deriver = :deriver,
> +    narSize = :size
> +WHERE id = :id"
> +                     #:cache? #t)))

I think we can make it a bit more dense (3 or 4 lines in total for the
statement).  :-)

In the future, we should probably add a macro to sqlite3 like that one
from Cuirass:

--8<---------------cut here---------------start------------->8---
(define-syntax-rule (exec-query/bind db query args ...)
  "Execute the specific QUERY with the given ARGS.  Uses of 'exec-query/bind'
typically look like this:

  (exec-query/bind db \"SELECT * FROM Foo WHERE x = \" x \"AND Y=\" y \";\")

References to variables 'x' and 'y' here are replaced by $1 and $2 in the
SQL query.

This ensures that (1) SQL injection is impossible, and (2) the number of
parameters matches the number of arguments to bind."
  (%exec-query/bind db () "" query args ...))
--8<---------------cut here---------------end--------------->8---

That makes things slightly more readable IMO since you don’t end up with
two separate calls, one to prepare the statement and the other one to
bind its arguments.

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.