Use PDO for MySQL queries when PDO is available

[scottconnerly]

the mysql_* functions are officially deprecated for PHP 5.4 and will begin throwing E_DEPRECATED errors in the next version of PHP.
​http://marc.info/?l=php-internals&m=131031747409271&w=2

Wordpress should use PDO by default, but fall back to mysql_* when PDO is not present.

See also: #11622 for last year's discussion.

Would it make sense to create translation functions for mysql_* if they're not available?

I can see this being a saving grace for plugins / themes that do not use $wpdb. ​Example plugin

Incomplete non-functioning concept code:

<?php

if (!function_exists('mysql_query')) {
        $pdo_conn = null;
        function mysql_connect($host = 'localhost', $user = '', $pass = '' ) {
                global $pdo_conn = new PDO('mysql:;host=' . $host, $user, $pass);
        }
        function mysql_query($sql) {
                global $pdo_conn;
                return $pdo_conn->query($sql);
        }
        function mysql_fetch_assoc($rs) {
                return $rs->fetch(PDO::FETCH_ASSOC);
        }
        // ... etc.
}

I don't think we should waste effort on making plugins/themes that still don't use $wpdb work in PHP 5.4.

The cleanest way to start using PDO would be something like this:

if ( PDO is available ) {
  require 'wp-includes/wp-db.php';
  $wpdb = new wpdb( ... );
} else {
  require 'wp-includes/wp-db-legacy.php';
  $wpdb = new wpdb_legacy( ... );
}

Relevant plugin: ​https://github.com/mrtorrent/pdo-for-wordpress

Proof of concept - use a database driver system, leave wp-db mostly intact

Related, #21055

---

Patch attached. Should be fully backwards compatible. All unit tests pass, and no code changes required anywhere outside of wpdb.

This leaves wpdb alone for the most part, and just swaps in a different database driver to handle any msyql_* specific calls. No new strings, even! It includes drivers for mysql, mysqli, and pdo_mysql.

This doesn't overreach (no transaction support, and no rewriting of prepare ). It's just designed to address main concern of the ticket: the mysql extension may not be available (due to deprecation?) at some point, and wpdb should support other options.

As we discussed in IRC, this is probably far in the future as deprecation notices won't be turned on until after php 5.4 and other drivers (mysqli, pdo) have their quirks, too.

+2 - I love this, transaction support in PDO would make me giddy.

Updated for changeset 21420

Patch updated for [21420]. Currently, the mysql driver is the only one that works with the new config options. The mysql, mysqli, and PDO drivers all have different ways to specify their options, so a single constant in wp-config.php won't work going forward with this patch.

Also, I'm not sure if there's a new_link flag for PDO/mysqli. Isn't this default unless specify persistent connections? Perhaps this is addressable in the drivers ...

Fixed a bug with mysql_num_rows

Latest patch fixes a bug where mysql_num_rows was called on a result set that may not actually be valid (i.e. bad query).

PHP is looking to deprecate the mysql soon:
​https://wiki.php.net/rfc/mysql_deprecation

To aid this process, Rasmus is urging applications such as WP to provide mysqli support:
​http://news.php.net/php.internals/63825

+1 on this, especially in light of the MySQL deprecation. The question is whether to use mysqli or PDO with the MySQL driver. I think we should stick to a single one in core rather than having both. mysqli has a larger install base than PDO as far as I remember.

Advantages of PDO:

• has client-side prepared statements
• lightweight ORM (WP_Post anyone?)
• supports multiple database types

nacin has expressed a preference for PDO, so that's looking likely at the moment. It'd be nice to see statistics of mysqli vs PDO + PDO_MySQL.

As much as I'd like for us to just switch to PDO and be done with it, I think scottconnerly's approach is the safest:

WordPress should use PDO by default, but fall back to mysql_* when PDO is not present.

Later on, if needed, we can switch the fallback from mysql_* to mysqli_*.

