Laravel 10: DB::transactionLevel() shows incorrect level, if DDL operation was done

[mrckzgl]

Laravel Version

10.28.0

PHP Version

8.2.11

Database Driver & Version

10.10.2-MariaDB-1:10.10.2+maria~ubu220

Description

We encounter that DB::transactionLevel() shows an incorrect level after a DDL operation was done, which does commit implicitly. Consider this code snippet which is the current behaviour:

Log::debug(DB::transactionLevel()); // 0
DB::beginTransaction();
Log::debug(DB::transactionLevel()); // 1
DB::commit();
Log::debug(DB::transactionLevel()); // 0

DB::beginTransaction();
Log::debug(DB::transactionLevel()); // 1
// some DDL operation which closes the transaction
Log::debug(DB::transactionLevel()); // 1

This is problematic on different levels. First, getting the correct transaction level is important to be able to close all pending transactions. Second, due to a current change (see: #35380), DB::commit() and DB::rollback() both throw an exception if no active transaction is present. That means after beginTransaction and a DDL statement, this code:

if (DB::transactionLevel() > 0) {
  DB::commit();
}

will fail.

Steps To Reproduce

DB::beginTransaction();
Schema::dropIfExists('test');
Schema::create('test', function (Blueprint $table) {
    $table->id();
});
if (DB::transactionLevel() > 0) {
  DB::commit();
}

I just confirmed this behaviour in a minimal example, happy to provide a repo if needed.

[crynobone]

Hi there,

We encounter that DB::transactionLevel() shows an incorrect level after a DDL operation was done, which does commit implicitly

I don't believe commit should be done explicitly but you should be able to override db.transactions and handle it based on your requirements.

framework/src/Illuminate/Database/DatabaseServiceProvider.php

Lines 77 to 79 in a47df68

	$this->app->singleton('db.transactions', function ($app) {
	return new DatabaseTransactionsManager;
	});

The current db.transactions is well-tested and working as intended.

[crynobone]

Also, this is documented here: https://laravel.com/docs/10.x/database#implicit-commits-in-transactions

[mrckzgl]

@crynobone Thank you very much for the documentation link, but I am unsure if there is a misunderstanding here. That documentation explicitly only applies to the statement and unprepared methods of the DB facade:

When using the DB facade's statement and unprepared methods within transactions you must be careful to avoid statements that cause implicit commits.

But the problem here occurs even if these methods are not used, but if Laravel methods like Schema::XXX are used. Please, I invite you to have again a look at the Step to Reproduce. I can understand that it is difficult (but probably not impossible) to track transaction level for raw SQL statements done by the user. However, in the example above we only use Laravel database abstraction methods, so the framework should very well be able to track the transaction level correctly internally there, and frankly this is what I would expect.

I don't believe commit should be done explicitly

I am not sure if I understand you correctly here. If we start a transaction manually via DB::beginTransaction() we have to explicitly commit at the end. This consumption of the Laravel API is officially documented here: https://laravel.com/docs/10.x/database#manually-using-transactions
I assume you refer to the other case, where transactions are run inside the callback of DB::transaction? I just reproduced, however that also in this case the internal transaction level is not updated correctly on a DDL operation done using Schema::XXXmethods. This example:

        DB::transaction(function () {
            Schema::dropIfExists('test');
            Schema::create('test', function (Blueprint $table) {
                $table->id();
            });
            $this->printTransactionLevel();
        });

Also fails with an "There is no active transaction" PDOException:

[2023-10-19 20:39:11] local.ERROR: There is no active transaction {"view":{"view":"/var/www/laravel_docker/resources/views/index.blade.php","data":{"errors":"<pre class=sf-dump id=sf-dump-142486146 data-indent-pad=\"  \"><span class=sf-dump-note>Illuminate\\Support\\ViewErrorBag</span> {<a class=sf-dump-ref>#277</a><samp data-depth=1 class=sf-dump-expanded>
  #<span class=sf-dump-protected title=\"Protected property\">bags</span>: []
</samp>}
</pre><script>Sfdump(\"sf-dump-142486146\", {\"maxDepth\":3,\"maxStringLength\":160})</script>
"}},"exception":"[object] (Spatie\\LaravelIgnition\\Exceptions\\ViewException(code: 0): There is no active transaction at /var/www/laravel_docker/vendor/laravel/framework/src/Illuminate/Database/Concerns/ManagesTransactions.php:194)
[stacktrace]
#0 /var/www/laravel_docker/vendor/laravel/framework/src/Illuminate/Database/Concerns/ManagesTransactions.php(194): PDO->commit()
#1 /var/www/laravel_docker/vendor/laravel/framework/src/Illuminate/Database/DatabaseManager.php(469): Illuminate\\Database\\Connection->commit()
#2 /var/www/laravel_docker/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(353): Illuminate\\Database\\DatabaseManager->__call('commit', Array)
#3 /var/www/laravel_docker/app/Http/Controllers/IndexController.php(27): Illuminate\\Support\\Facades\\Facade::__callStatic('commit', Array)
#4 /var/www/laravel_docker/resources/views/index.blade.php(3): App\\Http\\Controllers\\IndexController->run()

This is the same exception which occurs in the example code provided in the Steps to Reprocude section.
The transaction level at the end of the callback also is 1, instead of 0. This is an indication that the Schema::dropIfExists and Schema::create method do not correctly update the Laravel internal transaction level variable.

[crynobone]

If you refer to link from documentation (MySQL documentation) implicit commit include command used in Schema https://dev.mysql.com/doc/refman/8.0/en/implicit-commit.html

…

On Fri, 20 Oct 2023, 4:44 pm mrckzgl, ***@***.***> wrote: @crynobone <https://github.com/crynobone> Thank you very much for the documentation link, but I am unsure if there is a misunderstanding here. That documentation explicitly only applies to the statement and unprepared methods of the DB facade: When using the DB facade's statement and unprepared methods within transactions you must be careful to avoid statements that cause implicit commits <https://dev.mysql.com/doc/refman/8.0/en/implicit-commit.html>. But the problem here occurs even if these methods are not used, but if Laravel methods like Schema::XXX are used. Please, I invite you to have again a look at the Step to Reproduce. I can understand that it is difficult (but probably not impossible) to track transaction level for raw SQL statements done by the user. However, in the example above we only use Laravel database abstraction methods, so the framework should very well be able to track the transaction level correctly internally there, and frankly this is what I would expect. I don't believe commit should be done explicitly I am not sure if I understand you correctly here. If we start a transaction manually via DB::beginTransaction() we have to explicitly commit at the end. This consumption of the Laravel API is officially documented here: https://laravel.com/docs/10.x/database#manually-using-transactions I assume you refer to the other case, where transactions are run inside the callback of DB::transaction? I just reproduced, however that also in this case the internal transaction level is not updated correctly on a DDL operation done using Schema::XXXmethods. This example: DB::transaction(function () { Schema::dropIfExists('test'); Schema::create('test', function (Blueprint $table) { $table->id(); }); $this->printTransactionLevel(); }); Also fails with an "There is no active transaction" PDOException: [2023-10-19 20:39:11] local.ERROR: There is no active transaction {"view":{"view":"/var/www/laravel_docker/resources/views/index.blade.php","data":{"errors":"<pre class=sf-dump id=sf-dump-142486146 data-indent-pad=\" \"><span class=sf-dump-note>Illuminate\\Support\\ViewErrorBag</span> {<a class=sf-dump-ref>#277</a><samp data-depth=1 class=sf-dump-expanded> #<span class=sf-dump-protected title=\"Protected property\">bags</span>: [] </samp>} </pre><script>Sfdump(\"sf-dump-142486146\", {\"maxDepth\":3,\"maxStringLength\":160})</script> "}},"exception":"[object] (Spatie\\LaravelIgnition\\Exceptions\\ViewException(code: 0): There is no active transaction at /var/www/laravel_docker/vendor/laravel/framework/src/Illuminate/Database/Concerns/ManagesTransactions.php:194) [stacktrace] #0 /var/www/laravel_docker/vendor/laravel/framework/src/Illuminate/Database/Concerns/ManagesTransactions.php(194): PDO->commit() #1 /var/www/laravel_docker/vendor/laravel/framework/src/Illuminate/Database/DatabaseManager.php(469): Illuminate\\Database\\Connection->commit() #2 /var/www/laravel_docker/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(353): Illuminate\\Database\\DatabaseManager->__call('commit', Array) #3 /var/www/laravel_docker/app/Http/Controllers/IndexController.php(27): Illuminate\\Support\\Facades\\Facade::__callStatic('commit', Array) #4 /var/www/laravel_docker/resources/views/index.blade.php(3): App\\Http\\Controllers\\IndexController->run() This is the same exception which occurs in the example code provided in the Steps to Reprocude section. The transaction level at the end of the callback also is 1, instead of 0. This is an indication that the Schema::dropIfExists and Schema::create method do not correctly update the Laravel internal transaction level variable. — Reply to this email directly, view it on GitHub <#48777 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABKHJSXSO3BL7SPGT27EFTYAI2YDAVCNFSM6AAAAAA6HZ6K3WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZSGMZDQMRWGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[mrckzgl]

@crynobone Yes, but that documentation is from dev.mysql.com. It does not apply to Laravel. As stated, the documentation for Laravel API you referred to only applies to DB::statement and DB::unprepared. It does not apply to other Laravel methods. I invite you to please completely and carefully read my last comment with all the points I presented. A response after just one minute from yours via email indicates that this might not have been done.

[crynobone]

If you have any suggestion to override implicit commit imposed by MySQL and PDO feel free to send in a PR.

[mrckzgl]

Frankly, this is not my task here. I am here to make you, the maintainers of Laravel, aware of a bug in your code, or well if you want to state that as limitation you do not want to address, as a problem in your documentation. I would assume, that you as maintainers have an interest in quality code, and / or quality documentation, but I might be wrong. If this bug turns out to be a limitation you do not want to address, I would kindly ask you to update your documentation so that it correctly reflects the problem I posted initially. Trying to argue that the documentation applies here, if it explicitly does not, is not very helpful or friendly to be honest.

So, to make it more direct: If you do not want to implement DB:transactionLevel() such that it tracks all transaction related state changes Laravel does, I would kindly ask you to include this in the documentation you already pointed to here: https://laravel.com/docs/10.x/database#implicit-commits-in-transactions and also in the actual API reference here: https://laravel.com/api/10.x/Illuminate/Database/ConnectionInterface.html#method_transactionLevel and here: https://laravel.com/api/10.x/Illuminate/Database/Concerns/ManagesTransactions.html#method_transactionLevel

thank you very much

[github-actions]

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

[mrckzgl]

@crynobone Thank you for reopening again. Just another idea to handle DDL statements: Why not open a transaction inside Schema::XXX after any possible implicit commit and of course only if a transaction was active beforehand? This way, transactionLevel stays the same. Of course, this behaviour then should ideally be documented for the respective Schema:XXX methods.

best

[ionutantohi]

I wrote a comment on antoher issue, about having the posibility to resolve a separate db connection when the user wants to issue implict commit statements. Not sure if this is possible/feasible though.

#35380 (comment)

DB::beginTransaction();

DB::isolatedConnection('mysql')->statement('CREATE TABLE ...')

DB::commit();

[tpetry]

Yes, but that documentation is from dev.mysql.com. It does not apply to Laravel. As stated, the documentation for Laravel API you referred to only applies to DB::statement and DB::unprepared.

The Laravel docs share a link to the MySQL resource listing all database command making an implicit commit. So yes, this is clearly documented and everything works as expected and there is no way Laravel can do anything different. It is how MySQL behaves.

The only possible change would be listing migrations as another reason for imilicit commits additionally to the DB::unprepared and DB::statement method.