Page MenuHomePhabricator
Differential D14517

[SQLite] implement method to copy table between databases
AcceptedPublic
Actions

Authored by kamil on Thu, Apr 3, 2:37 AM.

Details

Reviewers
bartek
Summary

ENG-10489.

Depends on D14516

Test Plan

Tested later in the stack, existing tables (e.g. olm_persist_account) are not overwritten during restore

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil created this revision.Thu, Apr 3, 2:37 AM
kamil held this revision as a draft.
Herald added subscribers: tomek, ashoat. · View Herald TranscriptThu, Apr 3, 2:37 AM
kamil added a child revision: D14518: [SQLite] unify `restoreFromMainCompaction` on platforms.Thu, Apr 3, 2:42 AM
Harbormaster completed remote builds in B33872: Diff 47622.Thu, Apr 3, 2:55 AM
kamil published this revision for review.Thu, Apr 3, 3:10 AM
kamil added inline comments.
native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
3313 ↗(On Diff #47622)

We need to use this only on not-encrypted databases, see next diff in the stack

3317 ↗(On Diff #47622)

We want OR IGNORE statement to skip duplicated primary keys, the example is a thread with Ashoat, which is created for each user (we won't include thin thread data in backup, but we do now, so without this each copy statement could fail)

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.h
58 ↗(On Diff #47622)

We might want to have a custom copy method (by specifying fields), e.g. for messages where we have one generated column

bartek added a comment.Thu, Apr 3, 3:50 AM

Don't we want this method to take a vector of tables to copy? Looking at D14519, I see you're running this in a loop, so the ATTACH DATABASE ..., DETACH statements are repeated for each table

kamil planned changes to this revision.Thu, Apr 3, 3:55 AM
In D14517#403050, @bartek wrote:

Don't we want this method to take a vector of tables to copy? Looking at D14519, I see you're running this in a loop, so the ATTACH DATABASE ..., DETACH statements are repeated for each table

Good call

kamil updated this revision to Diff 47639.Thu, Apr 3, 5:20 AM

address review

Harbormaster completed remote builds in B33889: Diff 47639.Thu, Apr 3, 5:39 AM
bartek accepted this revision.Thu, Apr 3, 7:25 AM
This revision is now accepted and ready to land.Thu, Apr 3, 7:25 AM

Revision Contents
Changeset List

PathSize
native/
cpp/
CommonCpp/
DatabaseManagers/
4 lines
22 lines

Diff 47639

View Options

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.h

Show First 20 Lines • Show All 3,330 Lines • ▼ Show 20 Linesvoid SQLiteQueryExecutor::restoreFromMainCompaction(
} }
sqlite3 *backupDB; sqlite3 *backupDB;
if (!is_database_queryable( if (!is_database_queryable(
backupDB, true, mainCompactionPath, mainCompactionEncryptionKey)) { backupDB, true, mainCompactionPath, mainCompactionEncryptionKey)) {
throw std::runtime_error("Backup file or encryption key corrupted."); throw std::runtime_error("Backup file or encryption key corrupted.");
} }
// We don't want to run `PRAGMA key = ...;`
// on main web database. The context is here:
// https://linear.app/comm/issue/ENG-6398/issues-with-sqlcipher-on-web
#ifdef EMSCRIPTEN
kamilAuthorUnsubmitted
Done
Inline Actions

We follow web approach so this comment is not needed anymore

kamil: We follow web approach so this comment is not needed anymore
std::string plaintextBackupPath = mainCompactionPath + "_plaintext"; std::string plaintextBackupPath = mainCompactionPath + "_plaintext";
if (file_exists(plaintextBackupPath)) { if (file_exists(plaintextBackupPath)) {
attempt_delete_file( attempt_delete_file(
plaintextBackupPath, plaintextBackupPath,
"Failed to delete plaintext backup file from previous backup " "Failed to delete plaintext backup file from previous backup "
"attempt."); "attempt.");
} }
sqlite3_open(mainCompactionPath.c_str(), &backupDB);
std::string plaintextMigrationDBQuery = "PRAGMA key = \"x'" + std::string plaintextMigrationDBQuery = "PRAGMA key = \"x'" +
mainCompactionEncryptionKey + mainCompactionEncryptionKey +
"'\";" "'\";"
"ATTACH DATABASE '" + "ATTACH DATABASE '" +
plaintextBackupPath + plaintextBackupPath +
"' AS plaintext KEY '';" "' AS plaintext KEY '';"
"SELECT sqlcipher_export('plaintext');" "SELECT sqlcipher_export('plaintext');"
"DETACH DATABASE plaintext;"; "DETACH DATABASE plaintext;";
executeQuery(backupDB, plaintextMigrationDBQuery);
sqlite3_open(mainCompactionPath.c_str(), &backupDB);
char *plaintextMigrationErr;
sqlite3_exec(
backupDB,
plaintextMigrationDBQuery.c_str(),
nullptr,
nullptr,
&plaintextMigrationErr);
sqlite3_close(backupDB); sqlite3_close(backupDB);
if (plaintextMigrationErr) {
std::stringstream error_message;
error_message << "Failed to migrate backup SQLCipher file to plaintext "
"SQLite file. Details"
<< plaintextMigrationErr << std::endl;
std::string error_message_str = error_message.str();
sqlite3_free(plaintextMigrationErr);
throw std::runtime_error(error_message_str);
}
kamilAuthorUnsubmitted
Done
Inline Actions

executeQuery does exactly the same

kamil: `executeQuery` does exactly the same
sqlite3_open(plaintextBackupPath.c_str(), &backupDB); sqlite3_open(plaintextBackupPath.c_str(), &backupDB);
#else
sqlite3_open(mainCompactionPath.c_str(), &backupDB);
set_encryption_key(backupDB, mainCompactionEncryptionKey);
#endif
int version = this->getSyncedDatabaseVersion(backupDB).value_or(-1); int version = this->getSyncedDatabaseVersion(backupDB).value_or(-1);
if (version > std::stoi(maxVersion)) { if (version > std::stoi(maxVersion)) {
std::stringstream error_message; std::stringstream error_message;
error_message << "Failed to restore a backup because it was created " error_message << "Failed to restore a backup because it was created "
<< "with version " << version << "with version " << version
<< " that is newer than the max supported version " << " that is newer than the max supported version "
<< maxVersion << std::endl; << maxVersion << std::endl;
Show All 20 Lines throw std::runtime_error(
"Programmer error. Database in transaction during restore attempt."); "Programmer error. Database in transaction during restore attempt.");
} else if (backupResult != SQLITE_DONE) { } else if (backupResult != SQLITE_DONE) {
std::stringstream error_message; std::stringstream error_message;
error_message << "Failed to restore database from backup. Details: " error_message << "Failed to restore database from backup. Details: "
<< sqlite3_errstr(backupResult); << sqlite3_errstr(backupResult);
throw std::runtime_error(error_message.str()); throw std::runtime_error(error_message.str());
} }
#ifdef EMSCRIPTEN
attempt_delete_file( attempt_delete_file(
plaintextBackupPath, plaintextBackupPath,
"Failed to delete plaintext compaction file after successful restore."); "Failed to delete plaintext compaction file after successful restore.");
#endif
attempt_delete_file( attempt_delete_file(
mainCompactionPath, mainCompactionPath,
"Failed to delete main compaction file after successful restore."); "Failed to delete main compaction file after successful restore.");
} }
kamilAuthorUnsubmitted
Done
Inline Actions

