zim32 / mysql.dart

MySQL client for Dart written in Dart
BSD 3-Clause "New" or "Revised" License
64 stars 17 forks source link

fix inTransaction flg handle #17

Closed yamarkz closed 2 years ago

yamarkz commented 2 years ago

I use this package for private projects. When I tried to use transaction, encountered a control I did not expect.

Isn't COMMIT after _inTransaction = false; ...? It is possible that the intent of the control is not properly understood.

Thanks for the great package. I support the long term development of this package.

The following expressions are also possible.

try {
  final result = await callback(this);
  await execute("COMMIT");
  return result;
} catch {
  await execute("ROLLBACK");
  rethrow;
} finally {
  _inTransaction = false;
}
zim32 commented 2 years ago

Hi. Thanks for support. I do not understand whats the problem? Can you explain?

yamarkz commented 2 years ago

@zim32

May be using it incorrectly. I want to run multiple transactions in one process. Existing implementations allow it to be executed only once.

Future<void> main() async {
  final _connection = await MySQLConnection.createConnection();
  final items = []; // about domain object
  for (final item in items) {
    final sql = ‘’'‘’';
    await _connection.transactional((conn) async {
      final statement = await conn.prepare(sql);
      final result = await statement.execute(<dynamic>[item.name, item.price]);
    });
  }
}

This example results in an error of Already in transaction. However, it succeeds once. Is 1 connection 1 transaction the correct design?

zim32 commented 2 years ago

Hm. Finally block should be executed and inTransaction must be set to false. I thinks to be sure it works correctly, we must rewrite this piece of code without finally

zim32 commented 2 years ago

Can you remove finally and add isTransaction = false after rollback?

yamarkz commented 2 years ago

@zim32

Thanks to comments.

Can you remove finally and add isTransaction = false after rollback?

It has already become.

    try {
      final result = await callback(this);
      await execute("COMMIT");
      return result;
    } catch (e) {
      await execute("ROLLBACK");
      _inTransaction = false;
      rethrow;
    }

What I would like to confirm here is that there should be an _inTransaction = false; after COMMIT.

zim32 commented 2 years ago

Yes. There must be two inTransaction = false. After commit and after rollback

zim32 commented 2 years ago

And after your changes, please confirm it is working in your code

yamarkz commented 2 years ago

@zim32

I confirmed the code works and test case is added. To check executing in local. Add // _inTransaction = false; after await execute("COMMIT");

zim32 commented 2 years ago

Looks fine. Merging

zim32 commented 2 years ago

Thanks for contributing 👍