-
Notifications
You must be signed in to change notification settings - Fork 11.2k
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
Removed redundant database selection query #34389
Conversation
Feels scary to remove something that's been there so long on a patch release. Sure every edge case has been thought of? |
Agreed - although I can't see any need for it at all. PDO had already established a connection with the database in question. I presume if it's a multi database setup it'd re-establish the controller completely? Also whenever this would be called the DSN would have also been initiated. With Laravel requiring >= PHP 7.3 there aren't any backwards incompatibilities with PDO/MySQL. |
if (! empty($config['database'])) { | ||
$connection->exec("use `{$config['database']}`;"); | ||
} | ||
|
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.
In the initial implementation I dug out, there is a comment which has been removed over time (did not check why), but I guess it makes sense to check the mentioned case before fiddling around here => c203b02#diff-907ccb2ad354af6868ebb5618f2da86bR22-R23
See also #4138
Great - thanks @mfn - I'll take a look tomorrow and test with sockets - a lot has changed in six years! 😅 |
I'm not sure we should be tinkering around with this on a patch release. |
Thanks, Taylor, understood. I've written a package that makes this change and can be pulled in. As for me, the use statement with escaped backticks was killing my connection to MariaDB MaxScale. For anyone it may be of use to - https://packagist.org/packages/motomedialab/maxscale-connector |
MariaDB MaxScale will fix this in the next 2.5.6 release. See: https://jira.mariadb.org/browse/MXS-3292 |
Hello!
Due to an issue I was having with connecting Laravel to MariaDB's Maxscale, I came across these lines of redundant code. Ironically, they were the actual lines triggering the issue I was having.
Lines 26-28 in Illuminate\Database\Connectors\MySQLConnector perform a query to use the given database name, based on the MySQL Database connection config having a database flag.
However, due to how PDO establishes connections, the database is already being chosen within the DSN generation that's being performed, on line 113 and called within the connect method on line 17.
This makes the use call being made redundant and can be removed.
I've tested this change on MySQL 5.7, MySQL 8.0 & MariaDB 10.1 & 10.5 and everything works as expected.