Don't cache explain statements (#2464)

drift/CHANGELOG.md

@@ -11,6 +11,7 @@
   inside of the transaction, instead of causing a deadlock.
 - Improve stack traces for errors happening on drift isolates (which includes
   usages of `NativeDatabase.createInBackground`).
+- Don't cache `EXPLAIN` statements, avoiding schema locks.
 
 ## 2.15.0
 

drift/lib/src/runtime/executor/helpers/engines.dart

@@ -43,7 +43,7 @@ on how to reproduce this.''');
 This database or transaction runner has already been closed and may not be used
 anymore.
 
-If this is happening in a transaction, you might be using the transaction 
+If this is happening in a transaction, you might be using the transaction
 without awaiting every statement in it.''');
     }
 

drift/lib/src/sqlite3/database.dart

@@ -144,48 +144,52 @@ abstract class Sqlite3Delegate<DB extends CommonDatabase>
     if (args.isEmpty) {
       database.execute(statement);
     } else {
-      final stmt = _getPreparedStatement(statement);
+      final (stmt, cached) = _getPreparedStatement(statement);
       try {
         stmt.execute(args);
       } finally {
-        _returnStatement(stmt);
+        _returnStatement(stmt, cached);
       }
     }
   }
 
   @override
   Future<QueryResult> runSelect(String statement, List<Object?> args) async {
-    final stmt = _getPreparedStatement(statement);
+    final (stmt, cached) = _getPreparedStatement(statement);
 
     try {
       final result = stmt.select(args);
       return QueryResult.fromRows(result.toList());
     } finally {
-      _returnStatement(stmt);
+      _returnStatement(stmt, cached);
     }
   }
 
-  CommonPreparedStatement _getPreparedStatement(String sql) {
+  /// Returns a prepared statement for [sql] and reports whether this statement
+  /// was cached.
+  (CommonPreparedStatement, bool) _getPreparedStatement(String sql) {
     if (cachePreparedStatements) {
       final cachedStmt = _preparedStmtsCache.use(sql);
       if (cachedStmt != null) {
-        return cachedStmt;
+        return (cachedStmt, true);
       }
 
       final stmt = database.prepare(sql, checkNoTail: true);
-      _preparedStmtsCache.addNew(sql, stmt);
+      if (!stmt.isExplain) {
+        _preparedStmtsCache.addNew(sql, stmt);
+      }
 
-      return stmt;
+      return (stmt, !stmt.isExplain);
     } else {
       final stmt = database.prepare(sql, checkNoTail: true);
-      return stmt;
+      return (stmt, false);
     }
   }
 
-  void _returnStatement(CommonPreparedStatement stmt) {
+  void _returnStatement(CommonPreparedStatement stmt, bool cached) {
     // When using a statement cache, prepared statements are disposed as they
     // get evicted from the cache, so we don't need to do anything.
-    if (!cachePreparedStatements) {
+    if (!cached) {
       stmt.dispose();
     }
   }

drift/pubspec.yaml

@@ -15,7 +15,7 @@ dependencies:
   js: ^0.6.3
   meta: ^1.3.0
   stream_channel: ^2.1.0
-  sqlite3: ^2.2.0
+  sqlite3: ^2.4.0
   path: ^1.8.0
   stack_trace: ^1.11.1
 

drift/test/platforms/ffi/native_database_test.dart

@@ -185,6 +185,23 @@ void main() {
     expect(underlying.activeStatementCount, isZero);
   });
 
+  test('does not cache explain statements', () async {
+    final db = NativeDatabase.memory(cachePreparedStatements: true);
+    addTearDown(db.close);
+    await db.ensureOpen(_FakeExecutorUser());
+
+    await db.runCustom(
+        'create table test(id integer primary key, description text)');
+    await db.runCustom('create index i1 on test(description)');
+    // The schema is locked while an explain is active, so caching this
+    // statement makes the test fail at the `drop index` statement.
+    await db.runSelect(
+        'explain query plan select * from test where description = ?', ['t']);
+    await db.runCustom('drop index i1');
+    await db.runSelect(
+        'explain query plan select * from test where description = ?', ['t']);
+  });
+
   group('can disable migrations', () {
     Future<void> runTest(QueryExecutor executor) async {
       final db = TodoDb(executor);