db icon indicating copy to clipboard operation
db copied to clipboard

No usage of `AbstractCommand::requireTransaction()`

Open Tigrov opened this issue 7 months ago • 3 comments

Protected AbstractCommand::requireTransaction() method should be public or removed.

If it will be public possible usage

$command->requireTransaction()->execute();

Or if not use requireTransaction()

$db->transaction(fn () => $command->execute());

Tigrov avatar Nov 22 '23 08:11 Tigrov

Actually, this method don't allow mark command for run in transaction with default isolation level. ->requireTransaction() don't run command in transaction.

CommandInterface do not contain requireTransaction method. Suggest remove requireTransaction and isolationLevel from implementations. For run in transcation can be used $db->transcation(). If there is a request for requireTransaction() then we can add it in next major version to CommandInterface.

vjik avatar Nov 22 '23 13:11 vjik

This method has one advantage. If the logger or profiler is configured on the same db connection, the logger and profiler entries will be rolled back along with the failed query if use $db->transaction(fn () => $command->execute());

This method allows you to complete a transaction for a command only.

But I would not use a logger and profiler in production or need to configure them on different storage / db connection. I'm more inclined that this method is redundant, at least until today it was not used and there were no problems (in yii2 the same).

Tigrov avatar Nov 23 '23 05:11 Tigrov

@darkdef @terabytesoftw What do you think? Can we remove requireTransaction() method?

vjik avatar Nov 23 '23 06:11 vjik