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

[9.x] Alternative database name in Postgres DSN, allow pgbouncer aliased databases to continue working on 9.x #43542

Conversation

AlbinoDrought
Copy link
Contributor

I encountered this issue that prevented migrations from running after upgrading to Laravel 9: #43536 (php artisan migrate fails after attempting to create migrations table that already exists)

It is caused by my (fringe?) pgbouncer setup + the introduction of the table_catalog parameter into queries in 9.x via #35530

Here's a repo to reproduce the issue: https://github.com/AlbinoDrought/l9-pgsql-pgbouncer-alias

When using pgbouncer, the "database" name used to connect is just a pool name - pgbouncer can connect to a differently-named database behind the scenes. As an example, a sample_app database could have a sample_app_background pool and a sample_app_web pool. In Laravel you would specify ['database' => 'sample_app_background'] or ['database' => 'sample_app_web'] to connect.

In Laravel <9, the database config var does not appear to be referenced by the PostgresConnector after DSN generation. The above pgbouncer setup works transparently.

In Laravel 9, the database config var is referenced during PostgresConnector DSN generation, hasTable, and getColumnListing. The above pgbouncer setup now causes issues.

If we're connected to the sample_app_web pool, hasTable and getColumnListing query information_schema about the sample_app_web database, but that database doesn't exist. This causes hasTable to always return false which is problematic during migrations. We want hasTable and getColumListing to query about the sample_app database instead.

I don't know what getColumnListing is used for (something about guarded attributes?) but it also references the database config var, possibly outside of migrations.

This proposed fix allows specifying a different database name used only during PostgresConnector DSN generation. The pool name should be specified here. I currently chose the config name database_connect but don't really like it. Users would specify a config like ['database' => 'sample_app', 'database_connect' => 'sample_app_web'] to continue using a pgbouncer setup like above.

I'm not sure how common this kind of setup is. If this change isn't wanted upstream, it can be accomplished by overriding PostgresConnector in a third-party package (this is what I'm currently doing).

Thanks!

…name than is used for information_schema queries

See laravel#43536
@AlbinoDrought AlbinoDrought changed the title Alternative database name in Postgres DSN, allow pgbouncer aliased databases to continue working on 9.x [9.x] Alternative database name in Postgres DSN, allow pgbouncer aliased databases to continue working on 9.x Aug 4, 2022
@taylorotwell taylorotwell merged commit dc9af65 into laravel:9.x Aug 4, 2022
Ken-vdE pushed a commit to Ken-vdE/framework that referenced this pull request Aug 9, 2022
…sed databases to continue working on 9.x (laravel#43542)

* feat: allow creating a Postgres DSN string with a different database name than is used for information_schema queries

See laravel#43536

* formatting

Co-authored-by: Taylor Otwell <taylor@laravel.com>
nikosv added a commit to nikosv/laravel-backup that referenced this pull request Feb 3, 2023
I am adding $dbConfig['connect_via_database'] in setDbName() as the package doesn't work with pgbouncer the same way it was described here:
laravel/framework#43542
freekmurze pushed a commit to spatie/laravel-backup that referenced this pull request Feb 3, 2023
I am adding $dbConfig['connect_via_database'] in setDbName() as the package doesn't work with pgbouncer the same way it was described here:
laravel/framework#43542
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

2 participants