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

johnbillion · 2023-01-18T20:11:40Z

Trac ticket: https://core.trac.wordpress.org/ticket/47280

This carries on the work started on #330 and #2119. Lazy loading the found_posts and max_num_pages was an interesting exercise but upon discussion with colleagues it's become clear that it'll result in potentially inaccurate counts if either of those properties are accessed after changes have been made to a post that exists in the result set:

$query = WP_Query($args);
wp_update_post(...);
$query->found_posts;

This PR implements the changes from #2119 minus the lazy loading of those two properties. It's fully compatible with the WP_Query query caching that was introduced in WordPress 6.1.

High level changes:

Replaces usage of SQL_CALC_FOUND_ROWS and SELECT FOUND_ROWS() with an unbounded COUNT of the query, which is the method recommended by MySQL and is more efficient
Retains usage of SELECT FOUND_ROWS() only as a fallback if the query is filtered and a SQL_CALC_FOUND_ROWS modifier inserted (low likelihood but needs to be handled)

Details and todos:

Plugin authors:

If you're the maintainer of a plugin which makes changes to the way WordPress queries for posts, counts results, or uses the found_posts_query or posts_request filter, please test your plugin with this change in place.

Other query classes

Once WP_Query is complete, the process can be repeated for all query classes that can perform count queries:

WP_Network_Query
WP_Site_Query
WP_Comment_Query
WP_User_Query

grooverdan · 2023-02-20T19:44:18Z

src/wp-includes/class-wp-query.php


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


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

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.

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()

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.

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

peterwilsoncc

I've added a few notes inline but nothing major.

Are you able to set up a PR against trunk with the tests to ensure the suite passes on trunk with the exception of the SQL_CALC_FOUND_ROWS related assertions? This can wait if you've still got some wrapping up of this PR to do.

src/wp-includes/class-wp-query.php

peterwilsoncc · 2023-02-20T22:32:56Z

tests/phpunit/tests/post/query.php

+	 * @param string $fields Value of the `fields` argument for `WP_Query`.
+	 */
+	public function test_found_posts_are_correct_for_basic_query( $fields ) {
+		self::factory()->post->create_many( 5 );


This appears multiple times, is it possible to create it as a shared fixture. It might need to be a CPT to avoid messing up the assertions elsewhere in this file.

tests/phpunit/tests/post/query.php

Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com>

… returns boolean false).

spacedmonkey · 2023-07-18T14:32:30Z

src/wp-includes/class-wp-query.php

 			 * 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


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 );

spacedmonkey · 2023-07-18T14:40:49Z

src/wp-includes/class-wp-query.php

 			 * @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 ) );


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

spacedmonkey · 2023-07-18T14:41:58Z

src/wp-includes/class-wp-query.php

+			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()';
+			}


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.

spacedmonkey · 2023-07-18T14:43:08Z

src/wp-includes/class-wp-query.php

+		$count_field = "{$wpdb->posts}.ID";
+
 		if ( ! empty( $groupby ) ) {
+			$count_field = $groupby;


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

spacedmonkey · 2023-07-18T15:49:30Z

With 10k+ posts

Before

After

spacedmonkey · 2023-07-18T21:54:12Z

Maybe related - https://bugs.mysql.com/bug.php?id=21849

archon810 · 2023-11-09T20:10:27Z

Any updates here please? SQL_CALC_FOUND_ROWS is twice as slow as count+select in our use cases and would mean massive performance improvements. https://core.trac.wordpress.org/ticket/47280 is almost at the finish line.

mibmo · 2024-05-31T17:56:57Z

what's blocking here? i'd love to help fix this.

github-actions · 2024-05-31T17:57:09Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @grooverdan, @archon810, @mibmo.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

Core Committers: Use this line as a base for the props when committing in SVN:

Props johnbillion, peterwilsoncc, spacedmonkey.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Remove usage of deprecated MySQL SQL_CALC_FOUND_ROWS.

7aabde9

johnbillion marked this pull request as ready for review January 18, 2023 20:19

This was referenced Jan 18, 2023

Remove usage of deprecated MySQL SQL_CALC_FOUND_ROWS #330

Closed

#47280 - Remove SQL_CALC_FOUND_ROWS and lazy load found_rows #2119

Closed

johnbillion changed the title ~~#47280 Remove usage of deprecated MySQL SQL_CALC_FOUND_ROWS~~ #47280 Remove usage of deprecated MySQL SQL_CALC_FOUND_ROWS from WP_Query Jan 31, 2023

grooverdan reviewed Feb 20, 2023

View reviewed changes

peterwilsoncc reviewed Feb 20, 2023

View reviewed changes

johnbillion and others added 5 commits February 22, 2023 16:18

Apply suggestions from code review

7888988

Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com>

Update tests/phpunit/tests/post/query.php

7d59ed9

Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com>

Remove unnecessary variable assignment.

a830ea7

Better formatting for these docs.

991f0af

Add some invisible bytes.

f5b240d

spacedmonkey self-requested a review March 14, 2023 17:14

johnbillion added 3 commits May 19, 2023 12:16

Merge branch 'trunk' into 47280-sql-count-found-rows

9395380

Misc tweaks.

7ef5d41

This isn't guaranteed to be a string (at least one plugin in the wild…

18b12fb

… returns boolean false).

spacedmonkey reviewed Jul 18, 2023

View reviewed changes

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

Are you sure you want to change the base?

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

Conversation

johnbillion commented Jan 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High level changes:

Details and todos:

Plugin authors:

Other query classes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kkataria3010 May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterwilsoncc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spacedmonkey commented Jul 18, 2023

Uh oh!

spacedmonkey commented Jul 18, 2023

Uh oh!

archon810 commented Nov 9, 2023

Uh oh!

mibmo commented May 31, 2024

Uh oh!

github-actions bot commented May 31, 2024

Unlinked Accounts

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

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

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

johnbillion commented Jan 18, 2023 •

edited

Loading

kkataria3010 May 16, 2025 •

edited

Loading