The voting for the RFC is ​currently underway, for those interested. (It needs a 50% + 1 extra vote to succeed, as it's not a syntax change.)

Replying to rmccue:

The voting for the RFC is ​currently underway, for those interested. (It needs a 50% + 1 extra vote to succeed, as it's not a syntax change.)

Looks like it's going to pass.

I think we should try and tackle this in 3.6-early so that it gets the maximum testing time through the cycle.

The vote succeeded, 25-12. Post from PHP internals:

I ended up leaving the vote open for a couple of extra days (been a
busy week), but I've now closed the ext/mysql deprecation vote. The
full results are at ​https://wiki.php.net/rfc/mysql_deprecation — the
short version is that the final vote was 25-12 in favour of
deprecation in PHP 5.5.

This made the second question moot, but for the record, the result
there was 26-12 in favour of option (a) (deprecation in PHP 5.6 if not
in PHP 5.5).

[...]

I intend to commit the patch along with the relevant test updates in
the next few days. Admittedly, I'm bad with timeframes, so please
don't hang me if it slips a little. It'll be in before beta 1, I
promise.

We're now in 3.6-early, so I personally think we should try and land this as soon as possible. Will this need a refresh?

Bugfixes, 3.6 compat

21663.4.patch​ has been tested with 3.6, unit tests, and installer code. Seems to work in PDO and classic mysql modes. This new version addresses a few bugs and allows for the user to define a driver via a WPDB_DRIVER constant in wp-config.php or db.php.

I'm open to input. There was some disagreement over whether interfaces was the right way to go in #6821.

This model should give plugin authors the ability to define a new db driver in db.php, too.

If you go the driver route, you should reduce duplicate code between those classes, e.g. to have that in a (abstract?) base class.

Also I'd say there is pretty much an overhead through the level of indirection by having "drivers". I mean PHP offers this Drivers Thingy with PDO, no need to re-invent the wheel technically - just to integrate PDO which is already driver based.

Just while I've quickly seen it in the patch: For the PDO Driver, keep in mind that the DSN can be an alias. You can not just convert host / database / username / password here into a DSN like in the patch. Also you've hardencoded the actual PDO driver. That's limiting the use of a PDO based database class for no reason. Just saying, what jumps into my sight, it's maybe too early for concrete feedback at all.

To reduce the levels of indirection, it is probably more straight forward to offer a set of default database classes that Wordpress can load (by configuration). If you work with Interfaces, it is not important any longer if that included database class then is having a full blown driver implementation loading diverse drivers or is just a straight-forward database class as we have it since years (and as we love it). The legacy database class then can become the blueprint for the interface.

Next generation classes could implement an interface that extends from the legacy interface (to deal with the problems the original implementation has, or at least to offer some way to deal with it).

Also I suggest you leave warnings all over the place that the prepare function is misnamed. I've seen on the Wordpress dev blog that users are being told this is the way to "prepare" just not so long ago, but the prepare function actually just verifies and sanitizes the first string parameter (the query) and then does some dumb stuff on the passed array that should represent values. This is the exact opposite of what prepared and parametrized queries are.

If you want to improve things, start there offering an additional function that follows common parametric queries patterns so that it's not such a burden to go there with a Mysqli / PDO based class. The escape route is not used by a driver based abstraction, like PDO demonstrates.

Otherwise, this all might just not be worth the work because even as of today, you can simply replace the database class with the taste you like most.

Just my 2 cents.

I think we should do this in WordPress 3.6 Cycle to have a better performance and easy API :)

21663.4.patch​ still applies cleanly.

Seeing a notice with 21663.4.patch​:

Notice: Undefined offset: 2 in wp-includes/class.wp-db-driver-pdo_mysql.php on line 60

escape() should probably stay in wpdb since it is a weak escape, not a real escape. If we introduce a public real escape method then that can go in the drivers.

Replying to SergeyBiryukov:

Seeing a notice with 21663.4.patch​:

Notice: Undefined offset: 2 in wp-includes/class.wp-db-driver-pdo_mysql.php on line 60

How did you encounter this? ​PDO::errorInfo should always return a 3 element array.

Replying to kurtpayne:

How did you encounter this? ​PDO::errorInfo should always return a 3 element array.

Just applied the patch and saw a lot of these notices in the admin and on the front-end.

PHP 5.2.14 on Windows. This is what var_dump( $error ) shows:

array(1) { [0]=> string(5) "00000" } 

Related: ​http://stackoverflow.com/questions/3999850/pdo-error-message

It seems like somewhere internally in PDO it runs an array_filter() and removes the second and third elements since they're null. Yay PHP.

Address PDO::errorInfo notice

The PDO::errorInfo() notice should be addressed now. If there's no third element, this method will return an empty string.

In support of earlier comments -
I don't see any point in wasting time, as one put it, to keep plugins using mysql_* compatible.

If anything, such plugins should be removed from the repository and treated as a risk or as non-compatible to WordPress as a whole.
In a "get-with-the-program-or-stay-away" manner.

Replying to adegans:

If anything, such plugins should be removed from the repository and treated as a risk or as non-compatible to WordPress as a whole.
In a "get-with-the-program-or-stay-away" manner.

Why the support of mysql_* functions is still exists , and if you see the code above there is a constant WPDB_DRIVER that can select the needed database driver .

Notice that the plugin that used mysql_* functions was should using $wpdb class insted of :)

