Opened 3 years ago
Closed 7 months ago
#14426 closed enhancement (fixed)
unneeded "SELECT FOUND_ROWS()" when no posts are found
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Priority: | normal | Milestone: | 3.5 |
| Component: | Query | Version: | 3.0 |
| Severity: | normal | Keywords: | has-patch |
| Cc: | mark@…, tim@… |
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)
- Summary changed from unneede "SELECT FOUND_ROWS()" when no posts are found to unneeded "SELECT FOUND_ROWS()" when no posts are found
comment:3
follow-up:
↓ 4
Denis-de-Bernardy — 3 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;
- 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?
@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.
- 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.
comment:9
SergeyBiryukov — 11 months ago
comment:10
follow-up:
↓ 11
wonderboymusic — 9 months 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 ) ) );
comment:11
in reply to:
↑ 10
SergeyBiryukov — 9 months 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.
comment:12
nacin — 9 months 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.
SergeyBiryukov — 9 months ago
SergeyBiryukov — 9 months ago
comment:13
SergeyBiryukov — 9 months 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.
SergeyBiryukov — 9 months ago
comment:14
SergeyBiryukov — 9 months ago
14426.3.patch also sets post_count when fields parameter is set (mentioned in #21733).
comment:15
SergeyBiryukov — 9 months 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).
comment:16
wonderboymusic — 8 months ago
#17066 was marked as a duplicate.
comment:13
wonderboymusic — 8 months ago
#21147 was marked as a duplicate.
comment:14
nacin — 8 months 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?
SergeyBiryukov — 8 months ago
comment:15
SergeyBiryukov — 8 months ago
14426.4.patch also updates $found_posts description and adds PHPDocs for set_found_posts().
comment:16
ryan — 8 months ago
- Owner set to ryan
- Resolution set to fixed
- Status changed from reopened to closed
In [21928]:
comment:17
nacin — 7 months 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.
comment:18
follow-up:
↓ 19
ryan — 7 months 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.
comment:19
in reply to:
↑ 18
nacin — 7 months 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."
comment:20
ryan — 7 months ago
A caching plugin that sets posts to an empty array would sneak through, but I doubt there are any of those.
comment:21
ryan — 7 months ago
- Resolution set to fixed
- Status changed from reopened to closed
In [22258]:

Doesn't that filter allows to redo the query using a plugin?