-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
[5.2] Fix postgres Schema::hasTable(); #13008
Conversation
@@ -28,7 +28,7 @@ class PostgresGrammar extends Grammar | |||
*/ | |||
public function compileTableExists() | |||
{ | |||
return 'select * from information_schema.tables where table_name = ?'; | |||
return 'select * from information_schema.tables where table_schema = ? and table_name = ?'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GrahamCampbell changing the sql considered breaking? It's a public method. Should I submit to master?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably fine.
I'm a little confused because "database" is not the same as "schema" in Postgres? |
Yes, this fix searches for the existence of a table in a schema regardless of the database. When you compare Postgres to MySQL, if I'm on MySQL and would like to create a new DB for purpose (x) it's equivalent to creating a new schema in Postgres for the same purpose (x) not a new database. Maybe this PR is a bit opinionated, if you'd like I can close it and seek a more general solution that allows searching for a table with regards to the schema (and) the database. Or maybe change the syntax of What do you think? |
@taylorotwell: can this fix also be merged into the laravel 5.1 branch |
Please send a PR.
|
I've backported this to 5.1 for you. |
@GrahamCampbell I think that has caused an issue for 5.1 https://github.com/laravel/framework/blob/5.1/src/Illuminate/Database/Schema/Builder.php#L55 Still only passes in 1 parameter to the statement, causing my migrate commands to fail |
I'll revert the backport on 5.1. |
This reverts commit 1c1f2f2.
@GrahamCampbell umm might also be an issue in 5.2, since that too only gives 1 parameter. https://github.com/laravel/framework/blob/5.2/src/Illuminate/Database/Schema/Builder.php#L55 The comment in here also details adding schema to the parameters: #12856 but that is also not in 5.2 |
This reverts commit 023d0a0.
Feel free to send a fix to L5.1 and re-revert my revert if you can fix it. |
Ping @themsaid. |
@GrahamCampbell gosh I'm very sorry, it appears to be an issue on my extension of the Postgres library. The PR does correctly have it. Again very sorry. |
Cool, thanks for letting us know. |
This reverts commit 023d0a0.
As reported in #12856, the current query looks for tables in the
information_schema
regardless of theTABLE_SCHEMA
.