21663.6.patch​:

• renamed get_driver() to set_driver(), mark as private and update phpdoc
• renamed new files to better match the other files in wp-includes
• updated phpdoc for driver classes

What do we do with wpdb::set_charset()? It's calling mysql_set_charset().

get_driver() -> set_driver() + cleanup

Replying to scribu:

What do we do with wpdb::set_charset()? It's calling mysql_set_charset().

The new patch issues a query for SET NAMES %s and COLLATE %s which get delegated to the specific driver.

From ​IRC today, the prevailing thought is to dump mysqli support and probably the driver system, too.

My original thoughts when writing this: Get off of the old mysql extension. If PDO is available, great. If not, jump onto mysqli. The original cause for the ticket was "mysql is going to be deprecated." I don't know of a firm method or date for that deprecation, though.

mysqli doesn't buy us anything over PDO.

What are our options?

1. Keep the new system, it's fine
2. Drop the mysqli driver
3. Drop the new system and rewrite wp-db to just have PDO
4. ???

It doesn't look like WordPress will easily support other database engines (e.g. sqlite) at this point without rewriting queries or abstracting out the database layer further.

Considerations: How will this impact hyperdb? What can we do to add value while we're under the hood?

one thing pdo doesnt offer, but mysqli does is a set_charset() function. If you think its a good idea to emulate it by issuing a set names sql query, realize it's not fully equivalent.

read

​http://www.php.net/manual/en/mysqlinfo.concepts.charset.php

​http://stackoverflow.com/questions/1650591/whether-to-use-set-names/14132028#14132028

I don't use wordpress, but I have a feeling you and your users will still do manual string escaping for many years. And, they may change the charset at runtime after initial connection.

Not only could there be minor bugs, but maybe even rare sql injection opportunities reminiscent of the conditions talked about here
​http://shiflett.org/blog/2006/jan/addslashes-versus-mysql-real-escape-string (the point is the escaping function was naive of the character set, and therefore couldn't properly escape the string).

although - this is nothing new for any current wordpress code that uses mysql ext and uses a set names query instead of calling mysql_set_charset(). its exactly the same.

food for thought. good luck.

Replying to scribu:

I don't think we should waste effort on making plugins/themes that still don't use $wpdb work in PHP 5.4.

By my count, I've found 1,722 plugins in the WP.org plugin repository that make raw calls to mysql_*() functions. Some might have nothing to to with the WP main connection or not actually be calls to core PHP methods, but most of them likely are.

That's 7.4% of all plugins.

What those plugins are doing is wrong, however, it works, and I don't think we should intentionally break them before we absolutely need to. We can at least give them a fighting chance by only using the PDO (or mysqli) extension when either the mysql extension is no longer available, or just on PHP 5.5+ (to avoid dealing with deprecation notices during development even though the mysql extension is available). There's not any compelling reasons that I see for making the jump any earlier than PHP 5.5; we can continue using mysql through PHP 5.4.

