Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update expensify_prod branch #1616

Merged
merged 2 commits into from
Nov 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions sqlitecluster/SQLite.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -540,8 +540,9 @@ bool SQLite::_writeIdempotent(const string& query, bool alwaysKeepQueries) {
SASSERT(_insideTransaction);
_queryCache.clear();
_queryCount++;
SASSERT(query.empty() || SEndsWith(query, ";")); // Must finish everything with semicolon
SASSERTWARN(SToUpper(query).find("CURRENT_TIMESTAMP") == string::npos); // Else will be replayed wrong

// Must finish everything with semicolon.
SASSERT(query.empty() || SEndsWith(query, ";"));

// First, check our current state
SQResult results;
Expand All @@ -550,6 +551,7 @@ bool SQLite::_writeIdempotent(const string& query, bool alwaysKeepQueries) {
uint64_t schemaBefore = SToUInt64(results[0][0]);
uint64_t changesBefore = sqlite3_total_changes(_db);

_currentlyWriting = true;
// Try to execute the query
uint64_t before = STimeNow();
bool usedRewrittenQuery = false;
Expand All @@ -573,12 +575,14 @@ bool SQLite::_writeIdempotent(const string& query, bool alwaysKeepQueries) {

// If we got a constraints error, throw that.
if (resultCode == SQLITE_CONSTRAINT) {
_currentlyWriting = false;
throw constraint_error();
}

_checkInterruptErrors("SQLite::write"s);
_writeElapsed += STimeNow() - before;
if (resultCode) {
_currentlyWriting = false;
return false;
}

Expand All @@ -592,6 +596,8 @@ bool SQLite::_writeIdempotent(const string& query, bool alwaysKeepQueries) {
if (alwaysKeepQueries || (schemaAfter > schemaBefore) || (changesAfter > changesBefore)) {
_uncommittedQuery += usedRewrittenQuery ? _rewrittenQuery : query;
}

_currentlyWriting = false;
return true;
}

Expand Down Expand Up @@ -929,6 +935,13 @@ int SQLite::_authorize(int actionCode, const char* detail1, const char* detail2,
) {
_isDeterministicQuery = false;
}

if (!strcmp(detail2, "current_timestamp")) {
if (_currentlyWriting) {
// Prevent using `current_timestamp` in writes which could cause synchronization with followers to result in inconsistent data.
return SQLITE_DENY;
}
}
}

// If the whitelist isn't set, we always return OK.
Expand Down
3 changes: 3 additions & 0 deletions sqlitecluster/SQLite.h
Original file line number Diff line number Diff line change
Expand Up @@ -506,4 +506,7 @@ class SQLite {

// This is a string (which may be empty) containing the most recent logged error by SQLite in this thread.
static thread_local string _mostRecentSQLiteErrorLog;

// Set to true inside of a write query.
bool _currentlyWriting{false};
};
15 changes: 15 additions & 0 deletions test/tests/WriteTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ struct WriteTest : tpunit::TestFixture {
TEST(WriteTest::updateAndInsertWithHttp),
TEST(WriteTest::shortHandSyntax),
TEST(WriteTest::keywordsAsValue),
TEST(WriteTest::blockTimeFunctions),
AFTER_CLASS(WriteTest::tearDown)) { }

BedrockTester* tester;
Expand Down Expand Up @@ -183,4 +184,18 @@ struct WriteTest : tpunit::TestFixture {
tester->executeWaitVerifyContent(query3);
}

void blockTimeFunctions() {
// Verify writing the string 'CURRENT_TIMESTAMP' is fine.
SData query("query: INSERT INTO stuff VALUES ( NULL, 11, 'CURRENT_TIMESTAMP' );");
tester->executeWaitVerifyContent(query);

// But verify calling the function CURRENT_TIMESTAMP is blocked when writing.
query.methodLine = "query: INSERT INTO stuff VALUES ( NULL, 11, CURRENT_TIMESTAMP );";
tester->executeWaitVerifyContent(query, "502 Query failed");

// But allow the function to run in reads.
query.methodLine = "query: SELECT CURRENT_TIMESTAMP;";
tester->executeWaitVerifyContent(query);
}

} __WriteTest;