From 3d119f0a3b65b765a1451796c2b86c0b331d3599 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 30 Mar 2016 15:50:45 +0200 Subject: [PATCH] Improve the SQLite wrapper API In particular, this eliminates a bunch of boilerplate code. --- src/libstore/local-store.cc | 235 ++++++++++-------------------------- src/libstore/local-store.hh | 7 +- src/libstore/sqlite.cc | 104 +++++++++------- src/libstore/sqlite.hh | 55 ++++++--- src/libstore/store-api.hh | 4 +- 5 files changed, 169 insertions(+), 236 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 789eff68a..ab087452b 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -332,6 +332,7 @@ void LocalStore::openDB(bool create) // ensure efficient lookup. stmtQueryPathFromHashPart.create(db, "select path from ValidPaths where path >= ? limit 1;"); + stmtQueryValidPaths.create(db, "select path from ValidPaths"); } @@ -526,23 +527,16 @@ void LocalStore::checkDerivationOutputs(const Path & drvPath, const Derivation & } -unsigned long long LocalStore::addValidPath(const ValidPathInfo & info, bool checkOutputs) +uint64_t LocalStore::addValidPath(const ValidPathInfo & info, bool checkOutputs) { - SQLiteStmtUse use(stmtRegisterValidPath); - stmtRegisterValidPath.bind(info.path); - stmtRegisterValidPath.bind("sha256:" + printHash(info.narHash)); - stmtRegisterValidPath.bind(info.registrationTime == 0 ? time(0) : info.registrationTime); - if (info.deriver != "") - stmtRegisterValidPath.bind(info.deriver); - else - stmtRegisterValidPath.bind(); // null - if (info.narSize != 0) - stmtRegisterValidPath.bind64(info.narSize); - else - stmtRegisterValidPath.bind(); // null - if (sqlite3_step(stmtRegisterValidPath) != SQLITE_DONE) - throwSQLiteError(db, format("registering valid path ‘%1%’ in database") % info.path); - unsigned long long id = sqlite3_last_insert_rowid(db); + stmtRegisterValidPath.use() + (info.path) + ("sha256:" + printHash(info.narHash)) + (info.registrationTime == 0 ? time(0) : info.registrationTime) + (info.deriver, info.deriver != "") + (info.narSize, info.narSize != 0) + .exec(); + uint64_t id = sqlite3_last_insert_rowid(db); /* If this is a derivation, then store the derivation outputs in the database. This is useful for the garbage collector: it can @@ -559,12 +553,11 @@ unsigned long long LocalStore::addValidPath(const ValidPathInfo & info, bool che if (checkOutputs) checkDerivationOutputs(info.path, drv); for (auto & i : drv.outputs) { - SQLiteStmtUse use(stmtAddDerivationOutput); - stmtAddDerivationOutput.bind(id); - stmtAddDerivationOutput.bind(i.first); - stmtAddDerivationOutput.bind(i.second.path); - if (sqlite3_step(stmtAddDerivationOutput) != SQLITE_DONE) - throwSQLiteError(db, format("adding derivation output for ‘%1%’ in database") % info.path); + stmtAddDerivationOutput.use() + (id) + (i.first) + (i.second.path) + .exec(); } } @@ -572,24 +565,16 @@ unsigned long long LocalStore::addValidPath(const ValidPathInfo & info, bool che } -void LocalStore::addReference(unsigned long long referrer, unsigned long long reference) +void LocalStore::addReference(uint64_t referrer, uint64_t reference) { - SQLiteStmtUse use(stmtAddReference); - stmtAddReference.bind(referrer); - stmtAddReference.bind(reference); - if (sqlite3_step(stmtAddReference) != SQLITE_DONE) - throwSQLiteError(db, "adding reference to database"); + stmtAddReference.use()(referrer)(reference).exec(); } void LocalStore::registerFailedPath(const Path & path) { retrySQLite([&]() { - SQLiteStmtUse use(stmtRegisterFailedPath); - stmtRegisterFailedPath.bind(path); - stmtRegisterFailedPath.bind(time(0)); - if (sqlite3_step(stmtRegisterFailedPath) != SQLITE_DONE) - throwSQLiteError(db, format("registering failed path ‘%1%’") % path); + stmtRegisterFailedPath.use()(path)(time(0)).step(); }); } @@ -597,12 +582,7 @@ void LocalStore::registerFailedPath(const Path & path) bool LocalStore::hasPathFailed(const Path & path) { return retrySQLite([&]() { - SQLiteStmtUse use(stmtHasPathFailed); - stmtHasPathFailed.bind(path); - int res = sqlite3_step(stmtHasPathFailed); - if (res != SQLITE_DONE && res != SQLITE_ROW) - throwSQLiteError(db, "querying whether path failed"); - return res == SQLITE_ROW; + return stmtHasPathFailed.use()(path).next(); }); } @@ -610,18 +590,11 @@ bool LocalStore::hasPathFailed(const Path & path) PathSet LocalStore::queryFailedPaths() { return retrySQLite([&]() { - SQLiteStmtUse use(stmtQueryFailedPaths); + auto useQueryFailedPaths(stmtQueryFailedPaths.use()); PathSet res; - int r; - while ((r = sqlite3_step(stmtQueryFailedPaths)) == SQLITE_ROW) { - const char * s = (const char *) sqlite3_column_text(stmtQueryFailedPaths, 0); - assert(s); - res.insert(s); - } - - if (r != SQLITE_DONE) - throwSQLiteError(db, "error querying failed paths"); + while (useQueryFailedPaths.next()) + res.insert(useQueryFailedPaths.getStr(0)); return res; }); @@ -633,12 +606,8 @@ void LocalStore::clearFailedPaths(const PathSet & paths) retrySQLite([&]() { SQLiteTxn txn(db); - for (auto & i : paths) { - SQLiteStmtUse use(stmtClearFailedPath); - stmtClearFailedPath.bind(i); - if (sqlite3_step(stmtClearFailedPath) != SQLITE_DONE) - throwSQLiteError(db, format("clearing failed path ‘%1%’ in database") % i); - } + for (auto & path : paths) + stmtClearFailedPath.use()(path).exec(); txn.commit(); }); @@ -669,41 +638,28 @@ ValidPathInfo LocalStore::queryPathInfo(const Path & path) return retrySQLite([&]() { /* Get the path info. */ - SQLiteStmtUse use1(stmtQueryPathInfo); + auto useQueryPathInfo(stmtQueryPathInfo.use()(path)); - stmtQueryPathInfo.bind(path); + if (!useQueryPathInfo.next()) + throw Error(format("path ‘%1%’ is not valid") % path); - int r = sqlite3_step(stmtQueryPathInfo); - if (r == SQLITE_DONE) throw Error(format("path ‘%1%’ is not valid") % path); - if (r != SQLITE_ROW) throwSQLiteError(db, "querying path in database"); + info.id = useQueryPathInfo.getInt(0); - info.id = sqlite3_column_int(stmtQueryPathInfo, 0); + info.narHash = parseHashField(path, useQueryPathInfo.getStr(1)); - const char * s = (const char *) sqlite3_column_text(stmtQueryPathInfo, 1); - assert(s); - info.narHash = parseHashField(path, s); + info.registrationTime = useQueryPathInfo.getInt(2); - info.registrationTime = sqlite3_column_int(stmtQueryPathInfo, 2); - - s = (const char *) sqlite3_column_text(stmtQueryPathInfo, 3); + auto s = (const char *) sqlite3_column_text(stmtQueryPathInfo, 3); if (s) info.deriver = s; /* Note that narSize = NULL yields 0. */ - info.narSize = sqlite3_column_int64(stmtQueryPathInfo, 4); + info.narSize = useQueryPathInfo.getInt(4); /* Get the references. */ - SQLiteStmtUse use2(stmtQueryReferences); + auto useQueryReferences(stmtQueryReferences.use()(info.id)); - stmtQueryReferences.bind(info.id); - - while ((r = sqlite3_step(stmtQueryReferences)) == SQLITE_ROW) { - s = (const char *) sqlite3_column_text(stmtQueryReferences, 0); - assert(s); - info.references.insert(s); - } - - if (r != SQLITE_DONE) - throwSQLiteError(db, format("error getting references of ‘%1%’") % path); + while (useQueryReferences.next()) + info.references.insert(useQueryReferences.getStr(0)); return info; }); @@ -714,37 +670,26 @@ ValidPathInfo LocalStore::queryPathInfo(const Path & path) narSize field. */ void LocalStore::updatePathInfo(const ValidPathInfo & info) { - SQLiteStmtUse use(stmtUpdatePathInfo); - if (info.narSize != 0) - stmtUpdatePathInfo.bind64(info.narSize); - else - stmtUpdatePathInfo.bind(); // null - stmtUpdatePathInfo.bind("sha256:" + printHash(info.narHash)); - stmtUpdatePathInfo.bind(info.path); - if (sqlite3_step(stmtUpdatePathInfo) != SQLITE_DONE) - throwSQLiteError(db, format("updating info of path ‘%1%’ in database") % info.path); + stmtUpdatePathInfo.use() + (info.narSize, info.narSize != 0) + ("sha256:" + printHash(info.narHash)) + (info.path) + .exec(); } -unsigned long long LocalStore::queryValidPathId(const Path & path) +uint64_t LocalStore::queryValidPathId(const Path & path) { - SQLiteStmtUse use(stmtQueryPathInfo); - stmtQueryPathInfo.bind(path); - int res = sqlite3_step(stmtQueryPathInfo); - if (res == SQLITE_ROW) return sqlite3_column_int(stmtQueryPathInfo, 0); - if (res == SQLITE_DONE) throw Error(format("path ‘%1%’ is not valid") % path); - throwSQLiteError(db, "querying path in database"); + auto use(stmtQueryPathInfo.use()(path)); + if (!use.next()) + throw Error(format("path ‘%1%’ is not valid") % path); + return use.getInt(0); } bool LocalStore::isValidPath_(const Path & path) { - SQLiteStmtUse use(stmtQueryPathInfo); - stmtQueryPathInfo.bind(path); - int res = sqlite3_step(stmtQueryPathInfo); - if (res != SQLITE_DONE && res != SQLITE_ROW) - throwSQLiteError(db, "querying path in database"); - return res == SQLITE_ROW; + return stmtQueryPathInfo.use()(path).next(); } @@ -770,20 +715,9 @@ PathSet LocalStore::queryValidPaths(const PathSet & paths) PathSet LocalStore::queryAllValidPaths() { return retrySQLite([&]() { - SQLiteStmt stmt; - stmt.create(db, "select path from ValidPaths"); - + auto use(stmtQueryValidPaths.use()); PathSet res; - int r; - while ((r = sqlite3_step(stmt)) == SQLITE_ROW) { - const char * s = (const char *) sqlite3_column_text(stmt, 0); - assert(s); - res.insert(s); - } - - if (r != SQLITE_DONE) - throwSQLiteError(db, "error getting valid paths"); - + while (use.next()) res.insert(use.getStr(0)); return res; }); } @@ -791,19 +725,10 @@ PathSet LocalStore::queryAllValidPaths() void LocalStore::queryReferrers_(const Path & path, PathSet & referrers) { - SQLiteStmtUse use(stmtQueryReferrers); + auto useQueryReferrers(stmtQueryReferrers.use()(path)); - stmtQueryReferrers.bind(path); - - int r; - while ((r = sqlite3_step(stmtQueryReferrers)) == SQLITE_ROW) { - const char * s = (const char *) sqlite3_column_text(stmtQueryReferrers, 0); - assert(s); - referrers.insert(s); - } - - if (r != SQLITE_DONE) - throwSQLiteError(db, format("error getting references of ‘%1%’") % path); + while (useQueryReferrers.next()) + referrers.insert(useQueryReferrers.getStr(0)); } @@ -827,19 +752,11 @@ PathSet LocalStore::queryValidDerivers(const Path & path) assertStorePath(path); return retrySQLite([&]() { - SQLiteStmtUse use(stmtQueryValidDerivers); - stmtQueryValidDerivers.bind(path); + auto useQueryValidDerivers(stmtQueryValidDerivers.use()(path)); PathSet derivers; - int r; - while ((r = sqlite3_step(stmtQueryValidDerivers)) == SQLITE_ROW) { - const char * s = (const char *) sqlite3_column_text(stmtQueryValidDerivers, 1); - assert(s); - derivers.insert(s); - } - - if (r != SQLITE_DONE) - throwSQLiteError(db, format("error getting valid derivers of ‘%1%’") % path); + while (useQueryValidDerivers.next()) + derivers.insert(useQueryValidDerivers.getStr(1)); return derivers; }); @@ -849,19 +766,11 @@ PathSet LocalStore::queryValidDerivers(const Path & path) PathSet LocalStore::queryDerivationOutputs(const Path & path) { return retrySQLite([&]() { - SQLiteStmtUse use(stmtQueryDerivationOutputs); - stmtQueryDerivationOutputs.bind(queryValidPathId(path)); + auto useQueryDerivationOutputs(stmtQueryDerivationOutputs.use()(queryValidPathId(path))); PathSet outputs; - int r; - while ((r = sqlite3_step(stmtQueryDerivationOutputs)) == SQLITE_ROW) { - const char * s = (const char *) sqlite3_column_text(stmtQueryDerivationOutputs, 1); - assert(s); - outputs.insert(s); - } - - if (r != SQLITE_DONE) - throwSQLiteError(db, format("error getting outputs of ‘%1%’") % path); + while (useQueryDerivationOutputs.next()) + outputs.insert(useQueryDerivationOutputs.getStr(1)); return outputs; }); @@ -871,19 +780,11 @@ PathSet LocalStore::queryDerivationOutputs(const Path & path) StringSet LocalStore::queryDerivationOutputNames(const Path & path) { return retrySQLite([&]() { - SQLiteStmtUse use(stmtQueryDerivationOutputs); - stmtQueryDerivationOutputs.bind(queryValidPathId(path)); + auto useQueryDerivationOutputs(stmtQueryDerivationOutputs.use()(queryValidPathId(path))); StringSet outputNames; - int r; - while ((r = sqlite3_step(stmtQueryDerivationOutputs)) == SQLITE_ROW) { - const char * s = (const char *) sqlite3_column_text(stmtQueryDerivationOutputs, 0); - assert(s); - outputNames.insert(s); - } - - if (r != SQLITE_DONE) - throwSQLiteError(db, format("error getting output names of ‘%1%’") % path); + while (useQueryDerivationOutputs.next()) + outputNames.insert(useQueryDerivationOutputs.getStr(0)); return outputNames; }); @@ -897,12 +798,9 @@ Path LocalStore::queryPathFromHashPart(const string & hashPart) Path prefix = settings.nixStore + "/" + hashPart; return retrySQLite([&]() { - SQLiteStmtUse use(stmtQueryPathFromHashPart); - stmtQueryPathFromHashPart.bind(prefix); + auto useQueryPathFromHashPart(stmtQueryPathFromHashPart.use()(prefix)); - int res = sqlite3_step(stmtQueryPathFromHashPart); - if (res == SQLITE_DONE) return ""; - if (res != SQLITE_ROW) throwSQLiteError(db, "finding path in database"); + if (!useQueryPathFromHashPart.next()) return ""; const char * s = (const char *) sqlite3_column_text(stmtQueryPathFromHashPart, 0); return s && prefix.compare(0, prefix.size(), s, prefix.size()) == 0 ? s : ""; @@ -1143,7 +1041,7 @@ void LocalStore::registerValidPaths(const ValidPathInfos & infos) } for (auto & i : infos) { - unsigned long long referrer = queryValidPathId(i.path); + auto referrer = queryValidPathId(i.path); for (auto & j : i.references) addReference(referrer, queryValidPathId(j)); } @@ -1178,12 +1076,7 @@ void LocalStore::invalidatePath(const Path & path) drvHashes.erase(path); - SQLiteStmtUse use(stmtInvalidatePath); - - stmtInvalidatePath.bind(path); - - if (sqlite3_step(stmtInvalidatePath) != SQLITE_DONE) - throwSQLiteError(db, format("invalidating path ‘%1%’ in database") % path); + stmtInvalidatePath.use()(path).exec(); /* Note that the foreign key constraints on the Refs table take care of deleting the references entries for `path'. */ diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index e8c7ff4c4..1b1721185 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -208,6 +208,7 @@ private: SQLiteStmt stmtQueryValidDerivers; SQLiteStmt stmtQueryDerivationOutputs; SQLiteStmt stmtQueryPathFromHashPart; + SQLiteStmt stmtQueryValidPaths; /* Cache for pathContentsGood(). */ std::map pathContentsGoodCache; @@ -230,11 +231,11 @@ private: void makeStoreWritable(); - unsigned long long queryValidPathId(const Path & path); + uint64_t queryValidPathId(const Path & path); - unsigned long long addValidPath(const ValidPathInfo & info, bool checkOutputs = true); + uint64_t addValidPath(const ValidPathInfo & info, bool checkOutputs = true); - void addReference(unsigned long long referrer, unsigned long long reference); + void addReference(uint64_t referrer, uint64_t reference); void appendReferrer(const Path & from, const Path & to, bool lock); diff --git a/src/libstore/sqlite.cc b/src/libstore/sqlite.cc index 8646ff5b1..0f1bb4947 100644 --- a/src/libstore/sqlite.cc +++ b/src/libstore/sqlite.cc @@ -53,15 +53,6 @@ void SQLiteStmt::create(sqlite3 * db, const string & s) this->db = db; } -void SQLiteStmt::reset() -{ - assert(stmt); - /* Note: sqlite3_reset() returns the error code for the most - recent call to sqlite3_step(). So ignore it. */ - sqlite3_reset(stmt); - curArg = 1; -} - SQLiteStmt::~SQLiteStmt() { try { @@ -72,43 +63,74 @@ SQLiteStmt::~SQLiteStmt() } } -void SQLiteStmt::bind(const string & value) -{ - if (sqlite3_bind_text(stmt, curArg++, value.c_str(), -1, SQLITE_TRANSIENT) != SQLITE_OK) - throwSQLiteError(db, "binding argument"); -} - -void SQLiteStmt::bind(int value) -{ - if (sqlite3_bind_int(stmt, curArg++, value) != SQLITE_OK) - throwSQLiteError(db, "binding argument"); -} - -void SQLiteStmt::bind64(long long value) -{ - if (sqlite3_bind_int64(stmt, curArg++, value) != SQLITE_OK) - throwSQLiteError(db, "binding argument"); -} - -void SQLiteStmt::bind() -{ - if (sqlite3_bind_null(stmt, curArg++) != SQLITE_OK) - throwSQLiteError(db, "binding argument"); -} - -SQLiteStmtUse::SQLiteStmtUse(SQLiteStmt & stmt) +SQLiteStmt::Use::Use(SQLiteStmt & stmt) : stmt(stmt) { - stmt.reset(); + assert(stmt.stmt); + /* Note: sqlite3_reset() returns the error code for the most + recent call to sqlite3_step(). So ignore it. */ + sqlite3_reset(stmt); } -SQLiteStmtUse::~SQLiteStmtUse() +SQLiteStmt::Use & SQLiteStmt::Use::operator () (const std::string & value, bool notNull) { - try { - stmt.reset(); - } catch (...) { - ignoreException(); - } + if (notNull) { + if (sqlite3_bind_text(stmt, curArg++, value.c_str(), -1, SQLITE_TRANSIENT) != SQLITE_OK) + throwSQLiteError(stmt.db, "binding argument"); + } else + bind(); + return *this; +} + +SQLiteStmt::Use & SQLiteStmt::Use::operator () (int64_t value, bool notNull) +{ + if (notNull) { + if (sqlite3_bind_int64(stmt, curArg++, value) != SQLITE_OK) + throwSQLiteError(stmt.db, "binding argument"); + } else + bind(); + return *this; +} + +SQLiteStmt::Use & SQLiteStmt::Use::bind() +{ + if (sqlite3_bind_null(stmt, curArg++) != SQLITE_OK) + throwSQLiteError(stmt.db, "binding argument"); + return *this; +} + +int SQLiteStmt::Use::step() +{ + return sqlite3_step(stmt); +} + +void SQLiteStmt::Use::exec() +{ + int r = step(); + assert(r != SQLITE_ROW); + if (r != SQLITE_DONE) + throwSQLiteError(stmt.db, "executing SQLite statement"); +} + +bool SQLiteStmt::Use::next() +{ + int r = step(); + if (r != SQLITE_DONE && r != SQLITE_ROW) + throwSQLiteError(stmt.db, "executing SQLite query"); + return r == SQLITE_ROW; +} + +std::string SQLiteStmt::Use::getStr(int col) +{ + auto s = (const char *) sqlite3_column_text(stmt, col); + assert(s); + return s; +} + +int64_t SQLiteStmt::Use::getInt(int col) +{ + // FIXME: detect nulls? + return sqlite3_column_int64(stmt, col); } SQLiteTxn::SQLiteTxn(sqlite3 * db) diff --git a/src/libstore/sqlite.hh b/src/libstore/sqlite.hh index 0abdb7463..b95397841 100644 --- a/src/libstore/sqlite.hh +++ b/src/libstore/sqlite.hh @@ -22,29 +22,46 @@ struct SQLite /* RAII wrapper to create and destroy SQLite prepared statements. */ struct SQLiteStmt { - sqlite3 * db; - sqlite3_stmt * stmt; - unsigned int curArg; - SQLiteStmt() { stmt = 0; } + sqlite3 * db = 0; + sqlite3_stmt * stmt = 0; + SQLiteStmt() { } void create(sqlite3 * db, const std::string & s); - void reset(); ~SQLiteStmt(); operator sqlite3_stmt * () { return stmt; } - void bind(const std::string & value); - void bind(int value); - void bind64(long long value); - void bind(); -}; -/* Helper class to ensure that prepared statements are reset when - leaving the scope that uses them. Unfinished prepared statements - prevent transactions from being aborted, and can cause locks to be - kept when they should be released. */ -struct SQLiteStmtUse -{ - SQLiteStmt & stmt; - SQLiteStmtUse(SQLiteStmt & stmt); - ~SQLiteStmtUse(); + /* Helper for binding / executing statements. */ + class Use + { + friend struct SQLiteStmt; + private: + SQLiteStmt & stmt; + unsigned int curArg = 1; + Use(SQLiteStmt & stmt); + + public: + + /* Bind the next parameter. */ + Use & operator () (const std::string & value, bool notNull = true); + Use & operator () (int64_t value, bool notNull = true); + Use & bind(); // null + + int step(); + + /* Execute a statement that does not return rows. */ + void exec(); + + /* For statements that return 0 or more rows. Returns true iff + a row is available. */ + bool next(); + + std::string getStr(int col); + int64_t getInt(int col); + }; + + Use use() + { + return Use(*this); + } }; /* RAII helper that ensures transactions are aborted unless explicitly diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index b7209d4a3..7c6e4c079 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -96,8 +96,8 @@ struct ValidPathInfo Hash narHash; PathSet references; time_t registrationTime = 0; - unsigned long long narSize = 0; // 0 = unknown - unsigned long long id; // internal use only + uint64_t narSize = 0; // 0 = unknown + uint64_t id; // internal use only /* Whether the path is ultimately trusted, that is, it was built locally or is content-addressable (e.g. added via addToStore()