diff --git a/sqlitecluster/SQLite.cpp b/sqlitecluster/SQLite.cpp index 8dc02077c..2ca08af57 100644 --- a/sqlitecluster/SQLite.cpp +++ b/sqlitecluster/SQLite.cpp @@ -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; @@ -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; @@ -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; } @@ -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; } @@ -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. diff --git a/sqlitecluster/SQLite.h b/sqlitecluster/SQLite.h index 6321ac6da..ac5dd30e0 100644 --- a/sqlitecluster/SQLite.h +++ b/sqlitecluster/SQLite.h @@ -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}; }; diff --git a/test/tests/WriteTest.cpp b/test/tests/WriteTest.cpp index cc51a37c0..7407738e6 100644 --- a/test/tests/WriteTest.cpp +++ b/test/tests/WriteTest.cpp @@ -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; @@ -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;