Fixed bug issue sqlitebrowser#390 ("crash then correct table").

Some functions renamed for better readability. See pull request sqlitebrowser#401.
jgwin · Oct 14, 2015 · 9d7efca · 9d7efca
1 parent 8b4eeb3
commit 9d7efca
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 30 deletions.
diff --git a/src/EditTableDialog.cpp b/src/EditTableDialog.cpp
@@ -34,7 +34,7 @@ EditTableDialog::EditTableDialog(DBBrowserDB* db, const QString& tableName, bool
     }
 
     // And create a savepoint
-    pdb->setRestorePoint(m_sRestorePointName);
+    pdb->setSavepoint(m_sRestorePointName);
 
     // Update UI
     ui->editTableName->setText(curTable);
@@ -158,7 +158,7 @@ void EditTableDialog::accept()
 void EditTableDialog::reject()
 {    
     // Then rollback to our savepoint
-    pdb->revert(m_sRestorePointName);
+    pdb->revertToSavepoint(m_sRestorePointName);
     pdb->updateSchema();
 
     QDialog::reject();

diff --git a/src/ImportCsvDialog.cpp b/src/ImportCsvDialog.cpp
@@ -75,7 +75,7 @@ void rollback(
         QString error = sCSVInfo + QObject::tr(".\n%1").arg(message);
         QMessageBox::warning(dialog, QApplication::applicationName(), error);
     }
-    pdb->revert(savepointName);
+    pdb->revertToSavepoint(savepointName);
 }
 }
 
@@ -208,7 +208,7 @@ void ImportCsvDialog::accept()
     // Create a savepoint, so we can rollback in case of any errors during importing
     // db needs to be saved or an error will occur
     QString restorepointName = QString("CSVIMPORT_%1").arg(QDateTime::currentMSecsSinceEpoch());
-    if(!pdb->setRestorePoint(restorepointName))
+    if(!pdb->setSavepoint(restorepointName))
         return rollback(this, pdb, progress, restorepointName, 0, tr("Creating restore point failed: %1").arg(pdb->lastErrorMessage));
 
     // Create table

diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp
@@ -802,7 +802,7 @@ void MainWindow::executeQuery()
     // we create the prepared statement, otherwise every executed
     // statement will get committed after the prepared statement
     // gets finalized, see http://www.sqlite.org/lang_transaction.html
-    db.setRestorePoint();
+    db.setSavepoint();
 
     // Remove any error indicators
     sqlWidget->getEditor()->clearErrorIndicators();
@@ -891,7 +891,7 @@ void MainWindow::executeQuery()
     updatePlot(sqlWidget->getModel());
 
     if(!modified && !wasdirty)
-        db.revert(); // better rollback, if the logic is not enough we can tune it.
+        db.revertToSavepoint(); // better rollback, if the logic is not enough we can tune it.
 }
 
 void MainWindow::mainTabSelected(int tabindex)
@@ -951,7 +951,7 @@ void MainWindow::dbState( bool dirty )
 void MainWindow::fileSave()
 {
     if(db.isOpen())
-        db.saveAll();
+        db.releaseAllSavepoints();
 }
 
 void MainWindow::fileRevert()
@@ -2146,7 +2146,7 @@ void MainWindow::editEncryption()
         qApp->processEvents();
 
         // Apply all unsaved changes
-        bool ok = db.saveAll();
+        bool ok = db.releaseAllSavepoints();
         qApp->processEvents();
 
         // Create the new file first or it won't work

diff --git a/src/VacuumDialog.cpp b/src/VacuumDialog.cpp
@@ -44,7 +44,7 @@ void VacuumDialog::accept()
     QApplication::setOverrideCursor(Qt::WaitCursor);
 
     // Commit all changes first
-    db->saveAll();
+    db->releaseAllSavepoints();
 
     // All items selected?
     if(ui->treeSelectedObjects->selectedItems().count() ==  ui->treeSelectedObjects->topLevelItemCount())

