From 7aabde974621efa0888353536bb9f6fecb798067 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 18 Jan 2023 20:10:46 +0000 Subject: [PATCH 1/8] Remove usage of deprecated MySQL `SQL_CALC_FOUND_ROWS`. --- src/wp-includes/class-wp-query.php | 49 +- tests/phpunit/tests/post/query.php | 521 ++++++++++++++++++++++ tests/phpunit/tests/query/noFoundRows.php | 29 +- 3 files changed, 586 insertions(+), 13 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index a6b105ef2877c..4b6d037ebb79a 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -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. * @@ -1975,7 +1983,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 { @@ -3044,20 +3052,19 @@ public function get_posts() { $limits = isset( $clauses['limits'] ) ? $clauses['limits'] : ''; } + $count_field = "{$wpdb->posts}.ID"; + if ( ! empty( $groupby ) ) { + $count_field = $groupby; + $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 @@ -3066,17 +3073,38 @@ public function get_posts() { "; $this->request = $old_request; + $this->count_request = " + SELECT COUNT(DISTINCT {$count_field}) + 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 ) ); + + if ( false !== strpos( $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.' ), + 'SQL_CALC_FOUND_ROWS', + 'SELECT FOUND_ROWS()' + ) + ); + + $this->count_request = 'SELECT FOUND_ROWS()'; + } } /** @@ -3242,7 +3270,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 @@ -3254,6 +3282,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. @@ -3522,11 +3551,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 + * 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 { diff --git a/tests/phpunit/tests/post/query.php b/tests/phpunit/tests/post/query.php index 66bcdc26915eb..c5a8934615933 100644 --- a/tests/phpunit/tests/post/query.php +++ b/tests/phpunit/tests/post/query.php @@ -777,4 +777,525 @@ public function test_found_posts_should_be_integer_even_if_found_posts_filter_re $this->assertIsInt( $q->found_posts ); } + + /** + * @ticket 47280 + */ + public function test_found_posts_are_correct_for_empty_query() { + self::factory()->post->create_many( 12 ); + + $q = new WP_Query(); + + $this->assertSame( 0, $q->post_count ); + $this->assertSame( 0, $q->found_posts ); + $this->assertSame( 0, $q->max_num_pages ); + } + + /** + * @ticket 47280 + * @dataProvider data_fields + * + * @param string $fields Value of the `fields` argument for `WP_Query`. + */ + public function test_found_posts_are_correct_for_query_with_no_results( $fields ) { + $q = new WP_Query( + array( + 'fields' => $fields, + 'posts_status' => 'draft', + ) + ); + + $this->assertSame( 0, $q->post_count, self::get_count_message( $q ) ); + $this->assertSame( 0, $q->found_posts, self::get_count_message( $q ) ); + $this->assertSame( 0, $q->max_num_pages, self::get_count_message( $q ) ); + } + + /** + * @ticket 47280 + * @dataProvider data_fields + * + * @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 ); + + $q = new WP_Query( + array( + 'fields' => $fields, + 'posts_per_page' => 2, + ) + ); + + $this->assertSame( 2, $q->post_count, self::get_count_message( $q ) ); + $this->assertSame( 5, $q->found_posts, self::get_count_message( $q ) ); + $this->assertEquals( 3, $q->max_num_pages, self::get_count_message( $q ) ); + } + + /** + * @ticket 47280 + * @dataProvider data_fields + * + * @param string $fields Value of the `fields` argument for `WP_Query`. + */ + public function test_found_posts_are_correct_for_query_with_no_limit( $fields ) { + self::factory()->post->create_many( 5 ); + + $q = new WP_Query( + array( + 'fields' => $fields, + 'posts_per_page' => -1, + ) + ); + + $this->assertSame( 5, $q->post_count, self::get_count_message( $q ) ); + $this->assertSame( 5, $q->found_posts, self::get_count_message( $q ) ); + // You would expect this to be 1 but historically it's 0 for posts without paging. + $this->assertSame( 0, $q->max_num_pages, self::get_count_message( $q ) ); + } + + /** + * @ticket 47280 + * @dataProvider data_fields + * + * @param string $fields Value of the `fields` argument for `WP_Query`. + */ + public function test_found_posts_are_correct_for_query_with_no_paging( $fields ) { + self::factory()->post->create_many( 5 ); + + $q = new WP_Query( + array( + 'fields' => $fields, + 'nopaging' => true, + ) + ); + + $this->assertSame( 5, $q->post_count, self::get_count_message( $q ) ); + $this->assertSame( 5, $q->found_posts, self::get_count_message( $q ) ); + // You would expect this to be 1 but historically it's 0 for posts without paging. + $this->assertSame( 0, $q->max_num_pages, self::get_count_message( $q ) ); + } + + /** + * @ticket 47280 + * @dataProvider data_fields + * + * @param string $fields Value of the `fields` argument for `WP_Query`. + */ + public function test_found_posts_are_correct_for_query_with_no_found_rows( $fields ) { + self::factory()->post->create_many( 5 ); + + $q = new WP_Query( + array( + 'fields' => $fields, + 'no_found_rows' => true, + ) + ); + + $this->assertSame( 5, $q->post_count, self::get_count_message( $q ) ); + $this->assertSame( 0, $q->found_posts, self::get_count_message( $q ) ); + $this->assertSame( 0, $q->max_num_pages, self::get_count_message( $q ) ); + } + + /** + * @ticket 47280 + * @dataProvider data_fields + * + * @param string $fields Value of the `fields` argument for `WP_Query`. + */ + public function test_found_posts_are_correct_for_paged_query( $fields ) { + self::factory()->post->create_many( 5 ); + + $q = new WP_Query( + array( + 'fields' => $fields, + 'posts_per_page' => 2, + 'paged' => 3, + ) + ); + + $this->assertSame( 1, $q->post_count, self::get_count_message( $q ) ); + $this->assertSame( 5, $q->found_posts, self::get_count_message( $q ) ); + $this->assertEquals( 3, $q->max_num_pages, self::get_count_message( $q ) ); + } + + /** + * @ticket 47280 + * @dataProvider data_fields + * + * @param string $fields Value of the `fields` argument for `WP_Query`. + */ + public function test_found_posts_are_correct_for_author_queries( $fields ) { + $author = self::factory()->user->create(); + self::factory()->post->create_many( 5 ); + self::factory()->post->create_many( + 5, + array( + 'post_author' => $author, + ) + ); + + $q = new WP_Query( + array( + 'fields' => $fields, + 'posts_per_page' => 2, + 'author' => $author, + ) + ); + + $this->assertSame( 2, $q->post_count, self::get_count_message( $q ) ); + $this->assertSame( 5, $q->found_posts, self::get_count_message( $q ) ); + $this->assertEquals( 3, $q->max_num_pages, self::get_count_message( $q ) ); + } + + /** + * A query for the following triggers an additional LEFT JOIN on `wp_posts`: + * + * - Custom taxonomy query + * - `post_status` specified + * - `post_type` not specified + * + * @ticket 47280 + * @dataProvider data_fields + * + * @param string $fields Value of the `fields` argument for `WP_Query`. + */ + public function test_found_posts_are_correct_for_query_that_performs_post_status_join( $fields ) { + $taxonomy = 'post_status_join_tax'; + register_taxonomy( $taxonomy, 'post' ); + $term = self::factory()->term->create_and_get( + array( + 'taxonomy' => $taxonomy, + ) + ); + self::factory()->post->create_many( 5 ); + $ids = self::factory()->post->create_many( 5 ); + + foreach ( $ids as $id ) { + wp_set_post_terms( $id, $term->slug, $taxonomy ); + } + + $q = new WP_Query( + array( + 'fields' => $fields, + 'posts_per_page' => 2, + 'taxonomy' => $taxonomy, + 'term' => $term->slug, + 'post_status' => 'publish', + ) + ); + + $this->assertSame( 2, $q->post_count, self::get_count_message( $q ) ); + $this->assertSame( 5, $q->found_posts, self::get_count_message( $q ) ); + $this->assertEquals( 3, $q->max_num_pages, self::get_count_message( $q ) ); + } + + /** + * @ticket 47280 + * @dataProvider data_fields + * + * @param string $fields Value of the `fields` argument for `WP_Query`. + */ + public function test_found_posts_are_correct_for_tax_queries( $fields ) { + $term = self::factory()->term->create_and_get(); + self::factory()->post->create_many( 5 ); + $ids = self::factory()->post->create_many( 5 ); + + foreach ( $ids as $id ) { + wp_set_post_terms( $id, $term->slug ); + } + + $q = new WP_Query( + array( + 'fields' => $fields, + 'posts_per_page' => 2, + 'tag' => $term->slug, + ) + ); + + $this->assertSame( 2, $q->post_count, self::get_count_message( $q ) ); + $this->assertSame( 5, $q->found_posts, self::get_count_message( $q ) ); + $this->assertEquals( 3, $q->max_num_pages, self::get_count_message( $q ) ); + } + + /** + * @ticket 47280 + * @dataProvider data_fields + * + * @param string $fields Value of the `fields` argument for `WP_Query`. + */ + public function test_found_posts_are_correct_for_meta_queries( $fields ) { + self::factory()->post->create_many( 5 ); + $ids = self::factory()->post->create_many( 5 ); + + foreach ( $ids as $id ) { + add_post_meta( $id, 'my_meta', 'foo' ); + } + + $q = new WP_Query( + array( + 'fields' => $fields, + 'posts_per_page' => 2, + 'meta_key' => 'my_meta', + ) + ); + + $this->assertSame( 2, $q->post_count, self::get_count_message( $q ) ); + $this->assertSame( 5, $q->found_posts, self::get_count_message( $q ) ); + $this->assertEquals( 3, $q->max_num_pages, self::get_count_message( $q ) ); + } + + /** + * @ticket 47280 + * @dataProvider data_fields + * + * @param string $fields Value of the `fields` argument for `WP_Query`. + */ + public function test_found_posts_are_correct_for_multiple_meta_queries( $fields ) { + self::factory()->post->create_many( 5 ); + $ids = self::factory()->post->create_many( 5 ); + + foreach ( $ids as $id ) { + add_post_meta( $id, 'field_1', 'foo' ); + add_post_meta( $id, 'field_2', 'bar' ); + } + + $q = new WP_Query( + array( + 'fields' => $fields, + 'posts_per_page' => 2, + 'meta_query' => array( + array( + 'key' => 'field_1', + ), + array( + 'key' => 'field_2', + ), + ), + ) + ); + + $this->assertSame( 2, $q->post_count, self::get_count_message( $q ) ); + $this->assertSame( 5, $q->found_posts, self::get_count_message( $q ) ); + $this->assertEquals( 3, $q->max_num_pages, self::get_count_message( $q ) ); + } + + /** + * @ticket 47280 + * @dataProvider data_fields + * + * @param string $fields Value of the `fields` argument for `WP_Query`. + */ + public function test_found_posts_are_correct_for_OR_meta_queries( $fields ) { + self::factory()->post->create_many( 5 ); + $ids = self::factory()->post->create_many( 5 ); + + // Add a mixture of meta values so all 5 posts match the meta query. + add_post_meta( $ids[0], 'field_1', 'foo' ); + add_post_meta( $ids[0], 'field_2', 'bar' ); + add_post_meta( $ids[1], 'field_1', 'foo' ); + add_post_meta( $ids[2], 'field_1', 'foo' ); + add_post_meta( $ids[3], 'field_2', 'bar' ); + add_post_meta( $ids[4], 'field_2', 'bar' ); + + // This query results in a `GROUP BY wp_posts.ID` clause, which means the + // count query must count `DISTINCT wp_posts.ID` to eliminate duplicates. + $q = new WP_Query( + array( + 'fields' => $fields, + 'posts_per_page' => 2, + 'meta_query' => array( + 'relation' => 'OR', + array( + 'key' => 'field_1', + ), + array( + 'key' => 'field_2', + ), + ), + ) + ); + + $this->assertSame( 2, $q->post_count, self::get_count_message( $q ) ); + $this->assertSame( 5, $q->found_posts, self::get_count_message( $q ) ); + $this->assertEquals( 3, $q->max_num_pages, self::get_count_message( $q ) ); + } + + /** + * @ticket 47280 + * @dataProvider data_fields + * + * @param string $fields Value of the `fields` argument for `WP_Query`. + */ + public function test_found_posts_are_correct_for_group_by_queries( $fields ) { + $user1 = self::factory()->user->create(); + $user2 = self::factory()->user->create(); + + self::factory()->post->create_many( + 5, + array( + 'post_author' => $user1, + ) + ); + self::factory()->post->create_many( + 5, + array( + 'post_author' => $user2, + ) + ); + + /** + * Adds a GROUP BY clause to the query. + */ + add_filter( + 'posts_groupby_request', + function() { + return "{$GLOBALS['wpdb']->posts}.post_author"; + } + ); + + // This query results in a `GROUP BY wp_posts.post_author` clause, which means the + // count query must count `DISTINCT wp_posts.post_author` to eliminate duplicates. + $q = new WP_Query( + array( + 'fields' => $fields, + 'posts_per_page' => 2, + ) + ); + + $this->assertSame( 2, $q->post_count, self::get_count_message( $q ) ); + // There is a total of two distinct authors in the results. + $this->assertSame( 2, $q->found_posts, self::get_count_message( $q ) ); + $this->assertEquals( 1, $q->max_num_pages, self::get_count_message( $q ) ); + } + + /** + * @ticket 47280 + * @group ms-required + * @dataProvider data_fields + * + * @param string $fields Value of the `fields` argument for `WP_Query`. + */ + public function test_found_posts_are_correct_when_switching_between_sites( $fields ) { + $blog = self::factory()->blog->create(); + self::factory()->post->create_many( 5 ); + + $q = new WP_Query( + array( + 'fields' => $fields, + 'posts_per_page' => 2, + ) + ); + + // Switch to another site. + switch_to_blog( $blog ); + + // Count the posts from the original site. This works because the SQL query + // and its table names has already been formed during the original query. + $post_count = $q->post_count; + $found_posts = $q->found_posts; + $max_num_pages = $q->max_num_pages; + + // Switch back. + restore_current_blog(); + + $this->assertSame( 2, $post_count, self::get_count_message( $q ) ); + $this->assertSame( 5, $found_posts, self::get_count_message( $q ) ); + $this->assertEquals( 3, $max_num_pages, self::get_count_message( $q ) ); + } + + /** + * @ticket 47280 + * @expectedDeprecated The posts_request filter + */ + public function test_posts_are_counted_with_select_found_rows_when_query_includes_sql_calc_found_rows() { + // Create five published posts. + self::factory()->post->create_many( 5 ); + // Create ten draft posts. + self::factory()->post->create_many( + 10, + array( + 'post_status' => 'draft', + ) + ); + + add_filter( + 'posts_request', + static function( $request ) { + global $wpdb; + + return " + SELECT SQL_CALC_FOUND_ROWS {$wpdb->posts}.ID + FROM {$wpdb->posts} + WHERE 1=1 + AND {$wpdb->posts}.post_type = 'post' + AND {$wpdb->posts}.post_status = 'draft' + ORDER BY {$wpdb->posts}.post_date + DESC LIMIT 0, 2 + "; + } + ); + + $q = new WP_Query( + array( + 'posts_per_page' => 2, + ) + ); + + // These results should now reflect the results for draft posts, as set by the filter. + $this->assertStringContainsString( 'SELECT FOUND_ROWS()', $q->count_request ); + $this->assertSame( 2, $q->post_count, self::get_count_message( $q ) ); + $this->assertSame( 10, $q->found_posts, self::get_count_message( $q ) ); + $this->assertEquals( 5, $q->max_num_pages, self::get_count_message( $q ) ); + } + + /** + * Data provider for tests which need to run once for each possible value of the fields argument. + * + * @return array Test data. + */ + public function data_fields() { + return array( + 'posts' => array( + '', + ), + 'ids' => array( + 'ids', + ), + 'parents' => array( + 'id=>parent', + ), + ); + } + + /** + * Helper method which returns a readable representation of the SQL queries performed. + * + * @param WP_Query $query The current query instance. + * @return string The formatted message. + */ + protected static function get_count_message( WP_Query $query ) { + return sprintf( + "Request SQL:\n\n%s\n\nCount SQL:\n\n%s\n", + self::format_sql( $query->request ), + self::format_sql( $query->count_request ) + ); + } + + /** + * Applies some basic formatting to an SQL query to make it more readable during a test failure. + * + * @param string $sql The SQL query to be formatted. + * @return string The formatted SQL query. + */ + protected static function format_sql( $sql ) { + $sql = preg_replace( + '# (FROM|INNER JOIN|LEFT JOIN|ON|WHERE|AND|GROUP BY|ORDER BY|LIMIT) #', + "\n\$1 ", + $sql + ); + $sql = preg_replace( '#^\t+#m', '', $sql ); + + return $sql; + } + } diff --git a/tests/phpunit/tests/query/noFoundRows.php b/tests/phpunit/tests/query/noFoundRows.php index 7b4f66a416030..39544de91358f 100644 --- a/tests/phpunit/tests/query/noFoundRows.php +++ b/tests/phpunit/tests/query/noFoundRows.php @@ -4,6 +4,10 @@ * @group query */ class Tests_Query_NoFoundRows extends WP_UnitTestCase { + + /** + * @ticket 47280 + */ public function test_no_found_rows_default() { $q = new WP_Query( array( @@ -11,9 +15,12 @@ public function test_no_found_rows_default() { ) ); - $this->assertStringContainsString( 'SQL_CALC_FOUND_ROWS', $q->request ); + $this->assertStringNotContainsString( 'SQL_CALC_FOUND_ROWS', $q->request ); } + /** + * @ticket 47280 + */ public function test_no_found_rows_false() { $q = new WP_Query( array( @@ -22,9 +29,12 @@ public function test_no_found_rows_false() { ) ); - $this->assertStringContainsString( 'SQL_CALC_FOUND_ROWS', $q->request ); + $this->assertStringNotContainsString( 'SQL_CALC_FOUND_ROWS', $q->request ); } + /** + * @ticket 47280 + */ public function test_no_found_rows_0() { $q = new WP_Query( array( @@ -33,9 +43,12 @@ public function test_no_found_rows_0() { ) ); - $this->assertStringContainsString( 'SQL_CALC_FOUND_ROWS', $q->request ); + $this->assertStringNotContainsString( 'SQL_CALC_FOUND_ROWS', $q->request ); } + /** + * @ticket 47280 + */ public function test_no_found_rows_empty_string() { $q = new WP_Query( array( @@ -44,9 +57,12 @@ public function test_no_found_rows_empty_string() { ) ); - $this->assertStringContainsString( 'SQL_CALC_FOUND_ROWS', $q->request ); + $this->assertStringNotContainsString( 'SQL_CALC_FOUND_ROWS', $q->request ); } + /** + * @ticket 47280 + */ public function test_no_found_rows_true() { $q = new WP_Query( array( @@ -58,6 +74,9 @@ public function test_no_found_rows_true() { $this->assertStringNotContainsString( 'SQL_CALC_FOUND_ROWS', $q->request ); } + /** + * @ticket 47280 + */ public function test_no_found_rows_non_bool_cast_to_true() { $q = new WP_Query( array( @@ -71,6 +90,7 @@ public function test_no_found_rows_non_bool_cast_to_true() { /** * @ticket 29552 + * @ticket 47280 */ public function test_no_found_rows_default_with_nopaging_true() { $p = self::factory()->post->create(); @@ -88,6 +108,7 @@ public function test_no_found_rows_default_with_nopaging_true() { /** * @ticket 29552 + * @ticket 47280 */ public function test_no_found_rows_default_with_postsperpage_minus1() { $p = self::factory()->post->create(); From 7888988cbb5afe5b7c54995f8d6afb13dfff0740 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 22 Feb 2023 16:18:46 +0000 Subject: [PATCH 2/8] Apply suggestions from code review Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com> --- src/wp-includes/class-wp-query.php | 2 +- tests/phpunit/tests/post/query.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 4b6d037ebb79a..e22f1a3c935b7 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -3091,7 +3091,7 @@ public function get_posts() { */ $this->request = apply_filters_ref_array( 'posts_request', array( $this->request, &$this ) ); - if ( false !== strpos( $this->request, 'SQL_CALC_FOUND_ROWS' ) ) { + if ( str_contains( $this->request, 'SQL_CALC_FOUND_ROWS' ) ) { _deprecated_argument( 'The posts_request filter', 'x.x.x', diff --git a/tests/phpunit/tests/post/query.php b/tests/phpunit/tests/post/query.php index c5a8934615933..8aa0e409d78ea 100644 --- a/tests/phpunit/tests/post/query.php +++ b/tests/phpunit/tests/post/query.php @@ -1251,7 +1251,7 @@ static function( $request ) { /** * Data provider for tests which need to run once for each possible value of the fields argument. * - * @return array Test data. + * @return array[] Test data. */ public function data_fields() { return array( From 7d59ed98d345141881a459c38549f0ae463b0356 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 22 Feb 2023 17:54:29 +0000 Subject: [PATCH 3/8] Update tests/phpunit/tests/post/query.php Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com> --- tests/phpunit/tests/post/query.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/phpunit/tests/post/query.php b/tests/phpunit/tests/post/query.php index 8aa0e409d78ea..0e78928380a05 100644 --- a/tests/phpunit/tests/post/query.php +++ b/tests/phpunit/tests/post/query.php @@ -786,9 +786,9 @@ public function test_found_posts_are_correct_for_empty_query() { $q = new WP_Query(); - $this->assertSame( 0, $q->post_count ); - $this->assertSame( 0, $q->found_posts ); - $this->assertSame( 0, $q->max_num_pages ); + $this->assertSame( 0, $q->post_count, 'Post count is expected to be zero' ); + $this->assertSame( 0, $q->found_posts, 'Total found posts is expected to be zero' ); + $this->assertSame( 0, $q->max_num_pages, 'Number of pages is expected to be zero' ); } /** From a830ea7777bcad296af5c30a740ed5a45f324a95 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 22 Feb 2023 21:12:03 +0100 Subject: [PATCH 4/8] Remove unnecessary variable assignment. --- tests/phpunit/tests/post/query.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/phpunit/tests/post/query.php b/tests/phpunit/tests/post/query.php index 0e78928380a05..b5a02a7f9b2a1 100644 --- a/tests/phpunit/tests/post/query.php +++ b/tests/phpunit/tests/post/query.php @@ -748,7 +748,7 @@ public function test_set_found_posts_not_posts_as_an_array( $posts, $expected ) * @ticket 42469 */ public function test_found_posts_should_be_integer_not_string() { - $post_id = self::factory()->post->create(); + self::factory()->post->create(); $q = new WP_Query( array( @@ -763,7 +763,7 @@ public function test_found_posts_should_be_integer_not_string() { * @ticket 42469 */ public function test_found_posts_should_be_integer_even_if_found_posts_filter_returns_string_value() { - $post_id = self::factory()->post->create(); + self::factory()->post->create(); add_filter( 'found_posts', '__return_empty_string' ); From 991f0afd57e4d112c0d6d281aa70bf048d7d5489 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 22 Feb 2023 21:12:13 +0100 Subject: [PATCH 5/8] Better formatting for these docs. --- tests/phpunit/tests/post/query.php | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/phpunit/tests/post/query.php b/tests/phpunit/tests/post/query.php index b5a02a7f9b2a1..0aa8d01a3bd1e 100644 --- a/tests/phpunit/tests/post/query.php +++ b/tests/phpunit/tests/post/query.php @@ -1097,8 +1097,10 @@ public function test_found_posts_are_correct_for_OR_meta_queries( $fields ) { add_post_meta( $ids[3], 'field_2', 'bar' ); add_post_meta( $ids[4], 'field_2', 'bar' ); - // This query results in a `GROUP BY wp_posts.ID` clause, which means the - // count query must count `DISTINCT wp_posts.ID` to eliminate duplicates. + /* + * This query results in a `GROUP BY wp_posts.ID` clause, which means the + * count query must count `DISTINCT wp_posts.ID` to eliminate duplicates. + */ $q = new WP_Query( array( 'fields' => $fields, @@ -1153,8 +1155,10 @@ function() { } ); - // This query results in a `GROUP BY wp_posts.post_author` clause, which means the - // count query must count `DISTINCT wp_posts.post_author` to eliminate duplicates. + /* + * This query results in a `GROUP BY wp_posts.post_author` clause, which means the + * count query must count `DISTINCT wp_posts.post_author` to eliminate duplicates. + */ $q = new WP_Query( array( 'fields' => $fields, @@ -1189,8 +1193,10 @@ public function test_found_posts_are_correct_when_switching_between_sites( $fiel // Switch to another site. switch_to_blog( $blog ); - // Count the posts from the original site. This works because the SQL query - // and its table names has already been formed during the original query. + /* + * Count the posts from the original site. This works because the SQL query + * and its table names has already been formed during the original query. + */ $post_count = $q->post_count; $found_posts = $q->found_posts; $max_num_pages = $q->max_num_pages; From f5b240d1ef5431131f2ed4468d43ce333e70369c Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 22 Feb 2023 21:13:26 +0100 Subject: [PATCH 6/8] Add some invisible bytes. --- tests/phpunit/tests/post/query.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/phpunit/tests/post/query.php b/tests/phpunit/tests/post/query.php index 0aa8d01a3bd1e..998f070743a9e 100644 --- a/tests/phpunit/tests/post/query.php +++ b/tests/phpunit/tests/post/query.php @@ -793,6 +793,7 @@ public function test_found_posts_are_correct_for_empty_query() { /** * @ticket 47280 + * * @dataProvider data_fields * * @param string $fields Value of the `fields` argument for `WP_Query`. @@ -812,6 +813,7 @@ public function test_found_posts_are_correct_for_query_with_no_results( $fields /** * @ticket 47280 + * * @dataProvider data_fields * * @param string $fields Value of the `fields` argument for `WP_Query`. @@ -833,6 +835,7 @@ public function test_found_posts_are_correct_for_basic_query( $fields ) { /** * @ticket 47280 + * * @dataProvider data_fields * * @param string $fields Value of the `fields` argument for `WP_Query`. @@ -855,6 +858,7 @@ public function test_found_posts_are_correct_for_query_with_no_limit( $fields ) /** * @ticket 47280 + * * @dataProvider data_fields * * @param string $fields Value of the `fields` argument for `WP_Query`. @@ -877,6 +881,7 @@ public function test_found_posts_are_correct_for_query_with_no_paging( $fields ) /** * @ticket 47280 + * * @dataProvider data_fields * * @param string $fields Value of the `fields` argument for `WP_Query`. @@ -898,6 +903,7 @@ public function test_found_posts_are_correct_for_query_with_no_found_rows( $fiel /** * @ticket 47280 + * * @dataProvider data_fields * * @param string $fields Value of the `fields` argument for `WP_Query`. @@ -920,6 +926,7 @@ public function test_found_posts_are_correct_for_paged_query( $fields ) { /** * @ticket 47280 + * * @dataProvider data_fields * * @param string $fields Value of the `fields` argument for `WP_Query`. @@ -955,6 +962,7 @@ public function test_found_posts_are_correct_for_author_queries( $fields ) { * - `post_type` not specified * * @ticket 47280 + * * @dataProvider data_fields * * @param string $fields Value of the `fields` argument for `WP_Query`. @@ -991,6 +999,7 @@ public function test_found_posts_are_correct_for_query_that_performs_post_status /** * @ticket 47280 + * * @dataProvider data_fields * * @param string $fields Value of the `fields` argument for `WP_Query`. @@ -1019,6 +1028,7 @@ public function test_found_posts_are_correct_for_tax_queries( $fields ) { /** * @ticket 47280 + * * @dataProvider data_fields * * @param string $fields Value of the `fields` argument for `WP_Query`. @@ -1046,6 +1056,7 @@ public function test_found_posts_are_correct_for_meta_queries( $fields ) { /** * @ticket 47280 + * * @dataProvider data_fields * * @param string $fields Value of the `fields` argument for `WP_Query`. @@ -1081,6 +1092,7 @@ public function test_found_posts_are_correct_for_multiple_meta_queries( $fields /** * @ticket 47280 + * * @dataProvider data_fields * * @param string $fields Value of the `fields` argument for `WP_Query`. @@ -1124,6 +1136,7 @@ public function test_found_posts_are_correct_for_OR_meta_queries( $fields ) { /** * @ticket 47280 + * * @dataProvider data_fields * * @param string $fields Value of the `fields` argument for `WP_Query`. @@ -1175,6 +1188,7 @@ function() { /** * @ticket 47280 * @group ms-required + * * @dataProvider data_fields * * @param string $fields Value of the `fields` argument for `WP_Query`. @@ -1211,6 +1225,7 @@ public function test_found_posts_are_correct_when_switching_between_sites( $fiel /** * @ticket 47280 + * * @expectedDeprecated The posts_request filter */ public function test_posts_are_counted_with_select_found_rows_when_query_includes_sql_calc_found_rows() { From 7ef5d419a2bd9d9e5c2b796fcf31723bc1a40721 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Sun, 21 May 2023 02:26:01 +0100 Subject: [PATCH 7/8] Misc tweaks. --- tests/phpunit/tests/post/query.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/phpunit/tests/post/query.php b/tests/phpunit/tests/post/query.php index 44303eea44b80..3ed169794f655 100644 --- a/tests/phpunit/tests/post/query.php +++ b/tests/phpunit/tests/post/query.php @@ -1222,11 +1222,13 @@ public function test_found_posts_are_correct_when_switching_between_sites( $fiel } /** + * Ensures the count is correct when the query includes an `SQL_CALC_FOUND_ROWS` modifier. + * * @ticket 47280 * * @expectedDeprecated The posts_request filter */ - public function test_posts_are_counted_with_select_found_rows_when_query_includes_sql_calc_found_rows() { + public function test_posts_are_counted_with_select_found_rows_when_query_includes_sql_calc_found_rows_modifier() { // Create five published posts. self::factory()->post->create_many( 5 ); // Create ten draft posts. @@ -1261,7 +1263,7 @@ static function( $request ) { ); // These results should now reflect the results for draft posts, as set by the filter. - $this->assertStringContainsString( 'SELECT FOUND_ROWS()', $q->count_request ); + $this->assertStringContainsString( 'SELECT FOUND_ROWS()', $q->count_request, self::get_count_message( $q ) ); $this->assertSame( 2, $q->post_count, self::get_count_message( $q ) ); $this->assertSame( 10, $q->found_posts, self::get_count_message( $q ) ); $this->assertEquals( 5, $q->max_num_pages, self::get_count_message( $q ) ); @@ -1293,10 +1295,12 @@ public function data_fields() { * @return string The formatted message. */ protected static function get_count_message( WP_Query $query ) { + global $wpdb; + return sprintf( - "Request SQL:\n\n%s\n\nCount SQL:\n\n%s\n", - self::format_sql( $query->request ), - self::format_sql( $query->count_request ) + "Request SQL:\n%s\nCount SQL:\n\n%s\n", + self::format_sql( $wpdb->remove_placeholder_escape( $query->request ) ), + self::format_sql( $wpdb->remove_placeholder_escape( $query->count_request ) ) ); } From 18b12fb420050e925a7016ac7cd52f8de865bcf1 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Sat, 8 Jul 2023 01:39:44 +0200 Subject: [PATCH 8/8] This isn't guaranteed to be a string (at least one plugin in the wild returns boolean false). --- src/wp-includes/class-wp-query.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 9875e467ac001..f63d7952cb94c 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -3129,7 +3129,7 @@ public function get_posts() { */ $this->request = apply_filters_ref_array( 'posts_request', array( $this->request, &$this ) ); - if ( str_contains( $this->request, 'SQL_CALC_FOUND_ROWS' ) ) { + if ( is_string( $this->request ) && str_contains( $this->request, 'SQL_CALC_FOUND_ROWS' ) ) { _deprecated_argument( 'The posts_request filter', 'x.x.x',