Skip to content

#47280 Remove usage of deprecated MySQL SQL_CALC_FOUND_ROWS from WP_Query #3863

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
johnbillion wants to merge 9 commits into
base: trunk
Choose a base branch
from
+613 −15
Open

#47280 Remove usage of deprecated MySQL SQL_CALC_FOUND_ROWS from WP_Query #3863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7aabde9
Remove usage of deprecated MySQL `SQL_CALC_FOUND_ROWS`.
johnbillion Jan 18, 2023
7888988
Apply suggestions from code review
johnbillion Feb 22, 2023
7d59ed9
Update tests/phpunit/tests/post/query.php
johnbillion Feb 22, 2023
a830ea7
Remove unnecessary variable assignment.
johnbillion Feb 22, 2023
991f0af
Better formatting for these docs.
johnbillion Feb 22, 2023
f5b240d
Add some invisible bytes.
johnbillion Feb 22, 2023
9395380
Merge branch 'trunk' into 47280-sql-count-found-rows
johnbillion May 19, 2023
7ef5d41
Misc tweaks.
johnbillion May 21, 2023
18b12fb
This isn't guaranteed to be a string (at least one plugin in the wild…
johnbillion Jul 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 40 additions & 9 deletions src/wp-includes/class-wp-query.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,14 @@ class WP_Query {
*/
public $request;

/**
* Get post database count query.
*
* @since x.x.x
* @var string
*/
public $count_request;

/**
* Array of post objects or post IDs.
*
Expand Down Expand Up @@ -2013,7 +2021,7 @@ public function get_posts() {
$q['page'] = absint( $q['page'] );
}

// If true, forcibly turns off SQL_CALC_FOUND_ROWS even when limits are present.
// If true, forcibly turns off the query to count found rows even when limits are present.
if ( isset( $q['no_found_rows'] ) ) {
$q['no_found_rows'] = (bool) $q['no_found_rows'];
} else {
Expand Down Expand Up @@ -3082,20 +3090,19 @@ public function get_posts() {
$limits = isset( $clauses['limits'] ) ? $clauses['limits'] : '';
}

$count_field = "{$wpdb->posts}.ID";

if ( ! empty( $groupby ) ) {
$count_field = $groupby;
Copy link
Copy Markdown
Member

@spacedmonkey spacedmonkey Jul 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would love some unit tests for some weird and wonder groupby values.


$groupby = 'GROUP BY ' . $groupby;
}
if ( ! empty( $orderby ) ) {
$orderby = 'ORDER BY ' . $orderby;
}

$found_rows = '';
if ( ! $q['no_found_rows'] && ! empty( $limits ) ) {
$found_rows = 'SQL_CALC_FOUND_ROWS';
}

$old_request = "
SELECT $found_rows $distinct $fields
SELECT $distinct $fields
FROM {$wpdb->posts} $join
WHERE 1=1 $where
$groupby
Expand All @@ -3104,17 +3111,38 @@ public function get_posts() {
";

$this->request = $old_request;
$this->count_request = "
SELECT COUNT(DISTINCT {$count_field})
Copy link
Copy Markdown

@grooverdan grooverdan Feb 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't DISTINCT be $distinct. Adding when its not needed results in more work for the DB.

Copy link
Copy Markdown

@grooverdan grooverdan Feb 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you're counting "found rows" here, you really don't want it to be distinct (as distinct rows <= actual rows). So technically DISTINCT should be omitted.

{$count_field} could be left as a * literal like the MySQL documents suggest, so if a $where clause exist, to choose from whatever available index suites the query best.

Look at the explain plan for when there's a WHERE clause that can use an index. Might not make a difference. Also see explain plain when a JOIN is added.

Copy link
Copy Markdown
Member Author

@johnbillion johnbillion Feb 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding DISTINCT, I'll check this again to be 100% sure but I think this needs to remain in place because some queries can result in duplicate rows being returned. These tests cover that scenario:

  • test_found_posts_are_correct_for_OR_meta_queries()
  • test_found_posts_are_correct_for_group_by_queries()

Copy link
Copy Markdown
Member Author

@johnbillion johnbillion Feb 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked this again and SELECT COUNT(DISTINCT {$count_field}) is indeed needed for the scenarios covered by the two tests above. If there was a way to only add the DISTINCT clause when we knew we needed it that would be great, but I'm not too confident about that.

Copy link
Copy Markdown

@kkataria3010 kkataria3010 May 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnbillion Not testing the code. But it looks like DISTINCT is needed only when groupby field is used.

FROM {$wpdb->posts} $join
WHERE 1=1 $where
";

if ( ! $q['suppress_filters'] ) {
/**
* Filters the completed SQL query before sending.
*
* @since 2.0.0
* @since x.x.x This query no longer contains a `SQL_CALC_FOUND_ROWS` modifier by default.
*
* @param string $request The complete SQL query.
* @param WP_Query $query The WP_Query instance (passed by reference).
*/
$this->request = apply_filters_ref_array( 'posts_request', array( $this->request, &$this ) );
Copy link
Copy Markdown
Member

@spacedmonkey spacedmonkey Jul 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnbillion We also need to check the posts_request_ids filter as well.


if ( is_string( $this->request ) && str_contains( $this->request, 'SQL_CALC_FOUND_ROWS' ) ) {
_deprecated_argument(
'The posts_request filter',
'x.x.x',
sprintf(
/* translators: 1: SQL query modifier 2: SQL query */
__( 'The %1$s query modifier should no longer be added to queries because results are no longer counted with %2$s by default.' ),
'<code>SQL_CALC_FOUND_ROWS</code>',
'<code>SELECT FOUND_ROWS()</code>'
)
);

$this->count_request = 'SELECT FOUND_ROWS()';
}
Comment on lines +3132 to +3145
Copy link
Copy Markdown
Member

@spacedmonkey spacedmonkey Jul 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we are checking both posts_request_ids and posts_request filters, let's make this generic, reusable and make it into it's own method. This will also make it more testable.

}