Previously, there was a bug, for web mainCompactionPath wasn't deleted

kamil: Previously, there was a bug, for web `mainCompactionPath` wasn't deleted
void SQLiteQueryExecutor::restoreFromBackupLog( void SQLiteQueryExecutor::restoreFromBackupLog(
const std::vector<std::uint8_t> &backupLog) const { const std::vector<std::uint8_t> &backupLog) const {
SQLiteQueryExecutor::connectionManager.restoreFromBackupLog(backupLog); SQLiteQueryExecutor::connectionManager.restoreFromBackupLog(backupLog);
} }
} // namespace comm } // namespace comm
View Options

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp

Show First 20 Lines • Show All 3,330 Lines • ▼ Show 20 Linesvoid SQLiteQueryExecutor::restoreFromMainCompaction(
} }
sqlite3 *backupDB; sqlite3 *backupDB;
if (!is_database_queryable( if (!is_database_queryable(
backupDB, true, mainCompactionPath, mainCompactionEncryptionKey)) { backupDB, true, mainCompactionPath, mainCompactionEncryptionKey)) {
throw std::runtime_error("Backup file or encryption key corrupted."); throw std::runtime_error("Backup file or encryption key corrupted.");
} }
// We don't want to run `PRAGMA key = ...;`
// on main web database. The context is here:
// https://linear.app/comm/issue/ENG-6398/issues-with-sqlcipher-on-web
#ifdef EMSCRIPTEN
kamilAuthorUnsubmitted
Done
Inline Actions

We follow web approach so this comment is not needed anymore

