Commit 1f7e168e authored by Hugo Beauzée-Luyssen's avatar Hugo Beauzée-Luyssen

sqlite: Allow pragmas to be changed for the current connection

This removes the need for a specific constructor, and mostly allows us
not to instantiate a new short lived connection, which will make
cleaning up the multithreaded connections lifetime mess
parent 3f8ceb44
...@@ -811,12 +811,11 @@ bool MediaLibrary::recreateDatabase() ...@@ -811,12 +811,11 @@ bool MediaLibrary::recreateDatabase()
bool MediaLibrary::migrateModel3to4() bool MediaLibrary::migrateModel3to4()
{ {
/* /*
* Get a special SqliteConnection with Foreign Keys deactivated * Disable Foreign Keys & recursive triggers to avoid cascading deletion
* to avoid cascade deletion while remodeling the database into * while remodeling the database into the transaction.
* the transaction.
*/ */
SqliteConnection conn( getConn()->getDBPath(), false ); SqliteConnection::WeakDbContext weakConnCtx{ getConn() };
auto t = conn.newTransaction(); auto t = getConn()->newTransaction();
using namespace policy; using namespace policy;
// As SQLite do not allow us to remove or add some constraints, // As SQLite do not allow us to remove or add some constraints,
// we use the method described here https://www.sqlite.org/faq.html#q11 // we use the method described here https://www.sqlite.org/faq.html#q11
...@@ -826,12 +825,12 @@ bool MediaLibrary::migrateModel3to4() ...@@ -826,12 +825,12 @@ bool MediaLibrary::migrateModel3to4()
for ( const auto& req : reqs ) for ( const auto& req : reqs )
{ {
if ( sqlite::Tools::executeRequest( &conn, req ) == false ) if ( sqlite::Tools::executeRequest( getConn(), req ) == false )
return false; return false;
} }
// Re-create triggers removed in the process // Re-create triggers removed in the process
Media::createTriggers( &conn ); Media::createTriggers( getConn() );
Playlist::createTriggers( &conn ); Playlist::createTriggers( getConn() );
t->commit(); t->commit();
return true; return true;
} }
......
...@@ -32,15 +32,9 @@ namespace medialibrary ...@@ -32,15 +32,9 @@ namespace medialibrary
{ {
SqliteConnection::SqliteConnection( const std::string &dbPath ) SqliteConnection::SqliteConnection( const std::string &dbPath )
: SqliteConnection( dbPath, true )
{
}
SqliteConnection::SqliteConnection( const std::string &dbPath, bool enableForeignKeys )
: m_dbPath( dbPath ) : m_dbPath( dbPath )
, m_readLock( m_contextLock ) , m_readLock( m_contextLock )
, m_writeLock( m_contextLock ) , m_writeLock( m_contextLock )
, m_enableForeignKeys( enableForeignKeys )
{ {
if ( sqlite3_threadsafe() == 0 ) if ( sqlite3_threadsafe() == 0 )
throw std::runtime_error( "SQLite isn't built with threadsafe mode" ); throw std::runtime_error( "SQLite isn't built with threadsafe mode" );
...@@ -67,15 +61,11 @@ SqliteConnection::Handle SqliteConnection::getConn() ...@@ -67,15 +61,11 @@ SqliteConnection::Handle SqliteConnection::getConn()
+ sqlite3_errstr( res ) ); + sqlite3_errstr( res ) );
sqlite3_extended_result_codes( dbConnection, 1 ); sqlite3_extended_result_codes( dbConnection, 1 );
sqlite3_busy_timeout( dbConnection, 500 ); sqlite3_busy_timeout( dbConnection, 500 );
std::string pragmaForeignReq = "PRAGMA foreign_keys = "; // Don't use public wrapper, they need to be able to call getConn, which
sqlite::Statement s( dbConnection, pragmaForeignReq + ( m_enableForeignKeys ? "ON" : "OFF" ) ); // would result from a recursive call and a deadlock from here.
s.execute(); setPragmaEnabled( dbConnection, "foreign_keys", true );
while ( s.row() != nullptr ) setPragmaEnabled( dbConnection, "recursive_triggers", true );
;
sqlite::Statement s2( dbConnection, "PRAGMA recursive_triggers = ON" );
s2.execute();
while ( s2.row() != nullptr )
;
m_conns.emplace( compat::this_thread::get_id(), std::move( dbConn ) ); m_conns.emplace( compat::this_thread::get_id(), std::move( dbConn ) );
sqlite3_update_hook( dbConnection, &updateHook, this ); sqlite3_update_hook( dbConnection, &updateHook, this );
return dbConnection; return dbConnection;
...@@ -98,9 +88,51 @@ SqliteConnection::WriteContext SqliteConnection::acquireWriteContext() ...@@ -98,9 +88,51 @@ SqliteConnection::WriteContext SqliteConnection::acquireWriteContext()
return WriteContext{ m_writeLock }; return WriteContext{ m_writeLock };
} }
std::string SqliteConnection::getDBPath() void SqliteConnection::setPragmaEnabled( Handle conn,
const std::string& pragmaName,
bool value )
{
std::string reqBase = std::string{ "PRAGMA " } + pragmaName;
std::string reqSet = reqBase + " = " + ( value ? "ON" : "OFF" );
sqlite::Statement stmt( conn, reqSet );
stmt.execute();
if ( stmt.row() != nullptr )
throw std::runtime_error( "Failed to enable/disable " + pragmaName );
sqlite::Statement stmtCheck( conn, reqBase );
stmtCheck.execute();
auto resultRow = stmtCheck.row();
bool resultValue;
resultRow >> resultValue;
if( resultValue != value )
throw std::runtime_error( "PRAGMA " + pragmaName + " value mismatch" );
}
void SqliteConnection::setForeignKeyEnabled( bool value )
{
// Ensure no transaction will be started during the pragma change
auto ctx = acquireWriteContext();
// Changing this pragma during a transaction is a no-op (silently ignored by
// sqlite), so ensure we're doing something usefull here:
assert( sqlite::Transaction::transactionInProgress() == false );
setPragmaEnabled( getConn(), "foreign_keys", value );
}
void SqliteConnection::setRecursiveTriggers( bool value )
{ {
return m_dbPath; // Ensure no request will run while we change this setting
auto ctx = acquireWriteContext();
// Changing the recursive_triggers setting affects the execution of all
// statements prepared using the database connection, including those
// prepared before the setting was changed. Any existing statements prepared
// using the legacy sqlite3_prepare() interface may fail with an
// SQLITE_SCHEMA error after the recursive_triggers setting is changed.
// https://sqlite.org/pragma.html#pragma_recursive_triggers
sqlite::Statement::FlushStatementCache();
setPragmaEnabled( getConn(), "recursive_triggers", value );
} }
void SqliteConnection::registerUpdateHook( const std::string& table, SqliteConnection::UpdateHookCb cb ) void SqliteConnection::registerUpdateHook( const std::string& table, SqliteConnection::UpdateHookCb cb )
...@@ -129,4 +161,18 @@ void SqliteConnection::updateHook( void* data, int reason, const char*, ...@@ -129,4 +161,18 @@ void SqliteConnection::updateHook( void* data, int reason, const char*,
} }
} }
SqliteConnection::WeakDbContext::WeakDbContext( SqliteConnection* conn )
: m_conn( conn )
{
m_conn->setForeignKeyEnabled( false );
m_conn->setRecursiveTriggers( false );
}
SqliteConnection::WeakDbContext::~WeakDbContext()
{
m_conn->setForeignKeyEnabled( true );
m_conn->setRecursiveTriggers( true );
}
} }
...@@ -54,10 +54,21 @@ public: ...@@ -54,10 +54,21 @@ public:
Delete, Delete,
Update Update
}; };
struct WeakDbContext
{
WeakDbContext( SqliteConnection* conn );
~WeakDbContext();
WeakDbContext( const WeakDbContext& ) = delete;
WeakDbContext( WeakDbContext&& ) = delete;
WeakDbContext& operator=( const WeakDbContext& ) = delete;
WeakDbContext& operator=( WeakDbContext&& ) = delete;
private:
SqliteConnection* m_conn;
};
using UpdateHookCb = std::function<void(HookReason, int64_t)>; using UpdateHookCb = std::function<void(HookReason, int64_t)>;
explicit SqliteConnection( const std::string& dbPath ); explicit SqliteConnection( const std::string& dbPath );
explicit SqliteConnection( const std::string& dbPath, bool enableForeignKeys );
~SqliteConnection(); ~SqliteConnection();
// Returns the current thread's connection // Returns the current thread's connection
// This will initiate a connection if required // This will initiate a connection if required
...@@ -65,11 +76,13 @@ public: ...@@ -65,11 +76,13 @@ public:
std::unique_ptr<sqlite::Transaction> newTransaction(); std::unique_ptr<sqlite::Transaction> newTransaction();
ReadContext acquireReadContext(); ReadContext acquireReadContext();
WriteContext acquireWriteContext(); WriteContext acquireWriteContext();
std::string getDBPath(); void setForeignKeyEnabled( bool value );
void setRecursiveTriggers( bool value );
void registerUpdateHook( const std::string& table, UpdateHookCb cb ); void registerUpdateHook( const std::string& table, UpdateHookCb cb );
private: private:
void setPragmaEnabled( Handle conn, const std::string& pragmaName, bool value );
static void updateHook( void* data, int reason, const char* database, static void updateHook( void* data, int reason, const char* database,
const char* table, sqlite_int64 rowId ); const char* table, sqlite_int64 rowId );
...@@ -82,7 +95,6 @@ private: ...@@ -82,7 +95,6 @@ private:
utils::ReadLocker m_readLock; utils::ReadLocker m_readLock;
utils::WriteLocker m_writeLock; utils::WriteLocker m_writeLock;
std::unordered_map<std::string, UpdateHookCb> m_hooks; std::unordered_map<std::string, UpdateHookCb> m_hooks;
bool m_enableForeignKeys;
}; };
} }
......
...@@ -50,6 +50,7 @@ public: ...@@ -50,6 +50,7 @@ public:
{ {
unlink("test.db"); unlink("test.db");
std::ifstream file{ SRC_DIR "/test/unittest/db_v3.sql" }; std::ifstream file{ SRC_DIR "/test/unittest/db_v3.sql" };
{
medialibrary::SqliteConnection conn{ "test.db" }; medialibrary::SqliteConnection conn{ "test.db" };
// The backup file already contains a transaction // The backup file already contains a transaction
char buff[2048]; char buff[2048];
...@@ -69,7 +70,7 @@ public: ...@@ -69,7 +70,7 @@ public:
row >> dbVersion; row >> dbVersion;
ASSERT_NE( dbVersion, Settings::DbModelVersion ); ASSERT_NE( dbVersion, Settings::DbModelVersion );
ASSERT_EQ( dbVersion, 3u ); ASSERT_EQ( dbVersion, 3u );
}
Reload(); Reload();
} }
}; };
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment