[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#41658] [PATCH] fixes / improvements for (guix store database)
From: |
Ludovic Courtès |
Subject: |
[bug#41658] [PATCH] fixes / improvements for (guix store database) |
Date: |
Thu, 04 Jun 2020 18:40:35 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Hi,
Thanks for the thorough investigation and for the patches!
Caleb Ristvedt <caleb.ristvedt@cune.org> skribis:
> From cce653c590be1506e15044e445aa9805370ac759 Mon Sep 17 00:00:00 2001
> From: Caleb Ristvedt <caleb.ristvedt@cune.org>
> Date: Mon, 1 Jun 2020 18:50:07 -0500
> Subject: [PATCH 1/4] database: work around guile-sqlite3 bug preventing
> statement reset
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> guile-sqlite3 provides statement caching, making it unnecessary for sqlite to
> keep re-preparing statements that are frequently used. Unfortunately it
> doesn't quite emulate the semantics of sqlite_finalize properly, because it
> doesn't cause a commit if the statement being finalized is the last "active"
> statement. We work around this by wrapping sqlite-finalize with our own
> version that ensures sqlite-reset is called, which does The Right Thing™.
>
> * guix/store/database.scm (sqlite-finalize): new procedure that shadows the
> sqlite-finalize from (sqlite3).
Nice. It would be great if you could report it upstream (Danny and/or
myself can then patch it directly in guile-sqlite3 and push out a
release) and refer to the issue from here.
We can have this patch locally in the meantime, unless it would break
once the new guile-sqlite3 is out. WDYT?
> From ee24ab21122b1c75a7d67d7062550e15e54ab62f Mon Sep 17 00:00:00 2001
> From: Caleb Ristvedt <caleb.ristvedt@cune.org>
> Date: Mon, 1 Jun 2020 19:21:43 -0500
> Subject: [PATCH 2/4] database: rewrite query procedures in terms of
> with-statement.
>
> Most of our queries would fail to finalize their statements properly if sqlite
> returned an error during their execution. This resolves that, and also makes
> them somewhat more concise as a side-effect.
>
> This also makes some small changes to improve certain queries where behavior
> was strange or overly verbose.
>
> * guix/store/database.scm (call-with-statement): new procedure.
> (with-statement): new macro.
> (last-insert-row-id, path-id, update-or-insert, add-references): rewrite to
> use with-statement.
> (update-or-insert): factor last-insert-row-id out of the end of both
> branches.
> (add-references): remove pointless last-insert-row-id call.
>
> * .dir-locals.el (with-statement): add indenting information.
LGTM!
> From 7d34c27c33aed3e8a49b9796a62a8c19d352e653 Mon Sep 17 00:00:00 2001
> From: Caleb Ristvedt <caleb.ristvedt@cune.org>
> Date: Mon, 1 Jun 2020 21:43:14 -0500
> Subject: [PATCH 3/4] database: ensure update-or-insert is run within a
> transaction
>
> update-or-insert can break if an insert occurs between when it decides whether
> to update or insert and when it actually performs that operation. Putting the
> check and the update/insert operation in the same transaction ensures that the
> update/insert will only succeed if no other write has occurred in the middle.
>
> * guix/store/database.scm (call-with-savepoint): new procedure.
> (update-or-insert): use call-with-savepoint to ensure the read and the
> insert/update occur within the same transaction.
That’s a bit beyond my understanding, but I think you can also push this
one. :-)
Make sure “make check TESTS=tests/store-database.scm” is still happy.
Thanks a lot!
Ludo’.