kamil: We follow web approach so this comment is not needed anymore
std::string plaintextBackupPath = mainCompactionPath + "_plaintext"; std::string plaintextBackupPath = mainCompactionPath + "_plaintext";
if (file_exists(plaintextBackupPath)) { if (file_exists(plaintextBackupPath)) {
attempt_delete_file( attempt_delete_file(
plaintextBackupPath, plaintextBackupPath,
"Failed to delete plaintext backup file from previous backup " "Failed to delete plaintext backup file from previous backup "
"attempt."); "attempt.");
} }
sqlite3_open(mainCompactionPath.c_str(), &backupDB);
std::string plaintextMigrationDBQuery = "PRAGMA key = \"x'" + std::string plaintextMigrationDBQuery = "PRAGMA key = \"x'" +
mainCompactionEncryptionKey + mainCompactionEncryptionKey +
"'\";" "'\";"
"ATTACH DATABASE '" + "ATTACH DATABASE '" +
plaintextBackupPath + plaintextBackupPath +
"' AS plaintext KEY '';" "' AS plaintext KEY '';"
"SELECT sqlcipher_export('plaintext');" "SELECT sqlcipher_export('plaintext');"
"DETACH DATABASE plaintext;"; "DETACH DATABASE plaintext;";
executeQuery(backupDB, plaintextMigrationDBQuery);
sqlite3_open(mainCompactionPath.c_str(), &backupDB);
char *plaintextMigrationErr;
sqlite3_exec(
backupDB,
plaintextMigrationDBQuery.c_str(),
nullptr,
nullptr,
&plaintextMigrationErr);
sqlite3_close(backupDB); sqlite3_close(backupDB);
if (plaintextMigrationErr) {
std::stringstream error_message;
error_message << "Failed to migrate backup SQLCipher file to plaintext "
"SQLite file. Details"
<< plaintextMigrationErr << std::endl;
std::string error_message_str = error_message.str();
sqlite3_free(plaintextMigrationErr);
throw std::runtime_error(error_message_str);
}
kamilAuthorUnsubmitted
Done
Inline Actions

executeQuery does exactly the same

kamil: `executeQuery` does exactly the same
sqlite3_open(plaintextBackupPath.c_str(), &backupDB); sqlite3_open(plaintextBackupPath.c_str(), &backupDB);
#else
sqlite3_open(mainCompactionPath.c_str(), &backupDB);
set_encryption_key(backupDB, mainCompactionEncryptionKey);
#endif
int version = this->getSyncedDatabaseVersion(backupDB).value_or(-1); int version = this->getSyncedDatabaseVersion(backupDB).value_or(-1);
if (version > std::stoi(maxVersion)) { if (version > std::stoi(maxVersion)) {
std::stringstream error_message; std::stringstream error_message;
error_message << "Failed to restore a backup because it was created " error_message << "Failed to restore a backup because it was created "
<< "with version " << version << "with version " << version
<< " that is newer than the max supported version " << " that is newer than the max supported version "
<< maxVersion << std::endl; << maxVersion << std::endl;
Show All 20 Lines throw std::runtime_error(
"Programmer error. Database in transaction during restore attempt."); "Programmer error. Database in transaction during restore attempt.");
} else if (backupResult != SQLITE_DONE) { } else if (backupResult != SQLITE_DONE) {
std::stringstream error_message; std::stringstream error_message;
error_message << "Failed to restore database from backup. Details: " error_message << "Failed to restore database from backup. Details: "
<< sqlite3_errstr(backupResult); << sqlite3_errstr(backupResult);
throw std::runtime_error(error_message.str()); throw std::runtime_error(error_message.str());
} }
#ifdef EMSCRIPTEN
attempt_delete_file( attempt_delete_file(
plaintextBackupPath, plaintextBackupPath,
"Failed to delete plaintext compaction file after successful restore."); "Failed to delete plaintext compaction file after successful restore.");
#endif
attempt_delete_file( attempt_delete_file(
mainCompactionPath, mainCompactionPath,
"Failed to delete main compaction file after successful restore."); "Failed to delete main compaction file after successful restore.");
} }
kamilAuthorUnsubmitted
Done
Inline Actions

Previously, there was a bug, for web mainCompactionPath wasn't deleted

kamil: Previously, there was a bug, for web `mainCompactionPath` wasn't deleted
void SQLiteQueryExecutor::restoreFromBackupLog( void SQLiteQueryExecutor::restoreFromBackupLog(
const std::vector<std::uint8_t> &backupLog) const { const std::vector<std::uint8_t> &backupLog) const {
SQLiteQueryExecutor::connectionManager.restoreFromBackupLog(backupLog); SQLiteQueryExecutor::connectionManager.restoreFromBackupLog(backupLog);
} }
} // namespace comm } // namespace comm