[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.

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

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_