Skip to content

Commit

Permalink
Problems with WITHOUT ROWID tables with PK of string type (sqlitebrow…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
mgrojo authored and MKleusberg committed Oct 9, 2018
1 parent e930b20 commit 66ffa5f
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 3 deletions.
1 change: 1 addition & 0 deletions src/MainWindow.cpp
Expand Up @@ -775,6 +775,7 @@ void MainWindow::closeEvent( QCloseEvent* event )
void MainWindow::addRecord()
{
int row = m_browseTableModel->rowCount();

if(m_browseTableModel->insertRow(row))
{
selectTableLine(row);
Expand Down
5 changes: 3 additions & 2 deletions src/sqlitedb.cpp
Expand Up @@ -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";
}
Expand Down
9 changes: 8 additions & 1 deletion src/sqlitetablemodel.cpp
Expand Up @@ -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 {
Expand Down

0 comments on commit 66ffa5f

Please sign in to comment.