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

Add support for naming additional database connections #54162

Closed

Conversation

JanneDeVos
Copy link
Contributor

This pull request introduces the ability to name multiple database connections, improving the developer experience when working with multiple databases. By providing meaningful names for each connection, it becomes more intuitive to switch between different databases.

This feature adds an abstraction layer that decouples the database implementation from your code by allowing you to define multiple named connections in the database configuration:

/*
|--------------------------------------------------------------------------
| Default Database Connection Name
|--------------------------------------------------------------------------
|
| Here you may specify which of the database connections below you wish
| to use as your default connection for database operations. This is
| the connection which will be utilized unless another connection
| is explicitly specified when you execute a query / statement.
|
*/

'default' => env('DB_CONNECTION', 'sqlite'),
'other' => env('DB_CONNECTION_OTHER', 'mysql'),

Models can then specify which connection to use via the $connection property:

class Order extends Model
{
    protected $connection = 'other'; // Equals to mysql

Or you can use the connection directly in queries:

$orders = DB::connection('other')->table('orders')->get();

Copy link

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

@shaedrich
Copy link
Contributor

shaedrich commented Jan 11, 2025

What's wrong with naming additional connections under database.connections?

https://github.com/laravel/laravel/blob/f9bedb320cac1c77d78052c1355ea37709cd464b/config/database.php#L32-L113

There is already is a $connection property that can take connection names:

/**
* The connection name for the model.
*
* @var string|null
*/
protected $connection;

@JanneDeVos
Copy link
Contributor Author

JanneDeVos commented Jan 12, 2025

@shaedrich This pattern is particularly useful when managing database connections across different environments. For example, when using a Redshift Datawarehouse in production but local and tests with a replica of the DWH in Sqlite.

Additionally, I find it helpful to maintain a clear distinction between the functional naming (what the database represents in the business context) and the technical naming (the database type and provider).

/*
|--------------------------------------------------------------------------
| Default Database Connection Name
|--------------------------------------------------------------------------
|
| Here you may specify which of the database connections below you wish
| to use as your default connection for database operations. This is
| the connection which will be utilized unless another connection
| is explicitly specified when you execute a query / statement.
|
*/

'default' => env('DB_CONNECTION', 'mysql'),
'dwh' => env('DB_CONNECTION_DWH', 'sqlite'),

Using name dwh instead of using connection redshift from database.connections.redshift

class Order extends Model
{
    protected $connection = 'dwh';

@NickSdot
Copy link
Contributor

Using name dwh instead of using connection redshift from database.connections.redshift

You can name the connections under database.connections however you want (eg dwh) and use it in the way you mentioned protected $connection = 'dwh';.

It's not at all bound to the database type and provider, and does exactly what you want.

@shaedrich
Copy link
Contributor

Additionally, to what @NickSdot said: The naming of the connections is a little misleading or redundant. They are named after their database types but they didn't have to be. It would suffice, supplying this as a config option.

Also, what you suggest adding can be added to your own code without changing the framework fairly easy.

@rodrigopedra
Copy link
Contributor

I've been using named connections for years, not sure this PR adds that one cannot already do.

<?php // config/database.php

return [
    'default' => \env('DB_CONNECTION', 'mysql'),

    'connections' => [
        'reports' => [
            'driver' => 'sqlite',
            // ...
        ],

        'mysql' => [
            'driver' => 'mysql',
            // ...
        ],

        'logs' => [
            'driver' => 'mysql',
            // ...
        ],
    ],

    // ...
];

Usage is mostly the same as it is in OP's examples, both on Models and on DB::connection().

The database.default connection name is used when no connection is specified.

Maybe there is a confusion regarding connection names and database drivers?

@rodrigopedra
Copy link
Contributor

rodrigopedra commented Jan 12, 2025

Regarding switching database drivers depending on the environment, that can be done through environment variables, or by using a match clause:

<?php // config/database.php

return [
    'default' => \env('DB_CONNECTION', 'mysql'),

    'connections' => [
        'dwh' => match (env('APP_ENV', 'local')) {
            'production' => [
                'driver' => 'redshift',
                // ... additional connection parameters
            ],
            default => [
                'driver' => 'sqlite',
                // ... additional connection parameters
            ],
        },

        // ... other connections
    ],
];

Then you can just call DB::connection('dwh') or set a model's $connection property to dwh and the appropriate driver will be used depending on the environment.

@JanneDeVos
Copy link
Contributor Author

I've been using the setup from my initial example for years with databases, broadcasting, and filesystems. The extra abstraction layer often gives me more flexibility when I know we'll need migrations or refactors in the future. Additionally, it provides the ability to use a different database technology locally (like sqlite), while still being able to quickly connect to a staging database if needed.

Regarding switching database drivers depending on the environment, that can be done through environment variables, or by using a match clause:

I can also solve this problem using @rodrigopedra approach, I'll definitely experiment with that.

Currently, I often use a Trait with a nested config function and the result is the same. I personally find it a less elegant solution, but it works fine. So if no one sees value in this, I'm willing to close the PR.

@ejunker
Copy link
Contributor

ejunker commented Jan 13, 2025

As someone who maintains an "Enterprise" Laravel app it is common to use multiple database connections as shown in the example with a dwh data warehouse connection. I hope this change gets merged or some variation of it.

I don't think it was mentioned but where this is really helpful is with testing. The application code may be like: DB::connection('dwh')->table('orders')->get() and you want to swap that to be a local database when your tests run. With a change like this you can just set the DB_CONNECTION_DWH env var in your phpunit.xml and it will switch to your test database when tests run.

@shaedrich
Copy link
Contributor

@ejunker Please read the conversation. This is already possible.

@ejunker
Copy link
Contributor

ejunker commented Jan 13, 2025

@shaedrich are you referring to the example that has a match() statement in the config? That may work but I feel like the solution proposed in this PR is much more elegant and regardless I feel the framework should provide better documentation on how to handle multiple database connections. This change makes it feel more like "first class" support for multiple database connections.

@shaedrich
Copy link
Contributor

@ejunker Not just that. All the examples. There should be at least one that is fitting.

@rodrigopedra
Copy link
Contributor

I am not against this patch, just have a hard time reasoning over its benefits.

Also, I found the wording used to "sell" this feature misleading, as this was not possible before ("Models can then specify which connection..."). But that might be a misinterpretation from my English skills, please take no offense.

Specially as it basically adds another indirection, to read a config value that points to another config value.

As @ejunker, I also maintain some "Enterprise" Laravel apps for a living, and all but one make use of different connections for several reasons.

I, personally, find the use of the match (and before it was available, of hash maps) makes which configuration is used more explicit.

As a workaround, if one prefers this indirection over being explicit on their configuration, one can already use something like this:

DB::connection(config('database.dwh'))->table('orders')->get();

config('database.dwh') will resolve to whatever database connection name one added a custom key to their config/database.php file, and choose the right connection from their database.connection array.

@ejunker
Copy link
Contributor

ejunker commented Jan 13, 2025

@rodrigopedra I think this is more about making sure Laravel has good DX and ease of use rather than what it technically possible. Sure, for now I have to do DB::connection(config('database.dwh)) and it works but it is not a good developer experience and maybe not obvious to other developers that need to use multiple connections. I love that whenever I need to do something, Laravel has an established answer, but when it comes to multiple database connections, I feel this is an area where Laravel could improve.

@JanneDeVos JanneDeVos marked this pull request as ready for review January 13, 2025 20:55
@JanneDeVos
Copy link
Contributor Author

JanneDeVos commented Jan 13, 2025

Also, I found the wording used to "sell" this feature misleading, as this was not possible before ("Models can then specify which connection..."). But that might be a misinterpretation from my English skills, please take no offense.

This might also be a result of my non-native English skills. 😉

The intention is indeed not to make it sound like it’s about adding additional connections, but rather about introducing the extra abstraction layer over the database connections in an elegant way.

@rodrigopedra
Copy link
Contributor

@ejunker, the workaround I proposed was due to the comment where you found the proposed solution more "elegant" than using match in the database configuration file.

As I said before, I am not against this PR, and actually the changes are so little and localized, that, if you all think it has an appeal to it, I am all for it.

I just don't share the same view on Laravel not providing "'first class' support for multiple database connections." or that the current support "is not a good developer experience and maybe not obvious to other developers that need to use multiple connections."

It is even mentioned in the docs:

I also think it is an additional abstraction over connection and database drivers. I've seen much more confusion when using DB::extend() to add a custom database driver, as out of the box the connections are named the same as their driver (for which I don't have a better suggestion).

But again, if you all believe this is worth adding, I don't see a problem.

@shaedrich
Copy link
Contributor

And if one needs aliases, this can be as easy as this:

<?php // config/database.php

$dwhConnectionConfig = [
    'driver' => 'mysql',
    // ...
];

return [
    'default' => \env('DB_CONNECTION', 'mysql'),

    'connections' => [
        'reports' => [
            'driver' => 'sqlite',
            // ...
        ],

        'mysql' => $dwhConnectionConfig,
        'dwh' => $dwhConnectionConfig,
        'my-other-connection' => $dwhConnectionConfig,

        'logs' => [
            'driver' => 'mysql',
            // ...
        ],
    ],

    // ...
];

@ejunker
Copy link
Contributor

ejunker commented Jan 14, 2025

@rodrigopedra I am mostly concerned about the improvements to being able to easily swap the database used during testing. If you look at the default phpunit.xml for Laravel it has a commented out line that you can use to set the DB_CONNECTION to use the sqlite.

<!-- <env name="DB_CONNECTION" value="sqlite"/> -->

This allows you to easily swap to using the sqlite database for testing. But what if you have multiple connections? It would be nice to continue using the same pattern that Laravel introduced by doing something like this:

<env name="DB_CONNECTION" value="sqlite"/>
<env name="DB_CONNECTION_DWH" value="sqlite"/>

This would configure it to use sqlite for both connections. Now, you can get that to work by doing DB::connect(config('database.dwh')) but it would be much nicer if it could simply be DB::connection('dwh')

Yes, it is a layer of indirection but so are Facades. This is almost like a Facade for a database connection and just like Facades make it easy to swap dependencies during testing, so does this.

It seems that other developers are also running into this issue and having to invent their own solutions such as in this article: Database Unit Testing in Laravel with Multiple Connections

@taylorotwell
Copy link
Member

Frankly I don't understand what this is allowing that you can't already do.

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.

6 participants