Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] Fix database migrations $connection property #41161

Merged

Conversation

derekmd
Copy link
Contributor

@derekmd derekmd commented Feb 22, 2022

Fixes #41153 #40097 #38777 #36596 #31060

The docs show the below migration will be run on the named database connection:

return new class extends Migration
{
    protected $connection = 'pgsql';

    function up()
    {
        Schema::create('users', function (Blueprint $table) {
            // ...
        });
    }
}

However Schema facade-built queries will run on the app's default database connection or the --database command line option's named connection. The migration also requires Schema::connection('pgsql'):

return new class extends Migration
{
    protected $connection = 'pgsql';

    function up()
    {
        Schema::connection('pgsql')->create('users', function (Blueprint $table) {
            // ...
        });
    }
}

This fix allows the first example to work as documented for the up() & down() methods. The selected connection priority is:

  1. Schema::connection()
  2. migration class property $connection
  3. php artisan migrate* command line option --database
  4. config('database.default')

I also added tests for the Migrator@usingConnection() method added in 7.x for the migration commands.

Despite being documented as allowing `Schema` queries to run on the
given secondary connection, `Schema::connection()` must also be
explicitly called to avoid `up()` and `down()` queries to be run on the
default database connection.

Instead this property is only used by the migrator when wrapping
`DB::transaction()` & `DB::pretend()`. The queries still run on the
default database connection (or the command option --database.)

This change makes queries run using that $connection while not affecting
the migrations repository. i.e., the connection for the `migrations`
history table. If another `Schema::connection()` is called during `up()`
or `down()`, that will be used instead of `$connection`.

$connection also overrides MigrateCommand option '--database'. A
breaking change is required to switch to the opposite behavior.
@taylorotwell taylorotwell merged commit 03e3a80 into laravel:8.x Feb 22, 2022
@derekmd derekmd deleted the fix-migrations-connection-property branch February 22, 2022 19:37
@rawmarius
Copy link

Thank you so much for the fix! It would be nice if the priority would be added to the documentation.
Regards!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants