Problems with WITHOUT ROWID tables with PK of string type (sqlitebrow…

…ser#1559) * Problems with WITHOUT ROWID tables with PK of string type This fixes two related problems: - When the PK is updated the hidden column 0 must be updated too, otherwise any further editing of the same row before a table refresh is broken. - When a new record is inserted and the PK has string type we cannot simply make a pseudo auto-increment and insert that value as rowid because that added key would be impossible to update because our UPDATE clause will try to update a column with a string and it contains an integer. In this case it's better to let the user enter the PK value, so the new Add Record dialog is directly invoked. See issue sqlitebrowser#1332 and (tangentially) sqlitebrowser#1049. The first should be fixed now. The later not, but at least there is now a workaround: removing the AUTOINCREMENT option and use the WITHOUT ROWID one. * Problems with WITHOUT ROWID tables with PK of string type (alternative 2) Update after review: - cached_row is not modified after unlock(); - Alternative solution to initial key value insertion: When a new record is inserted and the PK has string type we still make a pseudo auto-increment and insert that value as a string literal. In this way we can later update that row. When we inserted it as integer an actual integer will be inserted by SQLite, and our UPDATE clause, which always uses string in the WHERE condition, won't match the row (SQLite does not convert to integer when the column is of type string in this context). See issue sqlitebrowser#1332.
abdihaikal · Oct 9, 2018 · 66ffa5f · 66ffa5f
1 parent e930b20
commit 66ffa5f
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 3 deletions.
diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp
@@ -775,6 +775,7 @@ void MainWindow::closeEvent( QCloseEvent* event )
 void MainWindow::addRecord()
 {
     int row = m_browseTableModel->rowCount();
+
     if(m_browseTableModel->insertRow(row))
     {
         selectTableLine(row);

diff --git a/src/sqlitedb.cpp b/src/sqlitedb.cpp
@@ -1099,12 +1099,13 @@ QString DBBrowserDB::emptyInsertStmt(const QString& schemaName, const sqlb::Tabl
 
             if(!pk_value.isNull())
             {
-                vals << pk_value;
+                vals << (f.isText()? "'" + pk_value + "'" : pk_value);
             } else {
                 if(f.notnull())
                 {
                     QString maxval = this->max(sqlb::ObjectIdentifier(schemaName, t.name()), f);
-                    vals << QString::number(maxval.toLongLong() + 1);
+                    QString newval = QString::number(maxval.toLongLong() + 1);
+                    vals << (f.isText()? "'" + newval + "'" : newval);
                 } else {
                     vals << "NULL";
                 }

diff --git a/src/sqlitetablemodel.cpp b/src/sqlitetablemodel.cpp
@@ -392,7 +392,14 @@ bool SqliteTableModel::setTypedData(const QModelIndex& index, bool isBlob, const
         if(m_db.updateRecord(m_sTable, m_headers.at(index.column()), cached_row.at(0), newValue, isBlob, m_pseudoPk))
         {
             cached_row.replace(index.column(), newValue);
-            lock.unlock();
+            if(m_headers.at(index.column()) == m_sRowidColumn) {
+                cached_row.replace(0, newValue);
+                const QModelIndex& rowidIndex = index.sibling(index.row(), 0);
+                lock.unlock();
+                emit dataChanged(rowidIndex, rowidIndex);
+            } else {
+                lock.unlock();
+            }
             emit dataChanged(index, index);
             return true;
         } else {