diff mbox series

[bug#69292,6/6] store: database: Rename a couple of procedures.

Message ID 12f872394c84039fa553e50381d040de24f24d3e.1708457946.git.mail@cbaines.net
State New
Headers show
Series Prepare the database code for use in the daemon | expand

Commit Message

Christopher Baines Feb. 20, 2024, 7:39 p.m. UTC
These names should be more descriptive.

* guix/store/database.scm (path-id): Rename to select-valid-path-id.
(sqlite-register): Rename to register-valid-path.
(register-items): Update accordingly.

Change-Id: I6d4a14d4cde9d71ab34d6ffdbfbfde51b2c0e1db
---
 guix/store/database.scm | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

Comments

Ludovic Courtès Feb. 23, 2024, 4:43 p.m. UTC | #1
Christopher Baines <mail@cbaines.net> skribis:

> These names should be more descriptive.
>
> * guix/store/database.scm (path-id): Rename to select-valid-path-id.
> (sqlite-register): Rename to register-valid-path.
> (register-items): Update accordingly.
>
> Change-Id: I6d4a14d4cde9d71ab34d6ffdbfbfde51b2c0e1db

OK for ‘register-valid-path’.

For ‘path-id’ my preference would be ‘valid-path-id’ or keeping
‘path-id’; ‘select’ looks odd as nothing’s being “selected” here in the
non-SQL sense of the word.

Ludo’.
Christopher Baines Feb. 26, 2024, 11:03 a.m. UTC | #2
Ludovic Courtès <ludo@gnu.org> writes:

> Christopher Baines <mail@cbaines.net> skribis:
>
>> These names should be more descriptive.
>>
>> * guix/store/database.scm (path-id): Rename to select-valid-path-id.
>> (sqlite-register): Rename to register-valid-path.
>> (register-items): Update accordingly.
>>
>> Change-Id: I6d4a14d4cde9d71ab34d6ffdbfbfde51b2c0e1db
>
> OK for ‘register-valid-path’.
>
> For ‘path-id’ my preference would be ‘valid-path-id’ or keeping
> ‘path-id’; ‘select’ looks odd as nothing’s being “selected” here in the
> non-SQL sense of the word.

The main thing I'm trying to do here is make the procedures that
directly interact with the database stand out (similar to the ! suffix
convention). Maybe the procedure taking the db as an argument is enough,
but to me path-id/valid-path-id look too much like record accessors.
Ludovic Courtès Feb. 27, 2024, 9:24 a.m. UTC | #3
Christopher Baines <mail@cbaines.net> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Christopher Baines <mail@cbaines.net> skribis:
>>
>>> These names should be more descriptive.
>>>
>>> * guix/store/database.scm (path-id): Rename to select-valid-path-id.
>>> (sqlite-register): Rename to register-valid-path.
>>> (register-items): Update accordingly.
>>>
>>> Change-Id: I6d4a14d4cde9d71ab34d6ffdbfbfde51b2c0e1db
>>
>> OK for ‘register-valid-path’.
>>
>> For ‘path-id’ my preference would be ‘valid-path-id’ or keeping
>> ‘path-id’; ‘select’ looks odd as nothing’s being “selected” here in the
>> non-SQL sense of the word.
>
> The main thing I'm trying to do here is make the procedures that
> directly interact with the database stand out (similar to the ! suffix
> convention). Maybe the procedure taking the db as an argument is enough,
> but to me path-id/valid-path-id look too much like record accessors.

I’d say that having an explicit ‘db’ argument is enough (similar to
procedures that take a ‘store’ argument).

Ludo’.
Christopher Baines April 3, 2024, 5:35 p.m. UTC | #4
Ludovic Courtès <ludo@gnu.org> writes:

> Christopher Baines <mail@cbaines.net> skribis:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>>
>>> Christopher Baines <mail@cbaines.net> skribis:
>>>
>>>> These names should be more descriptive.
>>>>
>>>> * guix/store/database.scm (path-id): Rename to select-valid-path-id.
>>>> (sqlite-register): Rename to register-valid-path.
>>>> (register-items): Update accordingly.
>>>>
>>>> Change-Id: I6d4a14d4cde9d71ab34d6ffdbfbfde51b2c0e1db
>>>
>>> OK for ‘register-valid-path’.
>>>
>>> For ‘path-id’ my preference would be ‘valid-path-id’ or keeping
>>> ‘path-id’; ‘select’ looks odd as nothing’s being “selected” here in the
>>> non-SQL sense of the word.
>>
>> The main thing I'm trying to do here is make the procedures that
>> directly interact with the database stand out (similar to the ! suffix
>> convention). Maybe the procedure taking the db as an argument is enough,
>> but to me path-id/valid-path-id look too much like record accessors.
>
> I’d say that having an explicit ‘db’ argument is enough (similar to
> procedures that take a ‘store’ argument).

I've changed it to valid-path-id now.
diff mbox series

Patch

diff --git a/guix/store/database.scm b/guix/store/database.scm
index 0b570eabcd..0190696ad5 100644
--- a/guix/store/database.scm
+++ b/guix/store/database.scm
@@ -40,8 +40,10 @@  (define-module (guix store database)
             store-database-file
             call-with-database
             with-database
-            path-id
-            sqlite-register
+
+            select-valid-path-id
+
+            register-valid-path
             register-items
             %epoch
             reset-timestamps
@@ -181,9 +183,9 @@  (define (last-insert-row-id db)
     (vector-ref (sqlite-step-and-reset stmt)
                 0)))
 
-(define* (path-id db path)
-  "If PATH exists in the 'ValidPaths' table, return its numerical
-identifier.  Otherwise, return #f."
+(define (select-valid-path-id db path)
+  "If PATH exists in the 'ValidPaths' table, return its numerical identifier.
+Otherwise, return #f."
   (let ((stmt (sqlite-prepare
                db
                "
@@ -229,9 +231,9 @@  (define-inlinable (assert-integer proc in-range? key number)
            "Integer ~A out of range: ~S" (list key number)
            (list number))))
 
-(define* (sqlite-register db #:key path (references '())
-                          deriver hash nar-size
-                          (time (timestamp)))
+(define* (register-valid-path db #:key path (references '())
+                              deriver hash nar-size
+                              (time (timestamp)))
   "Registers this stuff in DB.  PATH is the store item to register and
 REFERENCES is the list of store items PATH refers to; DERIVER is the '.drv'
 that produced PATH, HASH is the base16-encoded Nix sha256 hash of
@@ -249,7 +251,7 @@  (define* (sqlite-register db #:key path (references '())
   (assert-integer "sqlite-register" (cut >= <> 0) #:time registration-time)
 
   (define id
-    (let ((existing-id (path-id db path)))
+    (let ((existing-id (select-valid-path-id db path)))
       (if existing-id
           (let ((stmt (sqlite-prepare
                        db
@@ -287,7 +289,8 @@  (define* (sqlite-register db #:key path (references '())
   ;; Call 'path-id' on each of REFERENCES.  This ensures we get a
   ;; "non-NULL constraint" failure if one of REFERENCES is unregistered.
   (add-references db id
-                  (map (cut path-id db <>) references)))
+                  (map (cut select-valid-path-id db <>) references)))
+
 
 
 ;;;
@@ -364,18 +367,18 @@  (define* (register-items db items
     ;; When TO-REGISTER is already registered, skip it.  This makes a
     ;; significant differences when 'register-closures' is called
     ;; consecutively for overlapping closures such as 'system' and 'bootcfg'.
-    (unless (path-id db to-register)
+    (unless (select-valid-path-id db to-register)
       (let-values (((hash nar-size) (nar-sha256 real-file-name)))
         (call-with-retrying-transaction db
           (lambda ()
-            (sqlite-register db #:path to-register
-                             #:references (store-info-references item)
-                             #:deriver (store-info-deriver item)
-                             #:hash (string-append
-                                     "sha256:"
-                                     (bytevector->base16-string hash))
-                             #:nar-size nar-size
-                             #:time registration-time))))))
+            (register-valid-path db #:path to-register
+                                 #:references (store-info-references item)
+                                 #:deriver (store-info-deriver item)
+                                 #:hash (string-append
+                                         "sha256:"
+                                         (bytevector->base16-string hash))
+                                 #:nar-size nar-size
+                                 #:time registration-time))))))
 
   (let* ((prefix   (format #f "registering ~a items" (length items)))
          (progress (progress-reporter/bar (length items)