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 <decker.christian@gmail.com>

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));
+}

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 */

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,