Re: Cuirass news

[Danny Milosavljevic]

Hi Ludo,

On Thu, 08 Feb 2018 23:21:58 +0100
address@hidden (Ludovic Courtès) wrote:

> > from a security standpoint - except for db-get-builds, which I'm amending
> > right now.  
> 
> Oh sorry, I think I did the same thing as you were sending this message:
> 
>   
> https://git.savannah.gnu.org/cgit/guix/guix-cuirass.git/commit/?id=8c7c93922bbe0513ff4c4ff3a6e554e3a72635b6

> WDYT?

I'd prefer not to have so many different SQL statements, we get a
combinatorial explosion if we aren't careful (whether we cache or not,
the relational database management system is going to hate us anyway
when we do that).

But I guess there are not that many yet.

If we are fine in not being able to search for literal NULL we can use NULL as
"anything" marker and have a static WHERE statement (this way is customary).

Also, I've asked on the sqlite mailing list - ORDER BY cannot support "?", so
those are unavoidable (also, we can't usefully do the ORDER BY ourselves
by sorting the result - because of the LIMIT clause)

Anyway as long as we are under 10000 statements it should be fine :P

> Indeed!  Should we change ‘sqlite-finalize’ to a noop when called on a
> cached statement?  (Otherwise users would have to keep track of whether
> or not a statement is cached.)

Hmm maybe that's a good way.  But its a little magic.

If you are not finalizing the statement, it will be reused anyway the next time
you use the same SQL text.  The user just shouldn't call finalize - which sounds
simple enough for him not to do.

I think having sqlite-exec detect literal SQL text is a nice middle way.

If the SQL text is a literal it means it's right there in the source code
and is probably not going to change - how would it?

Otherwise err on the side of caution and finalize the statement - it's
a little slower but safer that way.  I think that would pretty much only
mean db-get-builds.

Do you think that's too much magic?  Or more than the other way?  I wonder...

I think that if we do this magic we do it right there in the cuirass 
database.scm
module and it's never going to move into guile-sqlite3 :)

On the other hand, if we special-cached sqlite-finalize, we'd have to provide
sqlite-finalize* or something that does the freeing anyway...

> Besides, on the big database on berlin, the initial:
> 
>   (db-get-builds db '((status pending)))
> 
> call takes a lot of time and memory.  I guess we’re doing something
> wrong, but I’m not sure what.  The same query in the ‘sqlite3’ CLI is
> snappy and does not consume much memory.

WTF.  I'll have a look.

> One of the things we’re doing wrong is that ‘Outputs’ table: each
> ‘db-format-build’ call triggers a lookup in that table.  We should
> instead probably simply store output lists in the ‘Derivations’ table.

Definitely.  That's one of the things we should inline into db-get-builds.
Relational databases are good at joins, let's not to their work for them.

> Which also means we should have schema versioning and a way to upgrade…

Yeah.

> > I've also reintroduced sqlite-bind-args in a nicer version, please pull:
> > https://notabug.org/civodul/guile-sqlite3/pulls/3 .  
> 
> It is OK with you to write it like this:

Yes, looks good!  Thanks!