diff --git a/src/sqlitedb.cpp b/src/sqlitedb.cpp
@@ -255,7 +255,7 @@ bool DBBrowserDB::tryEncryptionSettings(const QString& filename, bool* encrypted
     }
 }
 
-bool DBBrowserDB::setRestorePoint(const QString& pointname)
+bool DBBrowserDB::setSavepoint(const QString& pointname)
 {
     if(!isOpen())
         return false;
@@ -270,7 +270,7 @@ bool DBBrowserDB::setRestorePoint(const QString& pointname)
     return true;
 }
 
-bool DBBrowserDB::save(const QString& pointname)
+bool DBBrowserDB::releaseSavepoint(const QString& pointname)
 {
     if(!isOpen() || savepointList.contains(pointname) == false)
         return false;
@@ -283,7 +283,7 @@ bool DBBrowserDB::save(const QString& pointname)
     return true;
 }
 
-bool DBBrowserDB::revert(const QString& pointname)
+bool DBBrowserDB::revertToSavepoint(const QString& pointname)
 {
     if(!isOpen() || savepointList.contains(pointname) == false)
         return false;
@@ -298,21 +298,22 @@ bool DBBrowserDB::revert(const QString& pointname)
     return true;
 }
 
-bool DBBrowserDB::saveAll()
+bool DBBrowserDB::releaseAllSavepoints()
 {
     foreach(const QString& point, savepointList)
     {
-        if(!save(point))
+        if(!releaseSavepoint(point))
             return false;
     }
+    executeSQL("COMMIT;");  // Just to be sure
     return true;
 }
 
 bool DBBrowserDB::revertAll()
 {
     foreach(const QString& point, savepointList)
     {
-        if(!revert(point))
+        if(!revertToSavepoint(point))
             return false;
     }
     return true;
@@ -387,7 +388,7 @@ bool DBBrowserDB::close()
 
             // If he didn't it was either yes or no
             if(reply == QMessageBox::Yes)
-                saveAll();
+                releaseAllSavepoints();
             else
                 revertAll(); //not really necessary, I think... but will not hurt.
         }
