From 742bcdb2bcbd2ebdd0b1de4921e31214ba72d0d8 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 28 Aug 2019 16:25:42 +0200 Subject: [PATCH] db: Move statement expansion into the driver It's better to let the driver decide when and how to expand. It can then report the expanded statement back to the dispatch through the `db_changes_add` function. Signed-off-by: Christian Decker --- wallet/db.c | 51 ++++++++++++++------------------------------- wallet/db_common.h | 14 +++++++++---- wallet/db_sqlite3.c | 37 ++++++++++++++++++++------------ 3 files changed, 50 insertions(+), 52 deletions(-) diff --git a/wallet/db.c b/wallet/db.c index 07a19547f..f9e173d98 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -470,22 +470,6 @@ static void db_assert_no_outstanding_statements(struct db *db) } #endif -#if !HAVE_SQLITE3_EXPANDED_SQL -/* Prior to sqlite3 v3.14, we have to use tracing to dump statements */ -static void trace_sqlite3(void *dbv, const char *stmt) -{ - struct db *db = dbv; - - /* We get a "COMMIT;" after we've sent our changes. */ - if (!db->changes) { - assert(streq(stmt, "COMMIT;")); - return; - } - - tal_arr_expand(&db->changes, tal_strdup(db->changes, stmt)); -} -#endif - void db_stmt_done(sqlite3_stmt *stmt) { sqlite3_finalize(stmt); @@ -651,13 +635,6 @@ void db_exec_prepared_(const char *caller, struct db *db, sqlite3_stmt *stmt) if (sqlite3_step(stmt) != SQLITE_DONE) db_fatal("%s: %s", caller, sqlite3_errmsg(db->sql)); -#if HAVE_SQLITE3_EXPANDED_SQL - char *expanded_sql; - expanded_sql = sqlite3_expanded_sql(stmt); - tal_arr_expand(&db->changes, - tal_strdup(db->changes, expanded_sql)); - sqlite3_free(expanded_sql); -#endif db_stmt_done(stmt); } @@ -740,10 +717,6 @@ void db_commit_transaction(struct db *db) static void setup_open_db(struct db *db) { -#if !HAVE_SQLITE3_EXPANDED_SQL - sqlite3_trace(db->sql, trace_sqlite3, db); -#endif - /* This must be outside a transaction, so catch it */ assert(!db->in_transaction); @@ -1619,18 +1592,10 @@ void db_column_txid(struct db_stmt *stmt, int pos, struct bitcoin_txid *t) bool db_exec_prepared_v2(struct db_stmt *stmt TAKES) { - const char *expanded_sql; bool ret = stmt->db->config->exec_fn(stmt); stmt->executed = true; list_del_from(&stmt->db->pending_statements, &stmt->list); - if (stmt->db->config->expand_fn != NULL && ret && - !stmt->query->readonly) { - expanded_sql = stmt->db->config->expand_fn(stmt); - tal_arr_expand(&stmt->db->changes, - tal_strdup(stmt->db->changes, expanded_sql)); - } - /* The driver itself doesn't call `fatal` since we want to override it * for testing. Instead we check here that the error message is set if * we report an error. */ @@ -1656,3 +1621,19 @@ bool db_query_prepared(struct db_stmt *stmt) list_del_from(&stmt->db->pending_statements, &stmt->list); return ret; } + +void db_changes_add(struct db_stmt *stmt, const char * expanded) +{ + struct db *db = stmt->db; + + if (stmt->query->readonly) { + return; + } + /* We get a "COMMIT;" after we've sent our changes. */ + if (!db->changes) { + assert(streq(expanded, "COMMIT;")); + return; + } + + tal_arr_expand(&db->changes, tal_strdup(db->changes, expanded)); +} diff --git a/wallet/db_common.h b/wallet/db_common.h index 5652bdd6c..74b21b3ed 100644 --- a/wallet/db_common.h +++ b/wallet/db_common.h @@ -93,10 +93,6 @@ struct db_config { struct db_query *queries; size_t num_queries; - /* Function used to get a string representation of the executed query - * for the `db_write` plugin hook. */ - const char *(*expand_fn)(struct db_stmt *stmt); - /* Function used to execute a statement that doesn't result in a * response. */ bool (*exec_fn)(struct db_stmt *stmt); @@ -139,4 +135,14 @@ struct db_config { /* Provide a way for DB backends to register themselves */ AUTODATA_TYPE(db_backends, struct db_config); +/** + * Report a statement that changes the wallet + * + * Allows the DB driver to report an expanded statement during + * execution. Changes are queued up and reported to the `db_write` plugin hook + * upon committing. + */ +void db_changes_add(struct db_stmt *db_stmt, const char * expanded); + + #endif /* LIGHTNING_WALLET_DB_COMMON_H */ diff --git a/wallet/db_sqlite3.c b/wallet/db_sqlite3.c index 94194843c..5d568d456 100644 --- a/wallet/db_sqlite3.c +++ b/wallet/db_sqlite3.c @@ -7,20 +7,14 @@ #if HAVE_SQLITE3 -static const char *db_sqlite3_expand(struct db_stmt *stmt) +#if !HAVE_SQLITE3_EXPANDED_SQL +/* Prior to sqlite3 v3.14, we have to use tracing to dump statements */ +static void trace_sqlite3(void *stmtv, const char *stmt) { -#if HAVE_SQLITE3_EXPANDED_SQL - sqlite3_stmt *s = (sqlite3_stmt*)stmt->inner_stmt; - const char *sql; - char *expanded_sql; - expanded_sql = sqlite3_expanded_sql(s); - sql = tal_strdup(stmt, expanded_sql); - sqlite3_free(expanded_sql); - return sql; -#else - return NULL; -#endif + struct db_stmt = (struct db_stmt*)stmtv; + db_changes_add(stmt, stmt); } +#endif static const char *db_sqlite3_fmt_error(struct db_stmt *stmt) { @@ -89,6 +83,12 @@ static bool db_sqlite3_query(struct db_stmt *stmt) static bool db_sqlite3_exec(struct db_stmt *stmt) { int err; +#if !HAVE_SQLITE3_EXPANDED_SQL + /* Register the tracing function if we don't have an explicit way of + * expanding the statement. */ + sqlite3_trace(db->sql, trace_sqlite3, stmt); +#endif + if (!db_sqlite3_query(stmt)) { /* If the prepare step caused an error we hand it up. */ return false; @@ -101,6 +101,18 @@ static bool db_sqlite3_exec(struct db_stmt *stmt) return false; } +#if HAVE_SQLITE3_EXPANDED_SQL + /* Manually expand and call the callback */ + char *expanded_sql; + expanded_sql = sqlite3_expanded_sql(stmt->inner_stmt); + db_changes_add(stmt, expanded_sql); + sqlite3_free(expanded_sql); +#else + /* Unregister the trace callback to avoid it accessing the potentially + * stale pointer to stmt */ + sqlite3_trace(db->sql, NULL, NULL); +#endif + return true; } @@ -198,7 +210,6 @@ struct db_config db_sqlite3_config = { .name = "sqlite3", .queries = db_sqlite3_queries, .num_queries = DB_SQLITE3_QUERY_COUNT, - .expand_fn = &db_sqlite3_expand, .exec_fn = &db_sqlite3_exec, .query_fn = &db_sqlite3_query, .step_fn = &db_sqlite3_step,