/**
Expand Down Expand Up @@ -3280,7 +3308,7 @@ public function get_posts() {
// First get the IDs and then fill in the objects.

$this->request = "
SELECT $found_rows $distinct {$wpdb->posts}.ID
SELECT $distinct {$wpdb->posts}.ID
FROM {$wpdb->posts} $join
WHERE 1=1 $where
$groupby
Expand All @@ -3292,6 +3320,7 @@ public function get_posts() {
* Filters the Post IDs SQL request before sending.
*
* @since 3.4.0
* @since x.x.x This query now no longer contains a `SQL_CALC_FOUND_ROWS` modifier.
*
* @param string $request The post ID request.
* @param WP_Query $query The WP_Query instance.
Expand Down Expand Up @@ -3555,11 +3584,13 @@ private function set_found_posts( $q, $limits ) {
* Filters the query to run for retrieving the found posts.
*
* @since 2.1.0
* @since x.x.x This query was changed from `SELECT FOUND_ROWS()` to a more
Copy link
Copy Markdown
Member

@spacedmonkey spacedmonkey Jul 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought, I wonder if we could make it so we can call set_found_posts by itself with the rest of the call. We have examples in core where we get one post to get the count see.

wordpress-develop/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php

Line 402 in 722f737

$count_query->query( $query_args );

* efficient `COUNT` query.
*
* @param string $found_posts_query The query to run to find the found posts.
* @param WP_Query $query The WP_Query instance (passed by reference).
*/
$found_posts_query = apply_filters_ref_array( 'found_posts_query', array( 'SELECT FOUND_ROWS()', &$this ) );
$found_posts_query = apply_filters_ref_array( 'found_posts_query', array( $this->count_request, &$this ) );

$this->found_posts = (int) $wpdb->get_var( $found_posts_query );
} else {
Expand Down
Loading