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

[11.x] Fix inconsistent database parsing on PostgreSQL #49148

Merged
merged 5 commits into from
Dec 13, 2023

Conversation

hafezdivandari
Copy link
Contributor

@hafezdivandari hafezdivandari commented Nov 27, 2023

As explained on #49020, Schema::hasTable() uses Schema::getTables() on all database drivers except PostgreSQL, because Schema::hasTable() on PostgreSQL is the only function on the schema/grammar/builder that accepts 3-parts references like Schema::hasTable('database.schema.table'). This is an inconsistent behavior, as other functions and non of the other database drivers support this syntax.

This PR fixes this by throwing an exception if 3-parts reference is mistakenly passed to Schema::hasTable() and also changes Schema::hasTables() on PostgreSQL to use Schema::getTables() just like other databases.

Note: Passing 2-parts reference or just table name to these functions on PostgreSQL works as expected as before:

Schema::hasTable('schema.table');
Schema::getColumns('schema.table');
Schema::getIndexes('schema.table');
Schema::getForeignKeys('schema.table');
Schema::hasTable('table'); // uses default schema
Schema::getColumns('table'); // uses default schema
Schema::getIndexes('table'); // uses default schema
Schema::getForeignKeys('table'); // uses default schema

Upgrade Guide

Likelihood Of Impact: Very Low

The Schema::hasTable() on PostgreSQL database doesn't accept declaring database name by passing a 3-parts reference anymore and will throw an exception. Therefore, if you were using 3-parts reference on these methods you may use connection() to declare the database instead:

- Schema::hasTable('database.schema.table');
+ Schema::connection('database')->hasTable('schema.table');

@hafezdivandari hafezdivandari marked this pull request as ready for review November 27, 2023 20:20
@hafezdivandari hafezdivandari marked this pull request as draft November 28, 2023 11:16
@driesvints
Copy link
Member

driesvints commented Dec 11, 2023

@hafezdivandari are you still working on this?

@hafezdivandari
Copy link
Contributor Author

@driesvints yes, I'm wating for #49264 to be merged:

https://github.com/laravel/framework/pull/49264/files#diff-9fc77e1263d98fe08a38be1f5d172d2497320915fa8b8148e1823052eb33f403R231

@hafezdivandari
Copy link
Contributor Author

@driesvints would you please merge '10.x' into master? cc @crynobone

@driesvints
Copy link
Member

@hafezdivandari will do as soon as all of the fixes PRs that are open are merged.

@hafezdivandari hafezdivandari marked this pull request as ready for review December 13, 2023 13:30
@driesvints
Copy link
Member

@hafezdivandari done now

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.

3 participants