Re: Cuirass news

[Danny Milosavljevic]

Hi Ludo,

On Sun, 28 Jan 2018 22:47:34 +0100
address@hidden (Ludovic Courtès) wrote:

> I’ve already applied two commits in civodul/guile-sqlite3.  I think 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.  
> 
> I think we should stick to a single repo for guile-sqlite3, though it
> prolly shouldn’t be called “civodul/”.  Perhaps you 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’d (though 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’d still be caching potentially a lot more
> than needed.  For instance, we don’t know whether a statement is a
> one-off statement (used only once, for instance because it’s derived
> from user parameters passed through the HTTP API or something), 

Well, that shouldn't happen because it would allow SQL injection.

I usually write filters like (:parameter IS NULL OR (foo = :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’m inclined to not try to be smart and instead
> 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 query
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.