-
Notifications
You must be signed in to change notification settings - Fork 2
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
mysql_connect does not use a globally shared link #4
Comments
No, this wasn't intentional. Good catch! If you're able to come up with a fix, that would be great. Otherwise I'll take a look when I get the chance - bit overloaded with other tasks at the moment. |
added a pull request |
Thank you for this useful and convenient library. I used the following patch to mysql_connect to mimic the behavior of mysql_pconnect, which only sets the first new link to the global convenience link:
|
could you comment on why this works better than my fix? I'm curious |
I needed to add it to get some legacy code to work, I can't justify it beyond that. I just copied the current mysql_pconnect() behavior and that worked for my case. I briefly pondered the $new_link problem but couldn't find an easy agnostic solution. As far as I can tell, mysqli_connect() returns a new $link->thread_id every time it's called, whereas mysql_connect() reuses similar I suppose one could hash $server, $user, and $password values in parallel with $_php7_compat_global_db_link to eliminate redundant connections, but this compromise security. If the legacy code being made compatible is already managing multiple links explicitly, $_php7_compat_global_db_link should (?) be superfluous (but not unsafe) for those cases, because $_php7_compat_global_db_link only fills in for unspecified $link parameters. I didn't initially see your proposed change here : dafuki@a5d94fa. I would say that new lines 67-71 aren't necessary because $link is never used before being changed, just keep old line 67. It may be more similar to legacy PHP-Mysql behavior to unconditionally replace $_php7_compat_global_db_link every time a new connection is made, because PHP-Mysql uses the most recent connection for operations where $link is unspecified. This could forgive sloppy but otherwise working code.
I have not written PHP in a long time and may be overthinking kindness to poor legacy code. |
I'm trying to make sure I'm getting you right (not feeling well atm), but if there is no use of new_link, it's probably not a perfect match for the legacy function. |
I haven't forgotten about this issue, but unfortunately I've also not had time to fix it yet. |
I submitted pull request #7 just to get mysql_connect() working as well as mysql_pconnect() does, but I believe you're correct that the actual legacy mysql extension's behavior is slightly different. In my case it didn't matter though. |
Thanks for this fix. I'm leaving the issue open to remind myself to review it in more depth when (if?) I get the chance. For now the proposed changes sound like a good compromise if they allow most existing code to work without modification. |
mysql_connect does not use a globally shared link.
I needed to switch some sites to mysql_pconnect.
Is there a reason for mysql_conect to not create a global $link var?
This does not replicate the mysql_connect behavior of older php versions, was wondering if it is on purpose.
The text was updated successfully, but these errors were encountered: