get_body_class() needless messes with global variables

[filosofo]

get_body_class() calls setup_postdata() without reason.

Also, it makes a direct database call, when using the API would be better:

• Avoid including drafts and private posts as part of its condition
• Avoid querying the database directly is a good idea wherever possible.
• Caching benefits

Patch fixes.

[ryan]

Related [10877]

[Denis-de-Bernardy]

I'm suspicious of the suggested api call. the get_var() call could arguably be improved (to exclude drafts, etc.), but it's probably faster than any api call we can think of, since it's using an index to fetch a primary key (i.e. it shouldn't even read the db tables).

[filosofo]

Can you substantiate your suspicions? Here's the actual database query for a page using my patch:

SELECT * FROM wp_posts  WHERE (post_type = 'page' AND post_status = 'publish')    AND post_parent = 37   ORDER BY post_title ASC LIMIT 0,1

and here it is without the patch (current behavior):

SELECT ID FROM wp_posts WHERE post_parent = 37 AND post_type = 'page' LIMIT 1

Both claim to take 0.00 seconds with the query cache off.

it's probably faster than any api call we can think of

How about an API call that doesn't even have to connect to the database at all? That's a possibility which the object cache.

get_var() call could arguably be improved (to exclude drafts, etc.),

We shouldn't be trying to figure out this logic in multiple places (DRY). It's almost guaranteed to produce vexing bugs in the future, for page statuses that we forget about.

[Denis-de-Bernardy]

Replying to filosofo:

Can you substantiate your suspicions?

Sure...

Here's the actual database query for a page using my patch:

SELECT * FROM wp_posts  WHERE (post_type = 'page' AND post_status = 'publish')    AND post_parent = 37   ORDER BY post_title ASC LIMIT 0,1

and here it is without the patch (current behavior):

SELECT ID FROM wp_posts WHERE post_parent = 37 AND post_type = 'page' LIMIT 1

Both claim to take 0.00 seconds with the query cache off.

these two queries potentially have very different behaviors in reality.

had there been an index on (post_parent, post_type) the second one would be proceed as follows:

1. open index on (post_parent, post_type) if it's not loaded in the memory already
2. scan index for post_type = page with post_parent = 37 (fast)
3. return the first ID found (fast)

since we're missing that index, it'll actually load rows one by one and return the ID of the first row it finds that works (making it much slower than it should be):

1. open the relevant index (likely post_parent in your example)
2. scan index for rows that work (fast)
3. until an ID is found, open the needed disc page if needed (slow) and load the field we're missing (post_type) for that row (fast)
4. filter rows that don't work (fast)
5. return the first ID that works (fast)

by contrast, the query using the API does:

1. open whichever index the query engine decides is correct (probably post_parent in your example) if not loaded already
2. scan index for IDs that potentially work (fast)
3. for each ID, open the needed disc page if needed (slow) and load the fields we're missing (post_type, post_title) for that row (fast)
4. filter rows that don't work (fast)
5. filter if it's not the first row (order by post_title limit 1) (slow if the result set is large, and I'm probably wrong to make the assumption that MySQL does a top-1 search on the fly instead of a quicksort on the full set)
6. load the entire row (slow)
7. return the entire row (slow if post_content is large)

I hope my suspicions make more sense...

[filosofo]

This ticket got sidelined into punt limbo by a controversial line in the patch which wasn't even the main point of the patch.

I've taken out the controversial line and re-focused the issue in #11439.