@@ -576,7 +577,7 @@ bool DBBrowserDB::executeSQL ( const QString & statement, bool dirtyDB, bool log
         return false;
 
     if (logsql) logSQL(statement, kLogMsg_App);
-    if (dirtyDB) setRestorePoint();
+    if (dirtyDB) setSavepoint();
 
     if (SQLITE_OK == sqlite3_exec(_db, statement.toUtf8(), NULL, NULL, &errmsg))
     {
@@ -600,7 +601,7 @@ bool DBBrowserDB::executeMultiSQL(const QString& statement, bool dirty, bool log
 
     // Set DB to dirty/create restore point if necessary
     if(dirty)
-        setRestorePoint();
+        setSavepoint();
 
     // Show progress dialog
     int statement_size = statement.size();
@@ -833,7 +834,7 @@ bool DBBrowserDB::updateRecord(const QString& table, const QString& column, cons
             .arg(rowid);
 
     logSQL(sql, kLogMsg_App);
-    setRestorePoint();
+    setSavepoint();
 
     // If we get a NULL QByteArray we insert a NULL value, and for that
     // we can pass NULL to sqlite3_bind_text() so that it behaves like sqlite3_bind_null()
@@ -926,7 +927,7 @@ bool DBBrowserDB::renameColumn(const QString& tablename, const QString& name, sq
     }
 
     // Create savepoint to be able to go back to it in case of any error
-    if(!executeSQL("SAVEPOINT sqlitebrowser_rename_column", false))
+    if(!setSavepoint("sqlitebrowser_rename_column"))
     {
         lastErrorMessage = tr("renameColumn: creating savepoint failed. DB says: %1").arg(lastErrorMessage);
         qWarning() << lastErrorMessage;
@@ -970,7 +971,7 @@ bool DBBrowserDB::renameColumn(const QString& tablename, const QString& name, sq
     {
         QString error(tr("renameColumn: creating new table failed. DB says: %1").arg(lastErrorMessage));
         qWarning() << error;
-        executeSQL("ROLLBACK TO SAVEPOINT sqlitebrowser_rename_column;");
+        revertToSavepoint("sqlitebrowser_rename_column");
         lastErrorMessage = error;
         return false;
     }
@@ -980,7 +981,7 @@ bool DBBrowserDB::renameColumn(const QString& tablename, const QString& name, sq
     {
         QString error(tr("renameColumn: copying data to new table failed. DB says:\n%1").arg(lastErrorMessage));
         qWarning() << error;
-        executeSQL("ROLLBACK TO SAVEPOINT sqlitebrowser_rename_column;");
+        revertToSavepoint("sqlitebrowser_rename_column");
         lastErrorMessage = error;
         return false;
     }
@@ -1003,15 +1004,15 @@ bool DBBrowserDB::renameColumn(const QString& tablename, const QString& name, sq
     {
         QString error(tr("renameColumn: deleting old table failed. DB says: %1").arg(lastErrorMessage));
         qWarning() << error;
-        executeSQL("ROLLBACK TO SAVEPOINT sqlitebrowser_rename_column;");
+        revertToSavepoint("sqlitebrowser_rename_column");
         lastErrorMessage = error;
         return false;
     }
 
     // Rename the temporary table
     if(!renameTable("sqlitebrowser_rename_column_new_table", tablename))
     {
-        executeSQL("ROLLBACK TO SAVEPOINT sqlitebrowser_rename_column;");
+        revertToSavepoint("sqlitebrowser_rename_column");
         return false;
     }
 
@@ -1028,7 +1029,7 @@ bool DBBrowserDB::renameColumn(const QString& tablename, const QString& name, sq
     }
 
     // Release the savepoint - everything went fine
-    if(!executeSQL("RELEASE SAVEPOINT sqlitebrowser_rename_column;", false))
+    if(!releaseSavepoint("sqlitebrowser_rename_column"))
     {
         lastErrorMessage = tr("renameColumn: releasing savepoint failed. DB says: %1").arg(lastErrorMessage);
         qWarning() << lastErrorMessage;
@@ -1205,7 +1206,7 @@ bool DBBrowserDB::setPragma(const QString& pragma, const QString& value)
     // Set the pragma value
     QString sql = QString("PRAGMA %1 = \"%2\";").arg(pragma).arg(value);
 
-    save();
+    releaseSavepoint();
     bool res = executeSQL(sql, false, true); // PRAGMA statements are usually not transaction bound, so we can't revert
     if( !res )
         qWarning() << tr("Error setting pragma %1 to %2: %3").arg(pragma).arg(value).arg(lastErrorMessage);

diff --git a/src/sqlitedb.h b/src/sqlitedb.h
@@ -49,10 +49,10 @@ class DBBrowserDB : public QObject
     bool attach(const QString& filename, QString attach_as = "");
     bool create ( const QString & db);
     bool close();
-    bool setRestorePoint(const QString& pointname = "RESTOREPOINT");
-    bool save (const QString& pointname = "RESTOREPOINT");
-    bool revert (const QString& pointname = "RESTOREPOINT");
-    bool saveAll();
+    bool setSavepoint(const QString& pointname = "RESTOREPOINT");
+    bool releaseSavepoint(const QString& pointname = "RESTOREPOINT");
+    bool revertToSavepoint(const QString& pointname = "RESTOREPOINT");
+    bool releaseAllSavepoints();
     bool revertAll();
     bool dump(const QString & filename, const QStringList &tablesToDump, bool insertColNames, bool insertNew, bool exportSchemaOnly);
     bool executeSQL ( const QString & statement, bool dirtyDB=true, bool logsql=true);