Hi Ludo, On Thu, 08 Feb 2018 23:21:58 +0100 l...@gnu.org (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!
