Opened 15 years ago
Closed 13 years ago
#14426 closed enhancement (fixed)
unneeded "SELECT FOUND_ROWS()" when no posts are found
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 3.5 | Priority: | normal |
Severity: | normal | Version: | 3.0 |
Component: | Query | Keywords: | has-patch |
Focuses: | Cc: |
Description
the query is being executed even if no posts are found....
IMHO the lines in query.php
if ( !empty($limits) ) { $found_posts_query = apply_filters( 'found_posts_query', 'SELECT FOUND_ROWS()' ); $this->found_posts = $wpdb->get_var( $found_posts_query );
should be replaced with
if ( !empty($limits) ) { if (!empty($this->posts)) { $found_posts_query = apply_filters( 'found_posts_query', 'SELECT FOUND_ROWS()' ); $this->found_posts = $wpdb->get_var( $found_posts_query ); } else $this->found_posts = 0;
Attachments (5)
Change History (30)
#1
@
15 years ago
- Summary changed from unneede "SELECT FOUND_ROWS()" when no posts are found to unneeded "SELECT FOUND_ROWS()" when no posts are found
#4
in reply to:
↑ 3
@
15 years ago
Replying to Denis-de-Bernardy:
Doesn't that filter allows to redo the query using a plugin?
If that is the purpose then why is it depended on $limit being non zero?
Even if it is, then the proposed code should probably be
if ( !empty($limits) ) { $found_posts_query = apply_filters( 'found_posts_query', 'SELECT FOUND_ROWS()' ); if (!empty($this->posts) || ($found_posts_query != 'SELECT FOUND_ROWS()') $this->found_posts = $wpdb->get_var( $found_posts_query ); else $this->found_posts = 0;
#5
@
15 years ago
- Cc tim@… added
Is this issue already fixed? I'm seeing the following in my wp-includes/query.php file:
if ( !$q['no_found_rows'] && !empty($limits) ) { $found_posts_query = apply_filters_ref_array( 'found_posts_query', array( 'SELECT FOUND_ROWS()', &$this ) ); $this->found_posts = $wpdb->get_var( $found_posts_query ); $this->found_posts = apply_filters_ref_array( 'found_posts', array( $this->found_posts, &$this ) ); $this->max_num_pages = ceil($this->found_posts / $q['posts_per_page']); }
The additional !$q['no_found_rows']
should prevent the extra SELECT FOUND_ROWS() from running, correct?
#6
@
15 years ago
@tmoorewp, IMO no. It doesn't answer the situation in which I would generally like to get the number of rows, but at a specific query (a search for example) there are simply no matching results.
#7
@
15 years ago
- Keywords needs-patch added
There are a few days left before freeze. If someone wants to post a patch to get this in, now's the time; otherwise it will be punted to the 3.2 dev cycle.
#10
follow-up:
↓ 11
@
13 years ago
- Resolution set to worksforme
- Status changed from new to closed
I no longer see the SELECT FOUND_ROWS()
happening after empty query results
add_filter( 'query', function ( $sql ) { error_log( $sql ); return $sql; } ); $q = new WP_Query( array( 'post__in' => array( 0 ) ) );
#11
in reply to:
↑ 10
@
13 years ago
- Resolution worksforme deleted
- Status changed from closed to reopened
Replying to wonderboymusic:
I no longer see the
SELECT FOUND_ROWS()
happening after empty query results
This is partly fixed in [19918], but only if $split_the_query
is true:
http://core.trac.wordpress.org/browser/tags/3.4.1/wp-includes/query.php#L2630
If $split_the_query
is false, set_found_posts()
still runs regardless of empty results.
#12
@
13 years ago
- Component changed from Optimization to Query
- Milestone changed from Future Release to 3.5
set_found_posts() should check if ( $this->posts ), and otherwise set 0 for found_posts and max_num_pages. We shouldn't need to do $this->found_posts = $this->max_num_pages = 0;
manually.
#13
@
13 years ago
- Keywords has-patch added; needs-patch removed
found_posts
is 0 by default, so I guess set_found_posts()
can just return early if ! $this->posts
(14426.patch).
14426.2.patch is a combined patch for #17066 and #21147 as well.
#14
@
13 years ago
14426.3.patch also sets post_count
when fields
parameter is set (mentioned in #21733).
#15
@
13 years ago
$found_posts
description seems inaccurate:
http://core.trac.wordpress.org/browser/tags/3.4.1/wp-includes/query.php#L981
Currently, it's the amount of posts if limit clause _was_ used (unless I'm missing something).
#14
@
13 years ago
$found_posts description seems inaccurate
Yes, it is. It is meant to only be set when limits are *are* used. However, we should also set it when limits are *not* used, since it would be equal to post_count in this case.
We should also be able to avoid a FOUND ROWS query when the post_count is less than what the limit requested, but that's a bit more difficult as you also have to ensure there was no offset or paging in effect.
14426.3.patch looks good. Do we agree?
#15
@
13 years ago
14426.4.patch also updates $found_posts
description and adds PHPDocs for set_found_posts()
.
#16
@
13 years ago
- Owner set to ryan
- Resolution set to fixed
- Status changed from reopened to closed
In [21928]:
#17
@
13 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
[21928] broke the advanced caching plugin. That's an older version than what they run on WP.com, but you can see that the found_posts_query is avoided if the query is cached, and then on the the_posts hook, the posts are populated. Right after the_posts, core sets post_count.
If we're going to avoid setting found_posts if no posts are found, then we may need to re-adjust found_posts when we adjust post_count after the_posts. If we can't figure out the right logic, we should just remove || ! $this->posts
for now and come back to it in 3.6.
#18
follow-up:
↓ 19
@
13 years ago
Or have the plugin call set_found_posts() itself. There's only so much that can be done to retain back compat with a plugin that does such deep voodoo.
#19
in reply to:
↑ 18
@
13 years ago
Replying to ryan:
Or have the plugin call set_found_posts() itself. There's only so much that can be done to retain back compat with a plugin that does such deep voodoo.
Or we can type-check it. So, return if $this->posts === array()
(or is_array( $this->posts ) && ! $this->posts
). That allows a null/false for $this->posts to be considered as "hey, someone is doing some crazy stuff here, let's not make too many assumptions."
Doesn't that filter allows to redo the query using a plugin?