diff --git a/src/EditTableDialog.cpp b/src/EditTableDialog.cpp index 63e64183..bab5878a 100644 --- a/src/EditTableDialog.cpp +++ b/src/EditTableDialog.cpp @@ -37,6 +37,10 @@ EditTableDialog::EditTableDialog(DBBrowserDB& db, const sqlb::ObjectIdentifier& m_table = *(pdb.getObjectByName(curTable)); ui->labelEditWarning->setVisible(!m_table.fullyParsed()); + // Initialise the list of tracked columns for table layout changes + for(const auto& field : m_table.fields) + trackColumns[field.name()] = field.name(); + // Set without rowid checkbox and schema dropdown. No need to trigger any events here as we're only loading a table exactly as it is stored by SQLite, so no need // for error checking etc. ui->checkWithoutRowid->blockSignals(true); @@ -57,9 +61,6 @@ EditTableDialog::EditTableDialog(DBBrowserDB& db, const sqlb::ObjectIdentifier& // And create a savepoint pdb.setSavepoint(m_sRestorePointName); - // Check now if foreign keys are enabled so we don't have to query this later again and again - m_bForeignKeysEnabled = (pdb.getPragma("foreign_keys") == "1"); - // Update UI ui->editTableName->setText(curTable.name()); updateColumnWidth(); @@ -170,14 +171,11 @@ void EditTableDialog::accept() } else { // Editing of old table - // Rename table if necessary - if(ui->editTableName->text() != curTable.name()) + // Apply all changes to the actual table in the database + if(!pdb.alterTable(curTable, m_table, trackColumns, ui->comboSchema->currentText())) { - if(!pdb.renameTable(ui->comboSchema->currentText(), curTable.name(), ui->editTableName->text())) - { - QMessageBox::warning(this, QApplication::applicationName(), pdb.lastError()); - return; - } + QMessageBox::warning(this, QApplication::applicationName(), pdb.lastError()); + return; } } @@ -214,13 +212,8 @@ void EditTableDialog::checkInput() const auto& fields = m_table.fields; for(const sqlb::Field& f : fields) { auto fk = std::dynamic_pointer_cast(m_table.constraint({f.name()}, sqlb::Constraint::ForeignKeyConstraintType)); - if(fk) { - if (oldTableName == fk->table()) { - fk->setTable(normTableName); - if(!m_bForeignKeysEnabled) - pdb.alterTable(curTable, m_table, f.name(), &f, 0); - } - } + if(fk && oldTableName == fk->table()) + fk->setTable(normTableName); } populateFields(); @@ -238,16 +231,15 @@ void EditTableDialog::updateTypes(QObject *object) QString type = typeBox->currentText(); QString column = typeBox->property("column").toString(); - size_t index; - for(index=0; index < m_table.fields.size(); ++index) + for(size_t index=0; index < m_table.fields.size(); ++index) { if(m_table.fields.at(index).name() == column) + { + m_table.fields.at(index).setType(type); break; + } } - m_table.fields.at(index).setType(type); - if(!m_bNewTable) - pdb.alterTable(curTable, m_table, column, &m_table.fields[index]); checkInput(); } } @@ -272,7 +264,6 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column) if(index < static_cast(m_table.fields.size())) { sqlb::Field& field = m_table.fields.at(index); - bool callRenameColumn = false; QString oldFieldName = field.name(); switch(column) @@ -328,8 +319,16 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column) field.setName(item->text(column)); m_table.renameKeyInAllConstraints(oldFieldName, item->text(column)); qobject_cast(ui->treeWidget->itemWidget(item, kType))->setProperty("column", item->text(column)); + + // Update the field name in the map of old column names to new column names if(!m_bNewTable) - callRenameColumn = true; + { + for(const auto& key : trackColumns) + { + if(trackColumns[key] == oldFieldName) + trackColumns[key] = field.name(); + } + } } break; case kType: // see updateTypes() SLOT @@ -362,8 +361,6 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column) } else { item->setCheckState(kAutoIncrement, Qt::Unchecked); } - if(!m_bNewTable) - callRenameColumn = true; } break; case kNotNull: @@ -395,8 +392,6 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column) } } field.setNotNull(item->checkState(column) == Qt::Checked); - if(!m_bNewTable) - callRenameColumn = true; } break; case kAutoIncrement: @@ -446,9 +441,6 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column) } } field.setAutoIncrement(ischecked); - - if(!m_bNewTable) - callRenameColumn = true; } break; case kUnique: @@ -484,9 +476,6 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column) } } field.setUnique(item->checkState(column) == Qt::Checked); - - if(!m_bNewTable) - callRenameColumn = true; } break; case kDefault: @@ -517,28 +506,15 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column) } } field.setDefaultValue(new_value); - if(!m_bNewTable) - callRenameColumn = true; } break; case kCheck: field.setCheck(item->text(column)); - if(!m_bNewTable) - callRenameColumn = true; break; case kForeignKey: // handled in delegate - - if(!m_bNewTable) - callRenameColumn = true; break; } - - if(callRenameColumn) - { - if(!pdb.alterTable(curTable, m_table, oldFieldName, &field)) - QMessageBox::warning(this, qApp->applicationName(), tr("Modifying this column failed. Error returned from database:\n%1").arg(pdb.lastError())); - } } checkInput(); @@ -590,9 +566,9 @@ void EditTableDialog::addField() // add field to table object m_table.fields.emplace_back(tbitem->text(kName), typeBox->currentText()); - // Actually add the new column to the table if we're editing an existing table + // Add the new column to the list of tracked columns to indicate it has been added if(!m_bNewTable) - pdb.addColumn(curTable, m_table.fields.back()); + trackColumns.insert(QString(), tbitem->text(kName)); checkInput(); } @@ -603,33 +579,27 @@ void EditTableDialog::removeField() if(!ui->treeWidget->currentItem()) return; - // Are we creating a new table or editing an old one? - if(m_bNewTable) + // If we are editing an existing table, ask the user for confirmation + if(!m_bNewTable) { - // Creating a new one - - // Just delete that item. At this point there is no DB table to edit or data to be lost anyway - m_table.fields.erase(m_table.fields.begin() + ui->treeWidget->indexOfTopLevelItem(ui->treeWidget->currentItem())); - m_table.removeKeyFromAllConstraints(ui->treeWidget->currentItem()->text(kName)); - delete ui->treeWidget->currentItem(); - } else { - // Editing an old one - - // Ask user whether he really wants to delete that column QString msg = tr("Are you sure you want to delete the field '%1'?\nAll data currently stored in this field will be lost.").arg(ui->treeWidget->currentItem()->text(0)); - if(QMessageBox::warning(this, QApplication::applicationName(), msg, QMessageBox::Yes | QMessageBox::No, QMessageBox::No) == QMessageBox::Yes) + if(QMessageBox::warning(this, QApplication::applicationName(), msg, QMessageBox::Yes | QMessageBox::No, QMessageBox::No) == QMessageBox::No) + return; + + // Update the map of tracked columns to indicate the column is deleted + QString name = ui->treeWidget->currentItem()->text(0); + for(const auto& key : trackColumns) { - if(!pdb.alterTable(curTable, m_table, ui->treeWidget->currentItem()->text(0), nullptr)) - { - QMessageBox::warning(nullptr, QApplication::applicationName(), pdb.lastError()); - } else { - //relayout - m_table = *(pdb.getObjectByName(curTable)); - populateFields(); - } + if(trackColumns[key] == name) + trackColumns[key] = QString(); } } + // Just delete that item. At this point there is no DB table to edit or data to be lost anyway + m_table.fields.erase(m_table.fields.begin() + ui->treeWidget->indexOfTopLevelItem(ui->treeWidget->currentItem())); + m_table.removeKeyFromAllConstraints(ui->treeWidget->currentItem()->text(kName)); + delete ui->treeWidget->currentItem(); + checkInput(); } @@ -665,54 +635,27 @@ void EditTableDialog::moveCurrentField(bool down) int currentRow = ui->treeWidget->currentIndex().row(); int newRow = currentRow + (down ? 1 : -1); - // Are we creating a new table or editing an old one? - if(m_bNewTable) - { - // Creating a new one + // Save the combobox first by making a copy + QComboBox* oldCombo = qobject_cast(ui->treeWidget->itemWidget(ui->treeWidget->topLevelItem(currentRow), kType)); + QComboBox* newCombo = new QComboBox(ui->treeWidget); + newCombo->setProperty("column", oldCombo->property("column")); + newCombo->installEventFilter(this); + connect(newCombo, SIGNAL(currentIndexChanged(int)), this, SLOT(updateTypes())); + newCombo->setEditable(true); + for(int i=0; i < oldCombo->count(); ++i) + newCombo->addItem(oldCombo->itemText(i)); + newCombo->setCurrentIndex(oldCombo->currentIndex()); - // Save the combobox first by making a copy - QComboBox* oldCombo = qobject_cast(ui->treeWidget->itemWidget(ui->treeWidget->topLevelItem(currentRow), kType)); - QComboBox* newCombo = new QComboBox(ui->treeWidget); - newCombo->setProperty("column", oldCombo->property("column")); - newCombo->installEventFilter(this); - connect(newCombo, SIGNAL(currentIndexChanged(int)), this, SLOT(updateTypes())); - newCombo->setEditable(true); - for(int i=0; i < oldCombo->count(); ++i) - newCombo->addItem(oldCombo->itemText(i)); - newCombo->setCurrentIndex(oldCombo->currentIndex()); + // Now, just remove the item and insert it at it's new position, then restore the combobox + QTreeWidgetItem* item = ui->treeWidget->takeTopLevelItem(currentRow); + ui->treeWidget->insertTopLevelItem(newRow, item); + ui->treeWidget->setItemWidget(item, kType, newCombo); - // Now, just remove the item and insert it at it's new position, then restore the combobox - QTreeWidgetItem* item = ui->treeWidget->takeTopLevelItem(currentRow); - ui->treeWidget->insertTopLevelItem(newRow, item); - ui->treeWidget->setItemWidget(item, kType, newCombo); + // Select the old item at its new position + ui->treeWidget->setCurrentIndex(ui->treeWidget->currentIndex().sibling(newRow, 0)); - // Select the old item at its new position - ui->treeWidget->setCurrentIndex(ui->treeWidget->currentIndex().sibling(newRow, 0)); - - // Finally update the table SQL - std::swap(m_table.fields[newRow], m_table.fields[currentRow]); - } else { - // Editing an old one - - // Move the actual column - if(!pdb.alterTable( - curTable, - m_table, - ui->treeWidget->currentItem()->text(0), - &m_table.fields[ui->treeWidget->indexOfTopLevelItem(ui->treeWidget->currentItem())], - (down ? 1 : -1) - )) - { - QMessageBox::warning(nullptr, QApplication::applicationName(), pdb.lastError()); - } else { - // Reload table SQL - m_table = *(pdb.getObjectByName(curTable)); - populateFields(); - - // Select old item at new position - ui->treeWidget->setCurrentIndex(ui->treeWidget->indexAt(QPoint(1, 1)).sibling(newRow, 0)); - } - } + // Finally update the table SQL + std::swap(m_table.fields[newRow], m_table.fields[currentRow]); // Update the SQL preview updateSqlText(); @@ -748,33 +691,10 @@ void EditTableDialog::setWithoutRowid(bool without_rowid) // Update the SQL preview updateSqlText(); - - // Update table if we're editing an existing table - if(!m_bNewTable) - { - if(!pdb.alterTable(curTable, m_table, QString(), nullptr, 0)) - { - QMessageBox::warning(this, QApplication::applicationName(), - tr("Setting the rowid column for the table failed. Error message:\n%1").arg(pdb.lastError())); - } - } } -void EditTableDialog::changeSchema(const QString& schema) +void EditTableDialog::changeSchema(const QString& /*schema*/) { // Update the SQL preview updateSqlText(); - - // Update table if we're editing an existing table - if(!m_bNewTable) - { - if(pdb.alterTable(curTable, m_table, QString(), nullptr, 0, schema)) - { - // Save the new schema name to use it from now on - curTable.setSchema(schema); - } else { - QMessageBox::warning(this, QApplication::applicationName(), tr("Changing the table schema failed. Error message:\n%1").arg(pdb.lastError())); - ui->comboSchema->setCurrentText(curTable.schema()); // Set it back to the original schema - } - } } diff --git a/src/EditTableDialog.h b/src/EditTableDialog.h index b19a67ef..aca71a7e 100644 --- a/src/EditTableDialog.h +++ b/src/EditTableDialog.h @@ -64,12 +64,10 @@ private: DBBrowserDB& pdb; ForeignKeyEditorDelegate* m_fkEditorDelegate; sqlb::ObjectIdentifier curTable; + QMap trackColumns; sqlb::Table m_table; - QStringList types; - QStringList fields; bool m_bNewTable; QString m_sRestorePointName; - bool m_bForeignKeysEnabled; }; #endif diff --git a/src/sql/sqlitetypes.cpp b/src/sql/sqlitetypes.cpp index 22d18bf1..78ecbfee 100644 --- a/src/sql/sqlitetypes.cpp +++ b/src/sql/sqlitetypes.cpp @@ -120,6 +120,18 @@ private: antlr::RefAST m_root; }; +bool Object::operator==(const Object& rhs) const +{ + if(m_name != rhs.m_name) + return false; + if(m_fullyParsed != rhs.m_fullyParsed) // We check for the fully parsed flag to make sure not to lose anything in some corner cases + return false; + + // We don't care about the original SQL text + + return true; +} + QString Object::typeToString(Types type) { switch(type) @@ -204,6 +216,28 @@ QString CheckConstraint::toSql(const QStringList&) const return result; } +bool Field::operator==(const Field& rhs) const +{ + if(m_name != rhs.m_name) + return false; + if(m_type != rhs.m_type) + return false; + if(m_notnull != rhs.m_notnull) + return false; + if(m_check != rhs.m_check) + return false; + if(m_defaultvalue != rhs.m_defaultvalue) + return false; + if(m_autoincrement != rhs.m_autoincrement) + return false; + if(m_unique != rhs.m_unique) + return false; + if(m_collation != rhs.m_collation) + return false; + + return true; +} + QString Field::toString(const QString& indent, const QString& sep) const { QString str = indent + escapeIdentifier(m_name) + sep + m_type; @@ -319,6 +353,23 @@ Table& Table::operator=(const Table& rhs) return *this; } +bool Table::operator==(const Table& rhs) const +{ + if(!Object::operator==(rhs)) + return false; + + if(m_rowidColumn != rhs.m_rowidColumn) + return false; + if(m_constraints != rhs.m_constraints) + return false; + if(m_virtual != rhs.m_virtual) + return false; + if(fields != rhs.fields) + return false; + + return true; +} + Table::field_iterator Table::findPk() { // TODO This is a stupid function (and always was) which should be fixed/improved diff --git a/src/sql/sqlitetypes.h b/src/sql/sqlitetypes.h index 567d57d0..2827b6d6 100644 --- a/src/sql/sqlitetypes.h +++ b/src/sql/sqlitetypes.h @@ -54,6 +54,7 @@ void setIdentifierQuoting(escapeQuoting toQuoting); QString escapeIdentifier(QString id); std::string escapeIdentifier(std::string id); +QStringList escapeIdentifier(const QStringList& ids); class ObjectIdentifier { @@ -187,6 +188,8 @@ public: explicit Object(const QString& name): m_name(name), m_fullyParsed(false) {} virtual ~Object() {} + virtual bool operator==(const Object& rhs) const; + virtual Types type() const = 0; static QString typeToString(Types type); @@ -353,10 +356,7 @@ public: , m_collation(collation) {} - bool operator==(const Field& rhs) const - { - return m_name == rhs.name(); - } + bool operator==(const Field& rhs) const; QString toString(const QString& indent = "\t", const QString& sep = "\t") const; @@ -405,6 +405,8 @@ public: ~Table() override; Table& operator=(const Table& rhs); + bool operator==(const Table& rhs) const; + Types type() const override { return Object::Table; } FieldVector fields; @@ -585,15 +587,22 @@ private: template typename T::field_iterator findField(T* object, const QString& name) { - return std::find_if(object->fields.begin(), object->fields.end(), [&name](const typename T::field_type& f) {return f.name().compare(name, Qt::CaseInsensitive) == 0;}); + return std::find_if(object->fields.begin(), object->fields.end(), [&name](const typename T::field_type& f) { + return f.name().compare(name, Qt::CaseInsensitive) == 0; + }); } template -typename T::field_iterator findField(std::shared_ptr object, const QString& name) +typename T::field_iterator findField(const T* object, const QString& name) +{ + return findField(const_cast(object), name); +} +template +typename std::remove_reference::type::field_iterator findField(std::shared_ptr object, const QString& name) { return findField(object.get(), name); } template -typename T::field_iterator findField(T& object, const QString& name) +typename std::remove_reference::type::field_iterator findField(T& object, const QString& name) { return findField(&object, name); } diff --git a/src/sqlitedb.cpp b/src/sqlitedb.cpp index 73677f9d..1587def8 100644 --- a/src/sqlitedb.cpp +++ b/src/sqlitedb.cpp @@ -1308,156 +1308,216 @@ bool DBBrowserDB::addColumn(const sqlb::ObjectIdentifier& tablename, const sqlb: return executeSQL(sql); } -bool DBBrowserDB::alterTable(const sqlb::ObjectIdentifier& tablename, const sqlb::Table& table, QString name, const sqlb::Field* to, int move, QString newSchemaName) +bool DBBrowserDB::alterTable(const sqlb::ObjectIdentifier& tablename, const sqlb::Table& new_table, AlterTableTrackColumns track_columns, QString newSchemaName) { - /* - * USE CASES: - * 1) Set table; unset name, to and move: Change table constraints only. - * 2) Set table and name; unset to and move: Change table constraints and remove column. - * 3) Set table, name and to; unset move: Change table constraints and rename/edit column. - * 4) Set table, name, to and move: Change table constraints, rename/edit column and move it. - */ + // This function is split into three different parts: + // Part 1 checks the arguments and prepares them for processing. It also prepares the transaction etc. + // Part 2 uses the built-in abilities of SQLite to change as much of the table schema as possible. + // Part 3 does the rest of the changes to the table schema. It also finishes the transaction etc. - // TODO: This function needs to be cleaned up. It might make sense to split it up in several parts than can be reused - // more easily. Besides that, it might make sense to support some potential use cases in a more sophisticated way. These include: - // 1) Allow modifying multiple columns at once in order to only have to call this function (including all its overhead) once instead of once per change. - // 2) Include the addColumn() use case in here, so the calling side doesn't need to know anything about how this class handles table modifications. + // + // P A R T 1 + // // If no new schema name has been set, we just use the old schema name if(newSchemaName.isNull()) { newSchemaName = tablename.schema(); + + // When renaming the table in the current schema, check if it doesn't exist already in there + if(tablename.name() != new_table.name() && getObjectByName(sqlb::ObjectIdentifier(newSchemaName, new_table.name())) != nullptr) + { + lastErrorMessage = tr("A table with the name '%1' already exists in schema '%2'.").arg(new_table.name()).arg(newSchemaName); + return false; + } } else { // We're moving the table to a different schema. So check first if it doesn't already exist in the new schema. - if(getObjectByName(sqlb::ObjectIdentifier(newSchemaName, tablename.name())) != nullptr && newSchemaName != tablename.schema()) + if(newSchemaName != tablename.schema() && getObjectByName(sqlb::ObjectIdentifier(newSchemaName, new_table.name())) != nullptr) { - lastErrorMessage = tr("A table with the name '%1' already exists in schema '%2'.").arg(tablename.name()).arg(newSchemaName); + lastErrorMessage = tr("A table with the name '%1' already exists in schema '%2'.").arg(new_table.name()).arg(newSchemaName); return false; } } - // Create table schema - sqlb::TablePtr oldSchema = getObjectByName(tablename); - - // Check if field actually exists - if(!name.isNull() && sqlb::findField(oldSchema, name) == oldSchema->fields.end()) + // Get old table schema + sqlb::TablePtr old_table_ptr = getObjectByName(tablename); + if(old_table_ptr == nullptr) { - lastErrorMessage = tr("renameColumn: cannot find column %1.").arg(name); + lastErrorMessage = tr("No table with name '%1' exists in schema '%2'.").arg(tablename.name()).arg(tablename.schema()); return false; } + sqlb::Table old_table = *old_table_ptr; + + // Check if tracked fields actually exist in the old table + for(const auto& old_name : track_columns.keys()) + { + if(!old_name.isNull() && sqlb::findField(old_table, old_name) == old_table.fields.end()) + { + lastErrorMessage = tr("Cannot find column %1.").arg(old_name); + return false; + } + } + + // Check if there are any columns in the old table which are not mentioned in the tracked columns list. + // We do this before checking if all tracked fields are in the new table to make sure the following check includes them. + for(const auto& field : old_table.fields) + { + if(!track_columns.keys().contains(field.name())) + { + // If a field isn't tracked, add it to the list and indicate explicitly that it has the same name in the new table + track_columns[field.name()] = field.name(); + } + } + + // Check if tracked fields actually exist in the new table + for(const auto& new_name : track_columns.values()) + { + if(!new_name.isNull() && sqlb::findField(new_table, new_name) == new_table.fields.end()) + { + lastErrorMessage = tr("Cannot find column %1.").arg(new_name); + return false; + } + } // Create savepoint to be able to go back to it in case of any error QString savepointName = generateSavepointName("renamecolumn"); if(!setSavepoint(savepointName)) { - lastErrorMessage = tr("renameColumn: creating savepoint failed. DB says: %1").arg(lastErrorMessage); + lastErrorMessage = tr("Creating savepoint failed. DB says: %1").arg(lastErrorMessage); return false; } // No automatic schema updates from now on NoStructureUpdateChecks nup(*this); - // Newer versions of SQLite add a better ALTER TABLE support which we can use -#if SQLITE_VERSION_NUMBER >= 3025000 - // If the name of the field should be changed do that by using SQLite's ALTER TABLE feature - if(!name.isNull() && to && name != to->name()) + // + // P A R T 2 + // + + // This variable is used to track whether something was changed by this part of the function + bool changed_something = false; + + // Rename table if necessary + if(newSchemaName == tablename.schema() && tablename.name() != new_table.name()) { - if(!executeSQL(QString("ALTER TABLE %1 RENAME COLUMN %2 TO %3;") - .arg(tablename.toString()) - .arg(sqlb::escapeIdentifier(name)) - .arg(sqlb::escapeIdentifier(to->name())))) + if(!renameTable(tablename.schema(), old_table.name(), new_table.name())) { - QString error(tr("renameColumn: renaming the column failed. DB says:\n%1").arg(lastErrorMessage)); revertToSavepoint(savepointName); - lastErrorMessage = error; return false; } - // Update our schema representation to get all the changed triggers, views and indices - updateSchema(); + changed_something = true; + } - // Check if that was all we were asked to do. That's the case if the field is not to be deleted (which we already checked for above), if the field - // is not to be moved, if the table is not to be moved and if nothing besides the name of the field changed in the field definition. - sqlb::Field oldFieldWithNewName = *sqlb::findField(oldSchema, name); - oldFieldWithNewName.setName(to->name()); - if(move == 0 && tablename.schema() == newSchemaName && oldFieldWithNewName == *to) + // Add columns if necessary + const auto new_fields = track_columns.values(QString()); + for(const auto& field : new_table.fields) + { + // We loop through all the fields of the new table schema and check for each of them if they are new. + // If so, we add that field. The reason for looping through the new table schema instead of the track_columns + // map is that this way we make sure to preserve their order which increases our chances that we are done after + // this step. + if(new_fields.contains(field.name())) { - // We're done. - - // Release the savepoint - everything went fine - if(!releaseSavepoint(savepointName)) + if(!addColumn(sqlb::ObjectIdentifier(tablename.schema(), new_table.name()), field)) { - lastErrorMessage = tr("renameColumn: releasing savepoint failed. DB says: %1").arg(lastErrorMessage); + revertToSavepoint(savepointName); + return false; + } + } + + changed_something = true; + } + + // Newer versions of SQLite add a better ALTER TABLE support which we can use +#if SQLITE_VERSION_NUMBER >= 3025000 + // If the name of a field should be changed do that by using SQLite's ALTER TABLE feature + for(const auto& old_name : track_columns.keys()) + { + QString new_name = track_columns[old_name]; + if(!old_name.isNull() && !new_name.isNull() && new_name != old_name) + { + if(!executeSQL(QString("ALTER TABLE %1 RENAME COLUMN %2 TO %3;") + .arg(sqlb::ObjectIdentifier(tablename.schema(), new_table.name()).toString()) + .arg(sqlb::escapeIdentifier(old_name)) + .arg(sqlb::escapeIdentifier(new_name)))) + { + QString error(tr("Renaming the column failed. DB says:\n%1").arg(lastErrorMessage)); + revertToSavepoint(savepointName); + lastErrorMessage = error; return false; } - return true; - } else { - // There's more to do. - - // We can have the rest of the function deal with the remaining changes by reloading the table schema as it is now and updating the name of the column - // to change. - oldSchema = getObjectByName(tablename); - name = to->name(); + changed_something = true; } } #endif - // Create a new table with a name that hopefully doesn't exist yet. - // Its layout is exactly the same as the one of the table to change - except for the column to change - // of course, and the table constraints which are copied from the table parameter. - sqlb::Table newSchema = *oldSchema; - QString temp_table_name = generateTemporaryTableName(newSchemaName); - newSchema.setName(temp_table_name); - newSchema.setConstraints(table.allConstraints()); - newSchema.setRowidColumn(table.rowidColumn()); - QString select_cols; - if(!to) + // Update our schema representation to get the new table and all the changed triggers, views and indices + if(changed_something) { - // We want drop the column - so just remove the field. If the name is set to null, skip this step. This effectively leaves all fields as they are, - // thus only changing the table constraints. - if(!name.isNull()) - sqlb::removeField(newSchema, name); - - for(size_t i=0;iname(), to->name()); - *(index + move) = *to; + updateSchema(); + old_table = *getObjectByName(sqlb::ObjectIdentifier(tablename.schema(), new_table.name())); } - // Create the new table - if(!executeSQL(newSchema.sql(newSchemaName), true, true)) + // Check if there's still more work to be done or if we are finished now + if(tablename.schema() == newSchemaName && old_table == new_table) { - QString error(tr("renameColumn: creating new table failed. DB says: %1").arg(lastErrorMessage)); + // Release the savepoint - everything went fine + if(!releaseSavepoint(savepointName)) + { + lastErrorMessage = tr("Releasing savepoint failed. DB says: %1").arg(lastErrorMessage); + return false; + } + + // Success, update the DB schema before returning + updateSchema(); + return true; + } + + // + // P A R T 3 + // + + // Create a new table with the desired schema and a name that doesn't exist yet + QString new_table_name = new_table.name(); + sqlb::Table new_table_with_random_name = new_table; + new_table_with_random_name.setName(generateTemporaryTableName(newSchemaName)); + if(!executeSQL(new_table_with_random_name.sql(newSchemaName), true, true)) + { + QString error(tr("Creating new table failed. DB says: %1").arg(lastErrorMessage)); revertToSavepoint(savepointName); lastErrorMessage = error; return false; } + // Assemble list of column names to copy from in the old table and list of column names to into into in the new table + QStringList copy_values_from; + QStringList copy_values_to; + for(const auto& from : track_columns.keys()) + { + // Ignore new fields + if(from.isNull()) + continue; + + // Ignore deleted fields + QString to = track_columns[from]; + if(to.isNull()) + continue; + + copy_values_from.push_back(from); + copy_values_to.push_back(to); + } + // Copy the data from the old table to the new one - if(!executeSQL(QString("INSERT INTO %1.%2 SELECT %3 FROM %4;") + if(!executeSQL(QString("INSERT INTO %1.%2 (%3) SELECT %4 FROM %5;") .arg(sqlb::escapeIdentifier(newSchemaName)) - .arg(sqlb::escapeIdentifier(temp_table_name)) - .arg(select_cols) + .arg(sqlb::escapeIdentifier(new_table_with_random_name.name())) + .arg(sqlb::escapeIdentifier(copy_values_to).join(",")) + .arg(sqlb::escapeIdentifier(copy_values_from).join(",")) .arg(tablename.toString()))) { - QString error(tr("renameColumn: copying data to new table failed. DB says:\n%1").arg(lastErrorMessage)); + QString error(tr("Copying data to new table failed. DB says:\n%1").arg(lastErrorMessage)); revertToSavepoint(savepointName); lastErrorMessage = error; return false; @@ -1476,19 +1536,26 @@ bool DBBrowserDB::alterTable(const sqlb::ObjectIdentifier& tablename, const sqlb { sqlb::IndexPtr idx = std::dynamic_pointer_cast(it); - // Are we updating a field name or are we removing a field entirely? - if(to) + // Loop through all changes to the table schema. For indices only the column names are relevant, so it suffices to look at the + // list of tracked columns + for(const auto& from : track_columns) { - // We're updating a field name. So search for it in the index and replace it whereever it is found - for(size_t i=0;ifields.size();i++) + QString to = track_columns[from]; + + // Are we updating the field name or are we removing the field entirely? + if(!to.isNull()) { - if(idx->fields[i].name() == name) - idx->fields[i].setName(to->name()); + // We're updating the field name. So search for it in the index and replace it whereever it is found + for(size_t i=0;ifields.size();i++) + { + if(idx->fields[i].name() == from) + idx->fields[i].setName(to); + } + } else { + // We're removing a field. So remove it from any indices, too. + while(sqlb::removeField(idx, from)) + ; } - } else { - // We're removing a field. So remove it from any indices, too. - while(sqlb::removeField(idx, name)) - ; } // Only try to add the index later if it has any columns remaining. Also use the new schema name here, too, to basically move @@ -1514,14 +1581,14 @@ bool DBBrowserDB::alterTable(const sqlb::ObjectIdentifier& tablename, const sqlb // Delete the old table if(!executeSQL(QString("DROP TABLE %1;").arg(tablename.toString()), true, true)) { - QString error(tr("renameColumn: deleting old table failed. DB says: %1").arg(lastErrorMessage)); + QString error(tr("Deleting old table failed. DB says: %1").arg(lastErrorMessage)); revertToSavepoint(savepointName); lastErrorMessage = error; return false; } // Rename the temporary table - if(!renameTable(newSchemaName, temp_table_name, tablename.name())) + if(!renameTable(newSchemaName, new_table_with_random_name.name(), tablename.name())) { revertToSavepoint(savepointName); return false; @@ -1548,7 +1615,7 @@ bool DBBrowserDB::alterTable(const sqlb::ObjectIdentifier& tablename, const sqlb // Release the savepoint - everything went fine if(!releaseSavepoint(savepointName)) { - lastErrorMessage = tr("renameColumn: releasing savepoint failed. DB says: %1").arg(lastErrorMessage); + lastErrorMessage = tr("Releasing savepoint failed. DB says: %1").arg(lastErrorMessage); return false; } diff --git a/src/sqlitedb.h b/src/sqlitedb.h index 46243463..0e744fb4 100644 --- a/src/sqlitedb.h +++ b/src/sqlitedb.h @@ -3,13 +3,13 @@ #include "sql/sqlitetypes.h" +#include #include #include -#include -#include -#include #include +#include +#include struct sqlite3; class CipherSettings; @@ -155,17 +155,25 @@ public: bool addColumn(const sqlb::ObjectIdentifier& tablename, const sqlb::Field& field); /** - * @brief alterTable Can be used to rename, modify or drop an existing column of a given table - * @param schema Specifies the name of the schema, i.e. the database name, of the table - * @param tablename Specifies the name of the table to edit - * @param table Specifies the table to edit. The table constraints are used from this but not the columns - * @param name Name of the column to edit - * @param to The new field definition with changed name, type or the like. If Null-Pointer is given the column is dropped. - * @param move Set this to a value != 0 to move the new column to a different position + * @brief This type maps from old column names to new column names. Given the old and the new table definition, this suffices to + * track fields between the two. + * USE CASES: + * 1) Don't specify a column at all or specify equal column names: Keep its name as-is. + * 2) Specify different column names: Rename the field. + * 3) Map from an existing column name to a Null string: Delete the column. + * 4) Map from a Null column name to a new column name: Add the column. + */ + using AlterTableTrackColumns = QMap; + + /** + * @brief alterTable Can be used to rename, modify or drop existing columns of a given table + * @param tablename Specifies the schema and name of the table to edit + * @param new_table Specifies the new table schema. This is exactly how the new table is going to look like. + * @param track_columns Maps old column names to new column names. This is used to copy the data from the old table to the new one. * @param newSchema Set this to a non-empty string to move the table to a new schema * @return true if renaming was successful, false if not. In the latter case also lastErrorMessage is set */ - bool alterTable(const sqlb::ObjectIdentifier& tablename, const sqlb::Table& table, QString name, const sqlb::Field* to, int move = 0, QString newSchemaName = QString()); + bool alterTable(const sqlb::ObjectIdentifier& tablename, const sqlb::Table& new_table, AlterTableTrackColumns track_columns, QString newSchemaName = QString()); objectMap getBrowsableObjects(const QString& schema) const;