Make WordPress Core

Opened 13 years ago

Closed 13 years ago

#20236 closed defect (bug) (fixed)

Improper testing of cache retrievals causes wasted queries.

Reported by: andy's profile andy Owned by:
Milestone: 3.4 Priority: normal
Severity: normal Version:
Component: Cache API Keywords: has-patch
Focuses: Cc:

Description

wp_cache_get() can retrieve values such as the empty array which is considered false by the ! test. The following function is incorrect because it will always ignore the cache and repeat the get_col and wp_cache_add when the cache contains the empty array. This occurs on sites which have no pages.

function get_all_page_ids() {
        global $wpdb;

        if ( ! $page_ids = wp_cache_get('all_page_ids', 'posts') ) {
                $page_ids = $wpdb->get_col("SELECT ID FROM $wpdb->posts WHERE post_type = 'page'");
                wp_cache_add('all_page_ids', $page_ids, 'posts');
        }

        return $page_ids;
}

The cache check should be written with an understanding of the return type of $wpdb->get_col() (array). Here it is with the check fixed:

function get_all_page_ids() {
	global $wpdb;

	$page_ids = wp_cache_get('all_page_ids', 'posts');
	if ( ! is_array( $page_ids ) ) {
		$page_ids = $wpdb->get_col("SELECT ID FROM $wpdb->posts WHERE post_type = 'page'");
		wp_cache_add('all_page_ids', $page_ids, 'posts');
	}

	return $page_ids;
}

Patch 1 fixes this occurrence. Please keep this ticket open until all similar occurrences are found and fixed.

Attachments (1)

20236.1.diff (544 bytes) - added by andy 13 years ago.
fix get_all_page_ids()

Download all attachments as: .zip

Change History (5)

@andy
13 years ago

fix get_all_page_ids()

#1 @scribu
13 years ago

  • Component changed from General to Cache
  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.4

#2 @scribu
13 years ago

I always thought that (bool) array() evaluates to true, but that's obviously not the case:

http://ch2.php.net/manual/en/language.types.boolean.php

#3 @ryan
13 years ago

In [20216]:

Don't generate a query when the all_page_ids cache is an empty array. An empty array is a valid cache value when no pages are present. Avoids a useless query for sites with no pages. Props andy. see #20236

#4 @ryan
13 years ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.