Replying to bpetty:

By my count, I've found 1,722 plugins in the WP.org plugin repository that make raw calls to mysql_*() functions. Some might have nothing to to with the WP main connection or not actually be calls to core PHP methods, but most of them likely are.

That's 7.4% of all plugins.

You know how many of those are > 2 years old?

Does this include old versions too (from older tags)? I'm sure that could also slant things maybe.

Some might have nothing to to with the WP main connection or not actually be calls to core PHP methods, but most of them likely are.

That's a very important assumption. It could mean that 7.39% of all plugins would be affected or only 0.01%.

It's pretty awesome how much everyone here raves about how WordPress maintains full compatibility and never removes old API, but when it comes down to things like this, everyone is like "screw them".

What are we gaining by using PDO or mysqli in PHP 5.3 or 5.4 as opposed to using mysql? The answer is absolutely nothing. In fact, if this new code isn't extensively tested before release, we're actually just risking potential bugs (including possible SQL injection issues) with new changes that provide no additional benefits in PHP 5.4 and below. So why is it even being debated? We have the option of slowly rolling this out, and we should take advantage of that.

Supposedly, there are performance gains by using *any* module but the original mysql. PHP recommends against using it in their own documentation, and it will be deprecated before long.

I agree that it's important to maintain compatibility, but I'm looking forward to performance gains as well.

I am curious to know how you queried the repository to look for mysql_* calls. I have a feeling that scribu is right that this number is inflated by older code or older tags.

Question: What sort of performance impact would be seen by creating legacy connection in addition to the PDO/mysqli connection. Wouldn't that be enough to allow older code to continue undisturbed?

@bpetty In fairness, the ticket has been open for 5 months, there has been time to raise concerns.

@deltafactory I've applied the patch to a copy of our production DB/code and benchmarked actual page generation. Thus far it's working fine in PDO-mode at performance numbers within a margin of error. No significant loss/gain.

I'm also interested to hear how this would impact HyperDB. I'm personally interested in moving the idea of DB roles into core (for comparison, Drupal has been there for some time).

Apparently everyone seems to have gotten the impression that I'm suggesting we don't migrate to PDO/mysqli at all, so I just want to clarify that this is *not* the case at all. I like this patch, and I do know the mysql extension is deprecated in PHP 5.5, so this has to happen.

To be perfectly clear, what I'm saying is that the patch needs to not default to PDO first if it's available *unless* the version of PHP being used is 5.5 or above, and in cases of PHP 5.4 and below, it should default to using the existing mysql extension it's using right now. In doing so, we give plugin authors who did incorrectly use raw mysql calls a chance of fixing their plugin before it breaks on every WordPress installation the second everyone upgrades to 3.6, and it also gives users of those plugins the time to perform their plugin updates appropriately as well.

As long as we can disable mysql and enable PDO for any PHP version, and then run or test WordPress through PDO, we are fine. For PHP < 5.5, both drivers enabled, mysql takes priority. For PHP >= 5.5, both drivers enabled, PDO takes priority to avoid using deprecated functionality. For any PHP version, when only one driver is avaialable, WordPress will use that.

Replying to bpetty:

To be perfectly clear, what I'm saying is that the patch needs to not default to PDO first if it's available *unless* the version of PHP being used is 5.5 or above, and in cases of PHP 5.4 and below, it should default to using the existing mysql extension it's using right now. In doing so, we give plugin authors who did incorrectly use raw mysql calls a chance of fixing their plugin before it breaks on every WordPress installation the second everyone upgrades to 3.6, and it also gives users of those plugins the time to perform their plugin updates appropriately as well.

I agree with the idea, but disagree with the approach.

Plugins using mysql_* functions basically are relying on the mysql_connect and mysql_select_db functions having been called to set up that initial connection for them to use. It would be possible to call both of those in PHP < 5.5 installs, just to set up the connection for plugins, and then to continue on using the newer methods, if available, for the speed improvements.

