From 3e052e8dac87b8b38af73bf68e5cdd03c56930f2 Mon Sep 17 00:00:00 2001 From: John Ralls Date: Fri, 1 Jun 2018 14:12:56 -0700 Subject: [PATCH] [DBI Backend] Rework backup-table management. Removes a public function, GncDbiSqlConnection::table_manage_backup that should have been private in the first place. Better encapsulates table renames and drops with private functions and handles cases where there exist some primary tables and some backup tables. --- .../backend/dbi/gnc-dbisqlconnection.cpp | 124 ++++++++---------- .../backend/dbi/gnc-dbisqlconnection.hpp | 5 +- 2 files changed, 57 insertions(+), 72 deletions(-) diff --git a/libgnucash/backend/dbi/gnc-dbisqlconnection.cpp b/libgnucash/backend/dbi/gnc-dbisqlconnection.cpp index 5abeef29d5..f1e361dc05 100644 --- a/libgnucash/backend/dbi/gnc-dbisqlconnection.cpp +++ b/libgnucash/backend/dbi/gnc-dbisqlconnection.cpp @@ -588,38 +588,28 @@ GncDbiSqlConnection::retry_connection(const char* msg) } PERR ("DBI error: %s - Giving up after %d consecutive attempts.\n", msg, - DBI_MAX_CONN_ATTEMPTS); + DBI_MAX_CONN_ATTEMPTS); m_conn_ok = false; return false; } +bool +GncDbiSqlConnection::rename_table(const std::string& old_name, + const std::string& new_name) +{ + std::string sql = "ALTER TABLE " + old_name + " RENAME TO " + new_name; + auto stmt = create_statement_from_sql(sql); + return execute_nonselect_statement(stmt) >= 0; +} -dbi_result -GncDbiSqlConnection::table_manage_backup (const std::string& table_name, - TableOpType op) +bool +GncDbiSqlConnection::drop_table(const std::string& table) { - auto new_name = table_name + "_back"; - dbi_result result = nullptr; - switch (op) - { - case TableOpType::backup: - result = dbi_conn_queryf (m_conn, "ALTER TABLE %s RENAME TO %s", - table_name.c_str(), new_name.c_str()); - break; - case TableOpType::rollback: - result = dbi_conn_queryf (m_conn, - "ALTER TABLE %s RENAME TO %s", - new_name.c_str(), table_name.c_str()); - break; - case TableOpType::drop_backup: - result = dbi_conn_queryf (m_conn, "DROP TABLE %s", - new_name.c_str()); - break; - default: - break; - } - return result; + std::string sql = "DROP TABLE " + table; + auto stmt = create_statement_from_sql(sql); + return execute_nonselect_statement(stmt) >= 0; } + /** * Perform a specified SQL operation on every table in a * database. Possible operations are: @@ -648,58 +638,52 @@ GncDbiSqlConnection::table_manage_backup (const std::string& table_name, bool GncDbiSqlConnection::table_operation(TableOpType op) noexcept { - static const std::regex backupre (".*_back"); - bool retval{true}; - for (auto table : m_provider->get_table_list(m_conn, "")) - { - dbi_result result; - /* Skip the lock table and existing backup tables; the former we don't - * want to touch, the latter are handled by table_manage_backup. It - * would be nicer to handle this with the get_table_list query, but that - * can accept only SQL LIKE patterns (not even regexps) and there's no - * way to have a negative one. - */ - if (table == lock_table || std::regex_match(table, backupre)) + auto backup_tables{m_provider->get_table_list(m_conn, "%_back")}; + auto all_tables{m_provider->get_table_list(m_conn, "")}; + /* No operations on the lock table */ + auto new_end = std::remove(all_tables.begin(), all_tables.end(), lock_table); + all_tables.erase(new_end, all_tables.end()); + StrVec data_tables; + data_tables.reserve(all_tables.size() - backup_tables.size()); + std::set_difference(all_tables.begin(), all_tables.end(), + backup_tables.begin(), backup_tables.end(), + std::back_inserter(data_tables)); + switch(op) + { + case backup: + if (!backup_tables.empty()) { - continue; + PERR("Unable to backup database, an existing backup is present."); + qof_backend_set_error(m_qbe, ERR_BACKEND_DATA_CORRUPT); + return false; } - do - { - init_error(); - switch (op) - { - case rollback: + for (auto table : data_tables) + if (!rename_table(table, table +"_back")) + return false; /* Error, trigger rollback. */ + break; + case drop_backup: + for (auto table : backup_tables) { - auto all_tables = m_provider->get_table_list(m_conn, ""); - if (std::find(all_tables.begin(), - all_tables.end(), table) != all_tables.end()) - { - result = dbi_conn_queryf (m_conn, "DROP TABLE %s", - table.c_str()); - if (result) - break; - } - } - /* Fall through to rename the _back tables back.*/ - case backup: - case drop_backup: - result = table_manage_backup (table, op); - break; - break; + auto data_table = table.substr(0, table.find("_back")); + if (std::find(data_tables.begin(), data_tables.end(), + data_table) != data_tables.end()) + drop_table(table); /* Other table exists, OK. */ + else /* No data table, restore the backup */ + rename_table(table, data_table); } - } - while (m_retry); - - if (result != nullptr) + break; + case rollback: + for (auto table : backup_tables) { - if (dbi_result_free (result) < 0) - { - PERR ("Error in dbi_result_free() result\n"); - retval = false; - } + auto data_table = table.substr(0, table.find("_back")); + if (std::find(data_tables.begin(), data_tables.end(), + data_table) != data_tables.end()) + drop_table(data_table); /* Other table exists, OK. */ + rename_table(table, data_table); } + break; } - return retval; + return true; } bool diff --git a/libgnucash/backend/dbi/gnc-dbisqlconnection.hpp b/libgnucash/backend/dbi/gnc-dbisqlconnection.hpp index a72e53b851..e2ae49dd10 100644 --- a/libgnucash/backend/dbi/gnc-dbisqlconnection.hpp +++ b/libgnucash/backend/dbi/gnc-dbisqlconnection.hpp @@ -80,7 +80,7 @@ public: */ bool verify() noexcept override; bool retry_connection(const char* msg) noexcept override; - dbi_result table_manage_backup(const std::string& table_name, TableOpType op); + bool table_operation (TableOpType op) noexcept; std::string add_columns_ddl(const std::string& table_name, const ColVec& info_vec) const noexcept; @@ -110,8 +110,9 @@ private: unsigned int m_sql_savepoint; bool lock_database(bool ignore_lock); void unlock_database(); + bool rename_table(const std::string& old_name, const std::string& new_name); + bool drop_table(const std::string& table); bool check_and_rollback_failed_save(); - }; #endif //_GNC_DBISQLCONNECTION_HPP_