davidmartos96 / sqflite_sqlcipher

SQLite flutter plugin
BSD 2-Clause "Simplified" License
99 stars 44 forks source link

Transactions handled different from sqflite #15

Closed jaripekkala closed 4 years ago

jaripekkala commented 4 years ago

Noticed a small difference on how sqflite and sqflite_sqlcipher packages handle nested transactions.

As this package is only adding SQLCipher support to the sqflite, I would assume that both packages behaves the same otherwise.

Example

await database.rawQuery('BEGIN');
await database.rawQuery('BEGIN');
await database.insert('foos', {'bar': 'biz'});
await database.rawQuery('COMMIT');
await database.rawQuery('COMMIT');

sqflite

version 1.3.0 sqflite_common version 1.0.0+1

The record is inserted correctly.

sqflite_sqlcipher

version 1.0.0+5 sqflite_common version 1.0.0+1

Error is thrown. DatabaseException(cannot start a transaction within a transaction)

E/flutter (25686): [ERROR:flutter/lib/ui/ui_dart_state.cc(157)] Unhandled Exception: DatabaseException(cannot start a transaction within a transaction) sql 'BEGIN' args []}
E/flutter (25686): #0      SqfliteSqlCipherDatabaseFactoryImpl.wrapDatabaseException 
package:sqflite_sqlcipher/src/factory_sql_cipher_impl.dart:44
E/flutter (25686): <asynchronous suspension>
E/flutter (25686): #1      SqfliteDatabaseMixin.safeInvokeMethod 
package:sqflite_common/src/database_mixin.dart:208
E/flutter (25686): #2      SqfliteDatabaseMixin.txnRawQuery.<anonymous closure> 
package:sqflite_common/src/database_mixin.dart:394
E/flutter (25686): #3      SqfliteDatabaseMixin.txnSynchronized.<anonymous closure> 
package:sqflite_common/src/database_mixin.dart:327
E/flutter (25686): #4      BasicLock.synchronized 
package:synchronized/src/basic_lock.dart:32
E/flutter (25686): #5      SqfliteDatabaseMixin.txnSynchronized 
package:sqflite_common/src/database_mixin.dart:323
E/flutter (25686): #6      SqfliteDatabaseMixin.txnRawQuery 
package:sqflite_common/src/database_mixin.dart:393
E/flutter (25686): #7      SqfliteDatabaseExecutorMixin._rawQuery 
package:sqflite_common/src/database_mixin.dart:126
E/flutter (25686): #8      SqfliteDatabaseExecutorMixin.rawQuery 
package:sqflite_common/src/database_mixin.dart:120
davidmartos96 commented 4 years ago

I didn't know about that difference. Could you tell me the version of sqflite and sqflite_sqlcipher are you using? sqflite_sqlcipher only changes the native layer of the plugin, the main logic resides in the sqflite_common package, which tekartik maintains. If you tell me the versions you are using I could take a small look to see what is changed.

davidmartos96 commented 4 years ago

Nevermind, I see you linked the packages versions. I will try to reproduce.

davidmartos96 commented 4 years ago

@jaripekkala This seems to be some logic behind the SQLite library bundled with Android. I haven't tested your example in iOS. But your code is not valid in native SQLite or SQLCipher. I tested it on my Linux machine:

image

I think I can close this, since the error is expected to be thrown, even though that on SQLite Android is silent. Let me know what do you think.

davidmartos96 commented 4 years ago

@jaripekkala Also, I found this, which seems related https://stackoverflow.com/questions/26106237/sqlitedatabase-nested-transaction-and-workaround

jaripekkala commented 4 years ago

Thank you for jumping on to this so quickly!

That is correct, this is something specific handling that sqflite or its dependencies does. There is a note about this in the sqflite README.md

Due to the way transaction works in SQLite (threads), concurrent read and write transaction are not supported. All calls are currently synchronized and transactions block are exclusive. I thought that a basic way to support concurrent access is to open a database multiple times but it only works on iOS as Android reuses the same database object. I also thought a native thread could be a potential future solution however on android accessing the database in another thread is blocked while in a transaction...

It may be acceptable that this package shares the same API with sqflite package, however they use different underlaying native libraries that have their own behaviour on some cases.

With that said, it would be amazing to have this be just a wrapper for the main package with exact same behaviour. Easy drop-in replacement when you need encryption without changing code.

davidmartos96 commented 4 years ago

I'm not sure about trying yo make it exactly the same in that regard. I have never stumbled upon the need of using nested transactions. Also, if this is an Android specific "feature" it world fail on iOS as it does with the native libraries on Linux, so you would need 2 diferentes codes, one with nested transactions and another without them. In any case, thanks for bringing this difference up, it's interesting to know.

jaripekkala commented 4 years ago

Actually, nested transactions are working the same on Android and iOS with sqflite package.

The example was oversimplified, in order to explain the issue. In our case, we are inserting event records (at random times) to a table that happens already have a transaction ongoing based on other logic.

main() async {
  Timer(Duration(seconds: 1), (t) {
    addRecord();
  });

  // on button click or something
  await addRecord();
}

addRecord() async {
  await database.rawQuery('BEGIN');
  await database.insert('foos', {'bar': 'biz'});
  await database.rawQuery('COMMIT');
}

That surely can be implemented in other ways so no problemo.

Anyway, thank you again for your time! 🥇