mirror/sqlitebrowser
1
0
Fork 0
mirror of https://github.com/sqlitebrowser/sqlitebrowser.git synced 2026-01-28 15:04:36 -06:00
Code Issues Packages Projects Releases Wiki Activity

Fix alterTable() function

Browse Source 
Fix a few issues in the alterTable() code. One type of issue would
happen if there are any keys or constraints in the table. Because the
check whether more changes are needed did not work as expected, we would
try to edit the table again, even though it is already correct. The
second type of issue (which can be triggered independently but which can
also be a follow-up issue to the first one) tries to access the table by
its old name even though it might already have been renamed.

See issue #1725.
This commit is contained in:
Martin Kleusberg
2019-02-15 17:05:53 +01:00
parent a840a43161
commit 14c82ea817
2 changed files with 31 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
src/sql/sqlitetypes.cpp
View File

@@ -360,13 +360,36 @@ bool Table::operator==(const Table& rhs) const
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;
// We need to compare the constraint maps manually here. The reason is that the values are pointers and the default implementation
// would compare the pointers not the actual objects.
if(m_constraints.size() != rhs.m_constraints.size())
return false;
for(auto it=m_constraints.cbegin();it!=m_constraints.end();++it)
{
// For each element in this map we get the list of all elements with the same key from the other map.
// Then we loop through that list and check if we find an element of the same type which produces the same SQL substring. We use this
// approach to avoid casting both objects to their actual type, then dereferencing it etc.
auto range = rhs.m_constraints.equal_range(it->first);
bool found_something = false;
for(auto jt=range.first;jt!=range.second;++jt)
{
if(it->second->type() == jt->second->type() && it->second->toSql(it->first) == jt->second->toSql(jt->first))
{
found_something = true;
break;
}
}
// If no match was found, the constraint maps aren't equal
if(!found_something)
return false;
}
return true;
}

11
src/sqlitedb.cpp
View File

@@ -1592,12 +1592,13 @@ bool DBBrowserDB::alterTable(const sqlb::ObjectIdentifier& tablename, const sqlb
}
// Copy the data from the old table to the new one
if(!executeSQL(QString("INSERT INTO %1.%2 (%3) SELECT %4 FROM %5;")
if(!executeSQL(QString("INSERT INTO %1.%2 (%3) SELECT %4 FROM %5.%6;")
.arg(sqlb::escapeIdentifier(newSchemaName))
.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())))
.arg(sqlb::escapeIdentifier(tablename.schema()))
.arg(sqlb::escapeIdentifier(old_table.name()))))
{
QString error(tr("Copying data to new table failed. DB says:\n%1").arg(lastErrorMessage));
revertToSavepoint(savepointName);
@@ -1610,7 +1611,7 @@ bool DBBrowserDB::alterTable(const sqlb::ObjectIdentifier& tablename, const sqlb
for(auto it : schemata[tablename.schema()])
{
// If this object references the table and it's not the table itself save it's SQL string
if(it->baseTable() == tablename.name() && it->type() != sqlb::Object::Types::Table)
if(it->baseTable() == old_table.name() && it->type() != sqlb::Object::Types::Table)
{
// If this is an index, update the fields first. This highly increases the chance that the SQL statement won't throw an
// error later on when we try to recreate it.
@@ -1661,7 +1662,7 @@ bool DBBrowserDB::alterTable(const sqlb::ObjectIdentifier& tablename, const sqlb
setPragma("defer_foreign_keys", "1");
// Delete the old table
if(!executeSQL(QString("DROP TABLE %1;").arg(tablename.toString()), true, true))
if(!executeSQL(QString("DROP TABLE %1.%2;").arg(sqlb::escapeIdentifier(tablename.schema())).arg(sqlb::escapeIdentifier(old_table.name())), true, true))
{
QString error(tr("Deleting old table failed. DB says: %1").arg(lastErrorMessage));
revertToSavepoint(savepointName);
@@ -1670,7 +1671,7 @@ bool DBBrowserDB::alterTable(const sqlb::ObjectIdentifier& tablename, const sqlb
}
// Rename the temporary table
if(!renameTable(newSchemaName, new_table_with_random_name.name(), tablename.name()))
if(!renameTable(newSchemaName, new_table_with_random_name.name(), new_table.name()))
{
revertToSavepoint(savepointName);
return false;