For the people who argue that making a connection which is then unused is rather stupid, I agree. But it would both maintain backward compatibility and give the performance boost. And, we could wrap this extra connection in a define or some such to allow power-users to turn it off manually via their wp-config.

If you want to take this idea further and somehow detect when mysql_* functions are being used and do the connection on the fly somehow, then feel free to think about that. But for a first draft, just adding a couple of extra old mysql calls is enough for backward compat, I believe.

I'm really against putting all this effort into PDO and then only enabling it by default on sites with PHP 5.5+. It seems like a waste.

I'm not in love with Otto's suggestion, but it may be a good way around these poorly coded plugins (and they are poorly coded, we've had wpdb::query() for this since BEFORE v1.0).

Has anyone put together an actual list of these plugins that are doing it wrong? Would be good to call these plugins out in case anyone can get w/ these authors and help them migrate over.

I'd also like to see pdo/mysqli/mysql be the order priority of usage. Backwards compatibility connection (optional) to mysql_ if it's available and pre PHP 5.5.

I am very against bending over backwards to support bad old plugins. I like to think I do things the right way, and with every release, I've had to recode something to prepare. Exempting the lowest common denominator from such "pain" is just ignoring the problem. There was an admin action in 3.5 which was removed without warning that broke 3 of the plugins I wrote for our photo editor. Do we not expect plugin authors to be the ones responsible for keeping up with the times?

@sc0ttclark, I asked a similar question. Not sure how to efficiently search the code of the repository, short of downloading the entire trunk.

@wonderboymusic, it's not bending over backward to *keep* the current functionality (on some level) while trying to move the platform to its most current. I'm surprised you are so gung-ho about leaving people behind considering the work you had to do so recently.

The lead devs are conscious of the "brand" impact this has. WP doesn't want a reputation of breaking during updates for a number of reasons that I don't need to cover here. The $wpdb->prepare() situation from 3.5 was another recent instance where bad plugin code penalized "regular" users who had otherwise had a safe experience upgrading WP. I don't think I'm alone in my desire to prevent bad plugin code + hasty decisions in core from hurting users who aren't web developers.

I would like to see a phased approach based on what I and others have suggested:

1. Provide a legacy connection with mysql_connect() etc, no matter what $wpdb uses.
2. Gather stats about offending plugins in the repository and notify authors. (How?)
3. Announce the intention to deprecate, knowing it probably won't reach the offenders.
4. Possibly scan for mysql_*() functions and warn (on the plugin admin page, only?)
5. Remove legacy connection, with option to re-enable by config constant, starting in ~3.8

Dropping mysql isn't an option. Its removal from PHP is close enough that now is as good a time as any to start "ripping off that band-aid" as far as the WP community is concerned.

Replying to deltafactory:

Not sure how to efficiently search the code of the repository, short of downloading the entire trunk.

See ​https://github.com/markjaquith/WordPress-Plugin-Directory-Slurper

In general I think @deltafactory's suggestions are reasonable, with one caveat and one question.

The legacy connection needs to be able to be turned on and off by a constant as soon as it's introduced. I'd prefer off by default, but I could go either way so long as it exists. Opening both a PDO and mysql connection will double the open mysql connections. While PDO/mysql seem negligible in my testing, doubling the connections will be a huge performance issue.

Just think of all the WP sites we've ever seen go down for a DB error, now imagine that suddenly happens at half the traffic.

The question is whether WP has a precedent of version detection. I've seen plenty of feature detection and that makes sense, but I don't know if I get the value of version detection at an app level in the PHP world.

In regards to performance, this is the driver we're talking about, not the query itself. It's almost never the bottleneck compared with the query, your indexes, or your PHP code working on the results (which should already be cached anyway if it's been optimized properly).

Of course this doesn't stop people from benchmarking them anyway. ​Many of ​them don't find more than a 5% difference in performance with equivalent operations. Just as many find wildly different results. One benchmark even ​claims (though without publishing test details/results) that the original MySQL extension is faster in all tests they ran.

Regardless, don't believe most of the benchmarks out there in regards to this. Most of them don't configure their sandbox properly for benchmarking (tuning various optimization settings, clearing caches, etc), and don't measure the appropriate aspects, often mixing performance measurements of tasks unrelated to the driver itself such as tests that spend 90% of the time running the SQL query itself, or code that extracts data from the results - assuming that these will perform consistently enough that they won't affect measurements, and assuming that this is mostly how the driver is being used across all installations of the application.

Of course, we should be profiling these changes anyway, just in case some part of it is inefficiently written (outside of use of the driver anyway) and does make a significant difference for a default WP installation. However, focus on compatibility is much more important than database driver performance (and staying on what everyone is already using right now until PHP is upgraded isn't hurting anyone's current performance). I haven't seen any guides or presentations on WordPress performance ever suggest possibly switching this out to speed up WordPress (and it's ridiculous how many WP performance presentations I've seen, there's at least one per WordCamp). There's significantly more important aspects to consider when it comes to performance.

The scope of this ticket was originally to address the issue of compatibility, not performance. If performance was really a concern here, we would have seen a patch doing exactly the same thing here well before the MySQL extension was deprecated.

Ryan and I discussed this ticket about a month ago in IRC and decided then it should be punted. I am sorry this did not make it back to the ticket. For a few reasons.

PHP 5.5 isn't out yet, and PHP 5.4 (despite being out for a year) as a whopping 1% market penetration. Because this only results in E_DEPRECATED (which we suppress anyway), we're at least a few months to a year out before we even start to look bad here, and we'll work just fine for a while. I would love to revisit this and get it done in either 3.7 or 3.8 (as in, this year). It just isn't a priority for 3.6.

Emphasis this cycle on slashing, caching, and our nascent terms roadmap needed to take precedence, and we didn't have as much bandwidth as expected to cover everything.

Once PHP 5.5 is out, I imagine there will be increased interest in this ticket. That is good. Maybe at that point there can be a better consensus on the approach, as well as a clearer picture of the impact.

Even something as simple as deciding which engine will be used if both are available (pre-5.5) is an important decision. Our mysql engine *does work*. We don't want a bug in the PDO engine to cause us problems on servers and PHP versions that are just fine. Also, PDO has quite a bit of bugs in earlier versions of PHP (5.2, 5.3) which make up the most of our target audience. We should research said issues.

I was reading through this with growing excitement and repeated thoughts of "well, finally, about time", so I was pretty sad to hit nacin's comment at the end and have my hopes dashed for the next few years (most likely, PHP releases seem to roll around very slowly).

An approach which maintains backwards compatibility in the meantime (as advocated by some above), but allows PDO as a non-default option, would allow time for the code to stabilise, to get some practical research into any lingering PDO bugs (the only problems I ran into were pre-5.2) and to get feedback from other developers.

While we certainly can wait around, it's not something that we need to do and we're only delaying the inevitable and (most likely) making the eventual pain worse by choosing to do things at the last possible moment rather than allowing a smooth transition by getting it underway now.

Remove mysql extension requirement

I know it's an old discussion. Change is in wp_check_php_mysql_versions() so that disabling the mysql extension in PHP doesn't throw an error provided an alternate extension is available. Diff is against latest master branch on github; apologies if I screwed something up int the git => svn patch translation.

That was a nice catch on the extra extension check that was missing from the previous patches in wp-includes/load.php.

You're right though, your patch is unusable aside from that. I'm not really sure what you did wrong to end up with what looks like 3 copies of the every addition besides your small change.

Replying to bpetty:

You're right though, your patch is unusable aside from that. I'm not really sure what you did wrong to end up with what looks like 3 copies of the every addition besides your small change.

Sorry. I can't find any good information on generating a patch-compatible patch from diff-ing two git branches. "Patch compatible patch" doesn't get particularly good search results.

Replying to gohanman:

Sorry. I can't find any good information on generating a patch-compatible patch from diff-ing two git branches. "Patch compatible patch" doesn't get particularly good search results.

​http://scribu.net/wordpress/svn-patches-from-git.html should get you on your way