From mboxrd@z Thu Jan 1 00:00:00 1970 From: Danny Milosavljevic Subject: Re: Cuirass news Date: Sun, 28 Jan 2018 23:23:43 +0100 Message-ID: <20180128232343.152cfe86@scratchpost.org> References: <877es6x5xj.fsf@gnu.org> <87lggmjjgo.fsf@gmail.com> <87k1w6jjak.fsf@gmail.com> <87h8raxeym.fsf@gnu.org> <20180126153005.259a75e8@scratchpost.org> <87zi4z1eb0.fsf@gnu.org> <20180127181852.42f0bcbf@scratchpost.org> <87fu6pzmdl.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:44552) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1efvMf-0007O7-Vr for guix-devel@gnu.org; Sun, 28 Jan 2018 17:23:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1efvMc-0000bA-Ro for guix-devel@gnu.org; Sun, 28 Jan 2018 17:23:53 -0500 In-Reply-To: <87fu6pzmdl.fsf@gnu.org> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Ludovic =?ISO-8859-1?Q?Court=E8s?= Cc: guix-devel Hi Ludo, On Sun, 28 Jan 2018 22:47:34 +0100 ludo@gnu.org (Ludovic Court=C3=A8s) wrote: > I=E2=80=99ve already applied two commits in civodul/guile-sqlite3. I thi= nk the > statement cache requires some more work though (see below). Nice. > > If that's OK I'll replace the reference in guix-master, or we could do a > > pull request to the civodul repository. =20 >=20 > I think we should stick to a single repo for guile-sqlite3, though it > prolly shouldn=E2=80=99t be called =E2=80=9Ccivodul/=E2=80=9D. Perhaps y= ou can create > guile-sqlite3/guile-sqlite3 and add the two of us plus Andy there for a > start? Hmm, should I create a new user account on notabug for that? > The problem is that interned symbols are potentially not GC=E2=80=99d (th= ough I > think with Guile 2.2 and its weak sets they may be subject to GC.) Too bad. If that's the case it's not as good as it could be. > The other issue is that we=E2=80=99d still be caching potentially a lot m= ore > than needed. For instance, we don=E2=80=99t know whether a statement is a > one-off statement (used only once, for instance because it=E2=80=99s deri= ved > from user parameters passed through the HTTP API or something),=20 Well, that shouldn't happen because it would allow SQL injection. I usually write filters like (:parameter IS NULL OR (foo =3D :parameter)) so that the SQL text doesn't change. On the other hand, if only the parameter values change it will reuse the same statement. > Thinking more about it, I=E2=80=99m inclined to not try to be smart and i= nstead > let users explicitly ask for caching when they want to. Whatever else, I think it would be good to actually use prepared statements for what they are for ;-) That they prevent sql injection is a nice bonus but what they are supposed = to do is prevent the database engine from having to parse text and build a que= ry plan every time someone changes one bit somewhere. So I'm still for "caching" those. I've left sqlite-exec inside guix-cuirass rather than guile-sqlite3 so that the user of the guile-sqlite3 can decide caching, among other things. We could also have two macros, sqlite-exec-reuse and sqlite-exec (right now it decides that implicitly by whether the SQL text is a string literal or not - I think in practise that's what cuirass will always do). The stuff that's in dannym guile-sqlite3 has no new effect if you don't use sqlite-prepare* (note star) - so it should be quite conservative already. sqlite-prepare* is written in a way that it always caches - I purposefully left the two other cases with non-literal strings out. So you can't sqlite-prepare* with a non-literal string. And maybe also remove the arg bindings from in there. Then the new API would be: sqlite-prepare for new prepared statements, sqlite-prepare* for caching prepared statements.