Make WordPress Core

Opened 19 years ago

Closed 19 years ago

Last modified 6 years ago

#1157 closed defect (bug) (fixed)

post by post archives do one query per post

Reported by: michel-v's profile michel v Owned by: ryan's profile ryan
Milestone: Priority: normal
Severity: major Version: 1.5.1
Component: Administration Keywords:
Focuses: Cc:

Description

Since a few weeks or months ago, post by post archives do use one query per post in the database.

Attachments (3)

cache.diff (16.6 KB) - added by michel v 19 years ago.
template-functions-general.php.diff (1.1 KB) - added by michel v 19 years ago.
template-functions-links.php.diff (497 bytes) - added by michel v 19 years ago.

Download all attachments as: .zip

Change History (14)

#1 @michel v
19 years ago

  • Patch set to No

#2 @michel v
19 years ago

More info:

That's because we call get_permalink($arcresult->ID), which makes get_permalink() do a query.
Either get_permalink should be modified to handle being given extra info (such as the post's ID, title and date), or a new function duplicating its functionnality should be there, or get_archives should be hacked to have a temporary $post in the global space for get_permalink() to munch on.

Note: IIRC, when you make permalinks use a category too, the count rises to 2 queries per post that is not on the front page.

#3 @MC_incubus
19 years ago

Two patches:

First patch is to get_permalink(); Before, get_permalink only took an ID, performing a DB expensive lookup to get the post's info. My patch allows an already populated post object to be passed instead, using is_object() to determine that.

The second patch changes get_archives() so that it gets all the post columns that get_permalink will need (I added post_name, post_status, and post_author), and passes the entire object to get_permalink(), instead of just the ID.

edited on: 03-27-05 03:14

#4 @ryan
19 years ago

I've done something similar as part of a bigger cache cleanup. get_post() can take a post id or an already populated post object and return a reference to a post object. It will consult and update the post cache as needed. The same goes for get_category(). These functions are used whenever someone to get at post or category data. Template functions no longer need to delve into the caches directly.

I made use of references when passing around the posts array and when dealing with the caches to avoid lots of copying.

Diff is attached. This changeset is a bit outside the scope of this bug though.

#5 @MC_incubus
19 years ago

Ryan,

Those changes look good. But we'd still need get_archives() to populate $post_cache in order for get_post() to avoid redundant DB querying and fix this bug. Right?

#6 @ryan
19 years ago

They're cached on the fly each time get_permalink calls get_post(). The foreach loop in get_archives ends up setting the cache. At least, that's the theory. I need to test some more.

#7 @ryan
19 years ago

I had to make some small changes to be compatible with php4. Tested against php4 and 5.

http://trac.wordpress.org/changeset/2478

#8 @ryan
19 years ago

  • Owner changed from anonymous to rboren
  • Status changed from new to assigned

#9 @MC_incubus
19 years ago

But if the foreach loop sets the cache, you're making one query per post, and the problem remains. What we need to do is take the $arcrequests variable or whatever it is that grabs all the posts in one go (query 10 in Michel's original report), and cache that, so that get_permalink can just grab that cached data instead of making an additional query for each of the posts that it has already grabbed from the database. Do we have a function like cache_posts() that can take an array of post objects and cache them? If we did that right after get_archives() grabs the posts from the database, it should fix it.

#10 @ryan
19 years ago

Try it. Only one query is made. Each post retrieved with that one query is passed along to get_permalink() and then cached when get_post() is called. get_post() does not perform a query since the entire post is passed to it. This allows us to use only one loop instead of doing a separate cache loop and then doing the postbypost foreach. So, we're making the postbypost loop perform double duty.

update_post_cache() will take a post array and cache it.

edited on: 03-28-05 20:50

#11 @MC_incubus
19 years ago

Ah! I see it now. Blonde moment.

#12 @ryan
19 years ago

  • fixed_in_version set to 1.5.1
  • Resolution changed from 10 to 20
  • Status changed from assigned to closed

This ticket was mentioned in Slack in #core-coding-standards by jrf. View the logs.


6 years ago

Note: See TracTickets for help on using tickets.