WordPress.org

Make WordPress Core

Opened 9 years ago

Closed 6 years ago

Last modified 3 years ago

#15805 closed enhancement (wontfix)

get_page_by_title() lacks caching

Reported by: Viper007Bond Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.1
Component: Performance Keywords: has-patch needs-testing
Focuses: Cc:
PR Number:

Description

get_page_by_title() always does a database query. It should use the object cache.

Attachments (3)

15805.patch (810 bytes) - added by solarissmoke 9 years ago.
Cache get_page_by_title queries
15805.diff (773 bytes) - added by songsthatsaved 6 years ago.
Updated patch for 3.6
15805.1.patch (1.0 KB) - added by tollmanz 6 years ago.

Download all attachments as: .zip

Change History (14)

@solarissmoke
9 years ago

Cache get_page_by_title queries

#1 @solarissmoke
9 years ago

  • Keywords has-patch added; needs-patch removed

@songsthatsaved
6 years ago

Updated patch for 3.6

#3 @songsthatsaved
6 years ago

Updated the patch for 3.6, and find that it is working as expected. This is really just an update of solarissmoke's patch.

#4 @songsthatsaved
6 years ago

  • Keywords needs-testing added

#5 @SergeyBiryukov
6 years ago

Note that 15805.diff performs an unnecessary database query before checking the cache.

15805.patch does not have this issue and still applies cleanly.

#6 @SergeyBiryukov
6 years ago

  • Milestone changed from Future Release to 3.7

#7 @tollmanz
6 years ago

I think there are a few potential issues with the suggested patch.

  1. $post_type should be considered in the cache key. If not, looking up the page with title "This is my title" will return the same result as a post with title "This is my title" if it is pulled from cache.
  2. How is the cache invalidated? Theoretically, the value is being cached indefinitely (although different caching engine will likely naturally purge the cache over time), which can cause some bizarre behaviors. For instance, if a post with title "This is my title" is created, subsequently deleted, then a new post with title ""This is my title" is created, the cached value would be the ID of the original post. I think we would need something along the lines of the following code to run on post trash and delete:
$key = 'post_title_' . get_the_title( $post_id ) . get_post_type( $post_id );
wp_cache_delete( $key, 'posts' );
  1. I think the patch needs further refreshing as get_page is used in the function, whereas current code uses get_post.

@tollmanz
6 years ago

#8 @tollmanz
6 years ago

On further evaluation, I think that this ticket should be closed as wontfix. get_page_by_title is not used in core at all. To add a caching layer to the function and not address invalidation seems dangerous to me; however, to add invalidation into WordPress core for a function that is never used seems unnecessary. It seems that caching and cache invalidation of this function should be the responsibility of the plugin/theme author and not WordPress core.

#9 follow-up: @nacin
6 years ago

  • Milestone 3.7 deleted
  • Resolution set to wontfix
  • Status changed from new to closed

I agree with tollmanz. Going further, I think the whole idea of fetching something by its title is wrong. It's not unique, or particularly identifying, and the field doesn't have an index. This is better left to WP_Query itself (i.e. a search), and I could even go for deprecating this in favor of a (new?) query variable in WP_Query.

#10 in reply to: ↑ 9 @peterwilsoncc
4 years ago

Replying to nacin:

This is better left to WP_Query itself (i.e. a search), and I could even go for deprecating this in favor of a (new?) query variable in WP_Query.

Now WP_Query has a title variable, is this worth reopening/a new ticket? My inclination is to set it up as a wrapper for get_pages.

Last edited 4 years ago by peterwilsoncc (previous) (diff)

#11 @boonebgorges
3 years ago

Now WP_Query has a title variable, is this worth reopening/a new ticket? My inclination is to set it up as a wrapper for get_pages.

Let's move discussion to #36905, which has a patch.

Note: See TracTickets for help on using tickets.