Skip to content

#47280 - Remove SQL_CALC_FOUND_ROWS and lazy load found_rows#2119

Closed
johnbillion wants to merge 82 commits into
WordPress:trunkfrom
johnbillion:47280
Closed

#47280 - Remove SQL_CALC_FOUND_ROWS and lazy load found_rows#2119
johnbillion wants to merge 82 commits into
WordPress:trunkfrom
johnbillion:47280

Conversation

@johnbillion
Copy link
Copy Markdown
Member

@johnbillion johnbillion commented Jan 6, 2022

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

This continues the work of @morgo on #330.

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
  • Retains usage of SELECT FOUND_ROWS() only as a fallback if the query is filtered and a SQL_CALC_FOUND_ROWS modifier inserted
  • Lazily loads the found_posts and max_num_pages property values, thus avoiding the need to run the count query at all if it's not needed

Details and todos:

  • Convert found_posts and max_num_pages into private properties and then expose them via __get() which lazily populates (and caches) their values
  • Tests for basic queries
  • Tests for empty queries
  • Tests for queries with no results
  • Tests for queries with and without limits and paging
  • Tests for queries with and without no_found_rows
  • Tests for meta queries
  • Tests for taxonomy queries
  • Account for PHP serialization and JSON serialization of the WP_Query object -- the newly private properties still need to be included
    • Tests for serialization both before and after property access
  • Investigate uses of the found_posts_query filter: https://wpdirectory.net/search/01G2JTDXB9WQ0H1Y4P0VQSFBE3
    • Very low usage overall, most common usage is to no-op the query
    • If the query gets no-oped there's no change to the current behaviour
    • Zero instances of plugins enforcing SELECT FOUND_ROWS()
  • Decide how to handle a change to the query via the posts_request filter (the only filter that runs between the query being constructed and executed) https://wpdirectory.net/search/01G695ZX9RZ03PCDN3C0HFV6A1
    • If it's overridden and still contains SQL_CALC_FOUND_ROWS, need to retain the use of an immediate call to SELECT FOUND_ROWS().
      • Tests
      • Call _deprecated_argument()
    • If any other substring replacement is applied, the result of the count query could be inaccurate. Need to find examples.
      • "ACF: Better Search" filters posts_request to add a DISTINCT clause
  • Determine if the DISTINCT clause can be removed from the count query
    • DISTINCT is not used by core but could be used by plugins via the various clause filters
    • GROUP BY can be triggered by core, eg via a meta query with two or more clauses, an OR relation, and resulting posts that match more than one meta clause -- tests have been added to cover this
    • Therefore, the DISTINCT clause in the COUNT() query should remain
    • Tests
  • Handle switch_to_blog() and restore_current_blog() being called between the query and the count
    • Tests
  • Allow found_posts and max_num_pages to be set via __set()
    • Tests
  • Investigate any situations in core where found_posts or max_num_pages are unnecessarily accessed
    • None found

Plugin authors:

If you're the maintainer of a plugin which makes changes to the way WordPress queries for posts or counts results, uses the found_posts_query filter, or makes unusual use of the found_posts and max_num_pages properties, 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

@johnbillion johnbillion self-assigned this Jan 6, 2022
@johnbillion johnbillion marked this pull request as draft January 6, 2022 23:17
Comment thread src/wp-includes/class-wp-query.php Outdated
@MahdiAkrami01

This comment was marked as off-topic.

@johnbillion
Copy link
Copy Markdown
Member Author

Closing this in favour of #3863

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants