Don't populate $col_info in production

[GrahamKelly]

While profiling WordPress 3.3.2 I noticed that mysql_fetch_field() seemed to be fairly expensive. Upon looking into this I found that the col_info property is being set in wpdb but it does not appear (from some grepping through WP Core and quite a few extensions) that anyone is actually making use of the results (Either from directly accessing $col_info or from calls to get_col_info). In order to improve performance I wrapped the calls in 'if (WP_DEBUG)' which seems to do the trick.

Patch attached.

[johnbillion]

Out of curiosity, is your profiling data available? While it probably is a good idea to only fetch this while in debug mode (or maybe while SAVEQUERIES is true) I have previously profiled mysql_fetch_field and found the performance impact is negligable.

[nacin]

This could be an easy lazy-loading win.

We could use a magic getter to handle someone accessing col_info directly, then make get_col_info() act on $this->result directly. This would prevent mysql_fetch_field() from firing unless someone needs it.

[markjaquith]

GrahamKelly — can you please share your profiling data?

[pento]

New patch uses a magic getter to only load column info when requested.

The only downside of this is that we can't do the mysql_free_result( $this->result ) in wpdb::query(), so I've moved it to wpdb::flush().

[pento]

Patch 2 changes the access visibility of wpdb::col_info and wpdb::get_col_info() to protected - they may need to be over-ridden by replacements for wpdb.

[nacin]

In [21472]:

Lazy-load column info in wpdb. props pento. fixes #20838.

[josephscott]

The committed code - r21472 - includes error suppression:

@mysql_free_result( $this->result );

The PHP docs for ​mysql_free_result indicate that this is a problem only when a non-resource is provided:

If a non-resource is used for the result, an error of level E_WARNING will be emitted. It's worth noting that mysql_query() only returns a resource for SELECT, SHOW, EXPLAIN, and DESCRIBE queries.

Can we get a way with not having the @ with something like:

if ( is_resource( $this->result ) ) {
    mysql_free_result( $this->result );
}

?

It would be worth while to avoid the use of @ when ever possible.

[nacin]

Replying to josephscott:

It would be worth while to avoid the use of @ when ever possible.

We discussed this as well. I agree this is a worthwhile change. New ticket?

[SergeyBiryukov]

Replying to nacin:

We discussed this as well. I agree this is a worthwhile change. New ticket?

#21533

[nacin]

In [21511]:

Eliminate error suppression for mysql_free_result() and only call it when the result is actually a resource. Depending on the query, mysql_query() can return a boolean rather than a resource, hence the original use of error suppression.

Fixes a warning introduced in [21472] when calling mysql_free_result() was moved to flush().

fixes #20838.

[nacin]

[21511] also fixes #21533.

[jeffmcmahan]

Could someone update the $wpdb documentation at Codex to reflect this change? I don't think I'm competent to do so. But I can see that get_col isn't a lot of use anymore, is it?

[nacin]

Replying to jeffmcmahan:

Could someone update the $wpdb documentation at Codex to reflect this change? I don't think I'm competent to do so. But I can see that get_col isn't a lot of use anymore, is it?

There's no public API change here. We simply now load column information on demand, rather than with each query.