Sqlite: Fix insertUpdateDelete

Like you can read in https://www.sqlite.org/isolation.html after an
update they same value can be show up for an iterator advancement. This
would be lead to an delete. So the last value for update is saved and
then compared in the delete method. If they are equal the delete is
skipped.

Change-Id: Ic0aa6619f6a4a520eac77be4e5a83cbe533d102d
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>
This commit is contained in:
Marco Bubke
2021-12-01 16:28:15 +01:00
parent 815383bf0a
commit c1ca2a7103
3 changed files with 74 additions and 25 deletions

View File

@@ -27,6 +27,8 @@
#include <utils/smallstringview.h> #include <utils/smallstringview.h>
#include <type_traits>
namespace Sqlite { namespace Sqlite {
constexpr int compare(Utils::SmallStringView first, Utils::SmallStringView second) noexcept constexpr int compare(Utils::SmallStringView first, Utils::SmallStringView second) noexcept
@@ -41,6 +43,8 @@ constexpr int compare(Utils::SmallStringView first, Utils::SmallStringView secon
return difference; return difference;
} }
enum class UpdateChange { No, Update };
template<typename SqliteRange, template<typename SqliteRange,
typename Range, typename Range,
typename CompareKey, typename CompareKey,
@@ -58,6 +62,7 @@ void insertUpdateDelete(SqliteRange &&sqliteRange,
auto endSqliteIterator = sqliteRange.end(); auto endSqliteIterator = sqliteRange.end();
auto currentValueIterator = values.begin(); auto currentValueIterator = values.begin();
auto endValueIterator = values.end(); auto endValueIterator = values.end();
std::optional<std::decay_t<decltype(*currentValueIterator)>> lastValue;
while (true) { while (true) {
bool hasMoreValues = currentValueIterator != endValueIterator; bool hasMoreValues = currentValueIterator != endValueIterator;
@@ -67,21 +72,42 @@ void insertUpdateDelete(SqliteRange &&sqliteRange,
auto &&value = *currentValueIterator; auto &&value = *currentValueIterator;
auto compare = compareKey(sqliteValue, value); auto compare = compareKey(sqliteValue, value);
if (compare == 0) { if (compare == 0) {
updateCallback(sqliteValue, value); UpdateChange updateChange = updateCallback(sqliteValue, value);
++currentValueIterator; switch (updateChange) {
case UpdateChange::Update:
lastValue = value;
break;
case UpdateChange::No:
lastValue.reset();
break;
}
++currentSqliteIterator; ++currentSqliteIterator;
++currentValueIterator;
} else if (compare > 0) { } else if (compare > 0) {
insertCallback(value); insertCallback(value);
++currentValueIterator; ++currentValueIterator;
} else if (compare < 0) { } else if (compare < 0) {
if (lastValue) {
if (compareKey(sqliteValue, *lastValue) != 0)
deleteCallback(sqliteValue); deleteCallback(sqliteValue);
lastValue.reset();
} else {
deleteCallback(sqliteValue);
}
++currentSqliteIterator; ++currentSqliteIterator;
} }
} else if (hasMoreValues) { } else if (hasMoreValues) {
insertCallback(*currentValueIterator); insertCallback(*currentValueIterator);
++currentValueIterator; ++currentValueIterator;
} else if (hasMoreSqliteValues) { } else if (hasMoreSqliteValues) {
deleteCallback(*currentSqliteIterator); auto &&sqliteValue = *currentSqliteIterator;
if (lastValue) {
if (compareKey(sqliteValue, *lastValue) != 0)
deleteCallback(sqliteValue);
lastValue.reset();
} else {
deleteCallback(sqliteValue);
}
++currentSqliteIterator; ++currentSqliteIterator;
} else { } else {
break; break;

View File

@@ -581,7 +581,10 @@ private:
&projectData.sourceId, &projectData.sourceId,
&projectData.moduleId, &projectData.moduleId,
static_cast<int>(projectData.fileType)); static_cast<int>(projectData.fileType));
return Sqlite::UpdateChange::Update;
} }
return Sqlite::UpdateChange::No;
}; };
auto remove = [&](const Storage::ProjectData &projectData) { auto remove = [&](const Storage::ProjectData &projectData) {
@@ -621,7 +624,10 @@ private:
updateFileStatusStatement.write(&fileStatus.sourceId, updateFileStatusStatement.write(&fileStatus.sourceId,
fileStatus.size, fileStatus.size,
fileStatus.lastModified); fileStatus.lastModified);
return Sqlite::UpdateChange::Update;
} }
return Sqlite::UpdateChange::No;
}; };
auto remove = [&](const FileStatus &fileStatus) { auto remove = [&](const FileStatus &fileStatus) {
@@ -982,7 +988,9 @@ private:
relinkableAliasPropertyDeclarations); relinkableAliasPropertyDeclarations);
handlePrototypes(view.typeId, relinkablePrototypes); handlePrototypes(view.typeId, relinkablePrototypes);
updateExportedTypeNameTypeIdStatement.write(&view.exportedTypeNameId, &type.typeId); updateExportedTypeNameTypeIdStatement.write(&view.exportedTypeNameId, &type.typeId);
return Sqlite::UpdateChange::Update;
} }
return Sqlite::UpdateChange::No;
}; };
auto remove = [&](const Storage::ExportedTypeView &view) { auto remove = [&](const Storage::ExportedTypeView &view) {
@@ -1056,7 +1064,7 @@ private:
view.aliasId); view.aliasId);
} }
void synchronizePropertyDeclarationsUpdateProperty(const Storage::PropertyDeclarationView &view, auto synchronizePropertyDeclarationsUpdateProperty(const Storage::PropertyDeclarationView &view,
const Storage::PropertyDeclaration &value, const Storage::PropertyDeclaration &value,
SourceId sourceId, SourceId sourceId,
PropertyDeclarationIds &propertyDeclarationIds) PropertyDeclarationIds &propertyDeclarationIds)
@@ -1070,7 +1078,7 @@ private:
if (view.traits == value.traits && propertyTypeId == view.typeId if (view.traits == value.traits && propertyTypeId == view.typeId
&& propertyImportedTypeNameId == view.typeNameId) && propertyImportedTypeNameId == view.typeNameId)
return; return Sqlite::UpdateChange::No;
updatePropertyDeclarationStatement.write(&view.id, updatePropertyDeclarationStatement.write(&view.id,
&propertyTypeId, &propertyTypeId,
@@ -1079,6 +1087,7 @@ private:
updatePropertyAliasDeclarationRecursivelyWithTypeAndTraitsStatement updatePropertyAliasDeclarationRecursivelyWithTypeAndTraitsStatement
.write(&view.id, &propertyTypeId, static_cast<int>(value.traits)); .write(&view.id, &propertyTypeId, static_cast<int>(value.traits));
propertyDeclarationIds.push_back(view.id); propertyDeclarationIds.push_back(view.id);
return Sqlite::UpdateChange::Update;
} }
void synchronizePropertyDeclarations(TypeId typeId, void synchronizePropertyDeclarations(TypeId typeId,
@@ -1122,11 +1131,13 @@ private:
sourceId); sourceId);
propertyDeclarationIds.push_back(view.id); propertyDeclarationIds.push_back(view.id);
} else { } else {
synchronizePropertyDeclarationsUpdateProperty(view, return synchronizePropertyDeclarationsUpdateProperty(view,
value, value,
sourceId, sourceId,
propertyDeclarationIds); propertyDeclarationIds);
} }
return Sqlite::UpdateChange::No;
}; };
auto remove = [&](const Storage::PropertyDeclarationView &view) { auto remove = [&](const Storage::PropertyDeclarationView &view) {
@@ -1185,7 +1196,7 @@ private:
auto insert = [&](const Storage::PropertyDeclaration &) {}; auto insert = [&](const Storage::PropertyDeclaration &) {};
auto update = [&](const AliasPropertyDeclarationView &, auto update = [&](const AliasPropertyDeclarationView &,
const Storage::PropertyDeclaration &) {}; const Storage::PropertyDeclaration &) { return Sqlite::UpdateChange::No; };
auto remove = [&](const AliasPropertyDeclarationView &view) { auto remove = [&](const AliasPropertyDeclarationView &view) {
updatePropertyDeclarationAliasIdToNullStatement.write(&view.id); updatePropertyDeclarationAliasIdToNullStatement.write(&view.id);
@@ -1251,7 +1262,9 @@ private:
} }
}; };
auto update = [](const Storage::ImportView &, const Storage::Import &) {}; auto update = [](const Storage::ImportView &, const Storage::Import &) {
return Sqlite::UpdateChange::No;
};
auto remove = [&](const Storage::ImportView &view) { auto remove = [&](const Storage::ImportView &view) {
deleteDocumentImportStatement.write(&view.importId); deleteDocumentImportStatement.write(&view.importId);
@@ -1316,9 +1329,11 @@ private:
Utils::PathString signature{createJson(value.parameters)}; Utils::PathString signature{createJson(value.parameters)};
if (value.returnTypeName == view.returnTypeName && signature == view.signature) if (value.returnTypeName == view.returnTypeName && signature == view.signature)
return; return Sqlite::UpdateChange::No;
updateFunctionDeclarationStatement.write(&view.id, value.returnTypeName, signature); updateFunctionDeclarationStatement.write(&view.id, value.returnTypeName, signature);
return Sqlite::UpdateChange::Update;
}; };
auto remove = [&](const Storage::FunctionDeclarationView &view) { auto remove = [&](const Storage::FunctionDeclarationView &view) {
@@ -1353,9 +1368,11 @@ private:
Utils::PathString signature{createJson(value.parameters)}; Utils::PathString signature{createJson(value.parameters)};
if (signature == view.signature) if (signature == view.signature)
return; return Sqlite::UpdateChange::No;
updateSignalDeclarationStatement.write(&view.id, signature); updateSignalDeclarationStatement.write(&view.id, signature);
return Sqlite::UpdateChange::Update;
}; };
auto remove = [&](const Storage::SignalDeclarationView &view) { auto remove = [&](const Storage::SignalDeclarationView &view) {
@@ -1418,9 +1435,11 @@ private:
Utils::PathString enumeratorDeclarations{createJson(value.enumeratorDeclarations)}; Utils::PathString enumeratorDeclarations{createJson(value.enumeratorDeclarations)};
if (enumeratorDeclarations == view.enumeratorDeclarations) if (enumeratorDeclarations == view.enumeratorDeclarations)
return; return Sqlite::UpdateChange::No;
updateEnumerationDeclarationStatement.write(&view.id, enumeratorDeclarations); updateEnumerationDeclarationStatement.write(&view.id, enumeratorDeclarations);
return Sqlite::UpdateChange::Update;
}; };
auto remove = [&](const Storage::EnumerationDeclarationView &view) { auto remove = [&](const Storage::EnumerationDeclarationView &view) {
@@ -2066,6 +2085,7 @@ private:
{ {
Sqlite::Table table; Sqlite::Table table;
table.setUseIfNotExists(true); table.setUseIfNotExists(true);
table.setUseWithoutRowId(true);
table.setName("projectDatas"); table.setName("projectDatas");
auto &projectSourceIdColumn = table.addColumn("projectSourceId"); auto &projectSourceIdColumn = table.addColumn("projectSourceId");
auto &sourceIdColumn = table.addColumn("sourceId"); auto &sourceIdColumn = table.addColumn("sourceId");

View File

@@ -36,16 +36,14 @@ namespace {
class KeyValueView class KeyValueView
{ {
public: public:
KeyValueView(Utils::SmallStringView key, long long value, long long rowid) KeyValueView(Utils::SmallStringView key, long long value)
: key{key} : key{key}
, value{value} , value{value}
, rowid{rowid}
{} {}
public: public:
Utils::SmallStringView key; Utils::SmallStringView key;
long long value = 0; long long value = 0;
long long rowid = -1;
}; };
std::ostream &operator<<(std::ostream &out, KeyValueView keyValueView) std::ostream &operator<<(std::ostream &out, KeyValueView keyValueView)
@@ -103,6 +101,7 @@ public:
{ {
Sqlite::Table table; Sqlite::Table table;
table.setName("data"); table.setName("data");
table.setUseWithoutRowId(true);
table.addColumn("key", Sqlite::ColumnType::None, {Sqlite::PrimaryKey{}}); table.addColumn("key", Sqlite::ColumnType::None, {Sqlite::PrimaryKey{}});
table.addColumn("value"); table.addColumn("value");
@@ -118,22 +117,26 @@ protected:
Sqlite::Database database{":memory:", Sqlite::JournalMode::Memory}; Sqlite::Database database{":memory:", Sqlite::JournalMode::Memory};
Sqlite::ImmediateTransaction transaction{database}; Sqlite::ImmediateTransaction transaction{database};
Initializer initializer{database}; Initializer initializer{database};
Sqlite::ReadStatement<3> selectViewsStatement{"SELECT key, value, rowid FROM data ORDER BY key", Sqlite::ReadStatement<2> selectViewsStatement{"SELECT key, value FROM data ORDER BY key",
database}; database};
Sqlite::ReadStatement<2> selectValuesStatement{"SELECT key, value FROM data ORDER BY key", Sqlite::ReadStatement<2> selectValuesStatement{"SELECT key, value FROM data ORDER BY key",
database}; database};
Sqlite::WriteStatement insertStatement{"INSERT INTO data(key, value) VALUES (?1, ?2)", database}; Sqlite::WriteStatement insertStatement{"INSERT INTO data(key, value) VALUES (?1, ?2)", database};
Sqlite::WriteStatement updateStatement{"UPDATE data SET value = ?2 WHERE rowid = ?1", database}; Sqlite::WriteStatement updateStatement{"UPDATE data SET value = ?2 WHERE key=?1", database};
Sqlite::WriteStatement deleteStatement{"DELETE FROM data WHERE rowid = ?", database}; Sqlite::WriteStatement deleteStatement{"DELETE FROM data WHERE key=?", database};
std::function<void(const KeyValue &keyValue)> insert{ std::function<void(const KeyValue &keyValue)> insert{
[&](const KeyValue &keyValue) { insertStatement.write(keyValue.key, keyValue.value); }}; [&](const KeyValue &keyValue) { insertStatement.write(keyValue.key, keyValue.value); }};
std::function<void(KeyValueView keyValueView, const KeyValue &keyValue)> update{ std::function<Sqlite::UpdateChange(KeyValueView keyValueView, const KeyValue &keyValue)> update{
[&](KeyValueView keyValueView, const KeyValue &keyValue) { [&](KeyValueView keyValueView, const KeyValue &keyValue) {
if (!(keyValueView == keyValue)) if (!(keyValueView == keyValue)) {
updateStatement.write(keyValueView.rowid, keyValue.value); updateStatement.write(keyValueView.key, keyValue.value);
return Sqlite::UpdateChange::Update;
}
return Sqlite::UpdateChange::No;
}}; }};
std::function<void(KeyValueView keyValueView)> remove{ std::function<void(KeyValueView keyValueView)> remove{
[&](KeyValueView keyValueView) { deleteStatement.write(keyValueView.rowid); }}; [&](KeyValueView keyValueView) { deleteStatement.write(keyValueView.key); }};
}; };
auto compareKey = [](KeyValueView keyValueView, const KeyValue &keyValue) { auto compareKey = [](KeyValueView keyValueView, const KeyValue &keyValue) {