Make WordPress Core

Opened 13 years ago

Closed 12 years ago

#20177 closed defect (bug) (fixed)

Unneccessary term cache query for pages

Reported by: thedeadmedic's profile TheDeadMedic Owned by: nacin's profile nacin
Milestone: 3.5 Priority: normal
Severity: normal Version: 1.5
Component: Query Keywords: has-patch
Focuses: Cc:

Description (last modified by scribu)

When a request for a page is queried, $post_type in WP_Query::get_posts() is an empty string, which is then passed to update_post_caches().

This in turn calls update_object_term_cache() for 'post' (the default if $post_type is empty) instead of 'page'.

Since pages don't have taxonomies (at least not in a default setup), we're simply wasting a query. But even if they did have, we wouldn't even be caching the right ones (unless of course pages were assigned the same taxes as posts).

I would say there's two options here; review WP_Query::get_posts() to ensure the local variable $post_type is set correctly, or simply pass the string 'any' to update_post_caches() (which will then correctly determine the post type(s)) - thoughts?

Attachments (1)

query-missing-page-type.diff (498 bytes) - added by wonderboymusic 12 years ago.

Download all attachments as: .zip

Change History (12)

#1 @scribu
13 years ago

  • Keywords needs-patch added

I think we should make it so that update_post_caches() receives 'page' as the post type.

#2 @wonderboymusic
12 years ago

  • Keywords has-patch added; needs-patch removed

sets post_type to page when applicable before updating post caches - this should be harmless? patch added

#3 @wonderboymusic
12 years ago

  • Milestone changed from Awaiting Review to 3.5
  • Version set to 1.5

I think this has been happening ever since Pages have existed.

#4 @scribu
12 years ago

  • Keywords 2nd-opinion added

I thought the term cache priming only happened on the taxonomies registered for the post's type. If not, that's what we should fix.

#5 @wonderboymusic
12 years ago

yes, but the post_type for pages in update_post_caches is always post currently, since empty string is passed as the 2nd argument

#6 @scribu
12 years ago

I remember somewhere where it was proposed that we ditch the post_type arg and just check the post type directly on the objects.

#7 @scribu
12 years ago

  • Description modified (diff)
  • Keywords 2nd-opinion removed

Actually, update_post_caches() already does this if you pass $post_type = 'any' and that's the other proposed solution. Nevermind; patch looks good.

#8 @nacin
12 years ago

To make this more sturdy, what if we did if ( empty( $post_type ) ) $post_type = 'any';

#10 @nacin
12 years ago

Even better... We should change update_post_caches(). It has $post_type default to 'post'. But if $post_type ends up being empty, it again sets it to 'post'. It should probably set it to 'any' instead.

#11 @nacin
12 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In [21844]:

If update_post_caches() does not receive a post type, glean post types from the individual post objects, rather than assuming 'post'. fixes #20177.

Note: See TracTickets for help on using tickets.