cockroachdb-laravel icon indicating copy to clipboard operation
cockroachdb-laravel copied to clipboard

[Bug]: Transactions fail with autocommit_before_ddl (default for v25)

Open LegendEffects opened this issue 7 months ago • 2 comments

What happened?

CockroachDB have enabled autocommit_before_ddl default to true as of V25, this results in transactions getting committed before they're told and results in PDO erroring with "No active transaction".

I'm not a CockroachDB expert and we're currently assessing whether we'll continue to use it or just use trusty old Postgres, for now I've made a workaround by configuring the session variable on the connection before a transaction by registering events in the AppServiceProvider.

private function setupCockroachDBPreventAutoCommitBeforeDDL(): void
{
    $configured = [];

    Event::listen(ConnectionEstablished::class, function (ConnectionEstablished $event) use (&$configured) {
        if ($event->connection->getDriverName() !== "crdb") {
            return;
        }

        $configured[$event->connectionName] = false;
    });

    Event::listen(TransactionBeginning::class, function (TransactionBeginning $event) use (&$configured) {
        if ($event->connection->getDriverName() !== "crdb" || ($configured[$event->connectionName] ?? false)) {
            return;
        }

        $event->connection->statement("SET SESSION autocommit_before_ddl = false");
        $configured[$event->connectionName] = true;
    });
}

How to reproduce the bug

  • Run CRDB with autocommit_before_ddl set to true
  • Attempt to run migrations or anything else that starts a transaction
  • "No active transaction error"

Package Version

1.4.0

PHP Version

8.3.16

Laravel Version

10.48.22

CockroachDB Version

v25.1

Which operating systems does with happen with?

macOS

Notes

See: https://github.com/cockroachdb/cockroach/pull/140156

LegendEffects avatar May 21 '25 22:05 LegendEffects

I've done similar with a listener for only migrations by using the \Illuminate\Database\Events\MigrationsStarted::class event.

If I remember correctly, this is only an issue for migrations that make multiple schema changes at the same time. For these, you could also set the following:

public $withinTransaction = false;

Really, we shouldn't be using explicit transactions for migrations https://www.cockroachlabs.com/docs/v25.1/online-schema-changes#schema-changes-within-transactions so I don't think autocommit_before_ddl = false should be the default in this package. Each developer should understand how CRDB handles schema changes and adjust as needed for their specific use case.

wsamoht avatar Jun 19 '25 15:06 wsamoht

I added some code to make it support autocommit_before_ddl in database config https://github.com/vuthaihoc/crdb2025/commit/ccb8b16762872bdaeba5037e3310ae90f3f1aba2

And set to off when migrating only

// AppServiceProvider::boot()
if ($this->app->runningInConsole()) {
      Event::listen([
          MigrationStarted::class,
      ], function ($event) use (&$set_autocommit_before_ddl) {
          $set_autocommit_before_ddl ??= false;
          if (!$set_autocommit_before_ddl) {
              $connection_name = $event->migration->getConnection() ?? \DB::getDefaultConnection();
              \Config::set('database.connections.' . $connection_name . '.autocommit_before_ddl', 'off');
              \DB::purge($connection_name);
              \DB::connection($connection_name);
              $set_autocommit_before_ddl = true;
          }
      });
}

vuthaihoc avatar Jul 06 '25 03:07 vuthaihoc