From a9a00611056ec9daefa122274029cee6d2a9ee38 Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 15 Oct 2025 13:27:53 +1100 Subject: [PATCH 01/36] Enhance WP_Query ordering to ensure deterministic results by adding ID as a secondary sort field. This change addresses potential duplicate records across pages when multiple posts share the same value for a field. A list of fields requiring deterministic ordering has been introduced to improve query consistency --- src/wp-includes/class-wp-query.php | 63 ++++++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 3 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 9385ae832ff66..4ec396409f653 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -1892,6 +1892,7 @@ public function set( $query_var, $value ) { * database query. * * @since 1.5.0 + * @since x.x.x Adds deterministic ordering to prevent duplicate records across pages. * * @global wpdb $wpdb WordPress database abstraction object. * @@ -2513,12 +2514,45 @@ public function get_posts() { if ( isset( $query_vars['orderby'] ) && ( is_array( $query_vars['orderby'] ) || false === $query_vars['orderby'] ) ) { $orderby = ''; } else { - $orderby = "{$wpdb->posts}.post_date " . $query_vars['order']; + /* + * Ensure deterministic ordering to prevent duplicate records across pages. + * When multiple posts have the same value for a field, add ID as secondary sort to guarantee consistent ordering. + * Note: this is to circumvent a bug that is currently being tracked in https://core.trac.wordpress.org/ticket/44349. + */ + $orderby = "{$wpdb->posts}.post_date " . $query_vars['order'] . ', ' . "{$wpdb->posts}.ID " . $query_vars['order']; } } elseif ( 'none' === $query_vars['orderby'] ) { $orderby = ''; } else { - $orderby_array = array(); + /* + * Ensure deterministic ordering to prevent duplicate records across pages. + * When multiple posts have the same value for a field, add ID as secondary sort to guarantee consistent ordering. + * Note: this is to circumvent a bug that is currently being tracked in https://core.trac.wordpress.org/ticket/44349. + */ + $fields_requiring_deterministic_orderby = array( + 'post_name', + 'post_author', + 'post_date', + 'post_title', + 'post_modified', + 'post_mime_type', + 'post_parent', + 'post_type', + 'name', + 'author', + 'date', + 'title', + 'modified', + 'parent', + 'type', + 'menu_order', + 'comment_count', + ); + + $orderby_array = array(); + $needs_deterministic_orderby = false; + $has_id_orderby = false; + if ( is_array( $query_vars['orderby'] ) ) { foreach ( $query_vars['orderby'] as $_orderby => $order ) { $orderby = wp_slash( urldecode( $_orderby ) ); @@ -2529,7 +2563,20 @@ public function get_posts() { } $orderby_array[] = $parsed . ' ' . $this->parse_order( $order ); + + // Check if this field needs deterministic ordering + if ( in_array( $_orderby, $fields_requiring_deterministic_orderby, true ) ) { + $needs_deterministic_orderby = true; + } elseif ( 'ID' === $_orderby ) { + $has_id_orderby = true; + } + } + + // Add ID as tie-breaker if needed and not already present + if ( $needs_deterministic_orderby && ! $has_id_orderby ) { + $orderby_array[] = "{$wpdb->posts}.ID " . $query_vars['order']; } + $orderby = implode( ', ', $orderby_array ); } else { @@ -2544,11 +2591,21 @@ public function get_posts() { } $orderby_array[] = $parsed; + + // Check if this field needs deterministic ordering + if ( in_array( $orderby, $fields_requiring_deterministic_orderby, true ) ) { + $needs_deterministic_orderby = true; + } elseif ( 'ID' === $orderby ) { + $has_id_orderby = true; + } } $orderby = implode( ' ' . $query_vars['order'] . ', ', $orderby_array ); if ( empty( $orderby ) ) { - $orderby = "{$wpdb->posts}.post_date " . $query_vars['order']; + $orderby = "{$wpdb->posts}.post_date " . $query_vars['order'] . ', ' . "{$wpdb->posts}.ID " . $query_vars['order']; + } elseif ( $needs_deterministic_orderby && ! $has_id_orderby ) { + // Add ID as tie-breaker for deterministic ordering + $orderby .= ", {$wpdb->posts}.ID " . $query_vars['order']; } elseif ( ! empty( $query_vars['order'] ) ) { $orderby .= " {$query_vars['order']}"; } From 1131f240437794b7bc65af175b5341cf01cb6ae2 Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 15 Oct 2025 13:48:38 +1100 Subject: [PATCH 02/36] WHITESPACE! Oh no! --- 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 4ec396409f653..ef648f1c99f83 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -2532,7 +2532,7 @@ public function get_posts() { $fields_requiring_deterministic_orderby = array( 'post_name', 'post_author', - 'post_date', + 'post_date', 'post_title', 'post_modified', 'post_mime_type', From af63e0ef9030bbbaa3ea3de967cb7d59e3f95693 Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 22 Oct 2025 09:21:38 +1100 Subject: [PATCH 03/36] Refactor WP_Query ordering logic to ensure consistent results by appending ID as a secondary sort field. Update related unit tests to reflect changes in expected SQL output for various orderby scenarios --- src/wp-includes/class-wp-query.php | 5 ++--- .../tests/admin/wpPrivacyRequestsTable.php | 20 ++++++++++++------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index ef648f1c99f83..5aa68d56694bb 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -2535,7 +2535,6 @@ public function get_posts() { 'post_date', 'post_title', 'post_modified', - 'post_mime_type', 'post_parent', 'post_type', 'name', @@ -2590,7 +2589,7 @@ public function get_posts() { continue; } - $orderby_array[] = $parsed; + $orderby_array[] = $parsed . ' ' . $query_vars['order']; // Check if this field needs deterministic ordering if ( in_array( $orderby, $fields_requiring_deterministic_orderby, true ) ) { @@ -2599,7 +2598,7 @@ public function get_posts() { $has_id_orderby = true; } } - $orderby = implode( ' ' . $query_vars['order'] . ', ', $orderby_array ); + $orderby = implode( ', ', $orderby_array ); if ( empty( $orderby ) ) { $orderby = "{$wpdb->posts}.post_date " . $query_vars['order'] . ', ' . "{$wpdb->posts}.ID " . $query_vars['order']; diff --git a/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php b/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php index 66e3e02501cfb..08eadb62e6bd3 100644 --- a/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php +++ b/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php @@ -99,7 +99,13 @@ public function test_columns_should_be_sortable( $order, $orderby, $search, $exp unset( $_REQUEST['orderby'] ); unset( $_REQUEST['s'] ); - $this->assertStringContainsString( "ORDER BY {$wpdb->posts}.{$expected}", $this->sql ); + $expected_query = explode( ', ', $expected ); + $expected_query = array_map( function( $item ) use ( $wpdb ) { + return "{$wpdb->posts}.{$item}"; + }, $expected_query ); + $expected_query = implode( ', ', $expected_query ); + + $this->assertStringContainsString( "ORDER BY {$expected_query}", $this->sql ); } /** @@ -136,42 +142,42 @@ public function data_columns_should_be_sortable() { 'order' => null, 'orderby' => null, 's' => null, - 'expected' => 'post_date DESC', + 'expected' => 'post_date DESC, ID DESC', ), // Default order (ID) DESC. array( 'order' => '', 'orderby' => '', 's' => '', - 'expected' => 'post_date DESC', + 'expected' => 'post_date DESC, ID DESC', ), // Order by requester (post_title) ASC. array( 'order' => 'ASC', 'orderby' => 'requester', 's' => '', - 'expected' => 'post_title ASC', + 'expected' => 'post_title ASC, ID ASC', ), // Order by requester (post_title) DESC. array( 'order' => 'DESC', 'orderby' => 'requester', 's' => null, - 'expected' => 'post_title DESC', + 'expected' => 'post_title DESC, ID DESC', ), // Order by requested (post_date) ASC. array( 'order' => 'ASC', 'orderby' => 'requested', 's' => null, - 'expected' => 'post_date ASC', + 'expected' => 'post_date ASC, ID ASC', ), // Order by requested (post_date) DESC. array( 'order' => 'DESC', 'orderby' => 'requested', 's' => null, - 'expected' => 'post_date DESC', + 'expected' => 'post_date DESC, ID DESC', ), // Search and order by relevance. array( From fcdb7267e6c50bddf664eccce4f90586e84a1028 Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 22 Oct 2025 10:37:04 +1100 Subject: [PATCH 04/36] Consolidate ID tie-breaker logic and ensure consistent SQL output in unit tests for orderby scenarios. --- src/wp-includes/class-wp-query.php | 28 ++++++++----------- .../tests/admin/wpPrivacyRequestsTable.php | 12 ++++---- 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 5aa68d56694bb..1eb025b2cf35e 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -2570,14 +2570,6 @@ public function get_posts() { $has_id_orderby = true; } } - - // Add ID as tie-breaker if needed and not already present - if ( $needs_deterministic_orderby && ! $has_id_orderby ) { - $orderby_array[] = "{$wpdb->posts}.ID " . $query_vars['order']; - } - - $orderby = implode( ', ', $orderby_array ); - } else { $query_vars['orderby'] = urldecode( $query_vars['orderby'] ); $query_vars['orderby'] = wp_slash( $query_vars['orderby'] ); @@ -2598,16 +2590,18 @@ public function get_posts() { $has_id_orderby = true; } } - $orderby = implode( ', ', $orderby_array ); + } - if ( empty( $orderby ) ) { - $orderby = "{$wpdb->posts}.post_date " . $query_vars['order'] . ', ' . "{$wpdb->posts}.ID " . $query_vars['order']; - } elseif ( $needs_deterministic_orderby && ! $has_id_orderby ) { - // Add ID as tie-breaker for deterministic ordering - $orderby .= ", {$wpdb->posts}.ID " . $query_vars['order']; - } elseif ( ! empty( $query_vars['order'] ) ) { - $orderby .= " {$query_vars['order']}"; - } + // Add ID as tie-breaker if needed and not already present + if ( $needs_deterministic_orderby && ! $has_id_orderby ) { + $orderby_array[] = "{$wpdb->posts}.ID " . $query_vars['order']; + } + + // Build the final orderby string + if ( empty( $orderby_array ) ) { + $orderby = "{$wpdb->posts}.post_date " . $query_vars['order'] . ', ' . "{$wpdb->posts}.ID " . $query_vars['order']; + } else { + $orderby = implode( ', ', $orderby_array ); } } diff --git a/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php b/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php index 08eadb62e6bd3..9d1374e25a3c8 100644 --- a/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php +++ b/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php @@ -100,12 +100,14 @@ public function test_columns_should_be_sortable( $order, $orderby, $search, $exp unset( $_REQUEST['s'] ); $expected_query = explode( ', ', $expected ); - $expected_query = array_map( function( $item ) use ( $wpdb ) { - return "{$wpdb->posts}.{$item}"; - }, $expected_query ); - $expected_query = implode( ', ', $expected_query ); + $expected_query = array_map( + function( $item ) use ( $wpdb ) { + return "{$wpdb->posts}.{$item}"; + }, + $expected_query + ); - $this->assertStringContainsString( "ORDER BY {$expected_query}", $this->sql ); + $this->assertStringContainsString( "ORDER BY " . implode( ', ', $expected_query ), $this->sql ); } /** From 0245f46ddb3850b42234c7cdb4543e93786b0511 Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 22 Oct 2025 10:40:51 +1100 Subject: [PATCH 05/36] whitespace in unit test --- tests/phpunit/tests/admin/wpPrivacyRequestsTable.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php b/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php index 9d1374e25a3c8..bc7f9d3ac93d7 100644 --- a/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php +++ b/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php @@ -101,13 +101,13 @@ public function test_columns_should_be_sortable( $order, $orderby, $search, $exp $expected_query = explode( ', ', $expected ); $expected_query = array_map( - function( $item ) use ( $wpdb ) { + function ( $item ) use ( $wpdb ) { return "{$wpdb->posts}.{$item}"; }, $expected_query ); - $this->assertStringContainsString( "ORDER BY " . implode( ', ', $expected_query ), $this->sql ); + $this->assertStringContainsString( 'ORDER BY ' . implode( ', ', $expected_query ), $this->sql ); } /** From eb78345f3e3aab028394fc2e0aa3859c38e8069f Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 22 Oct 2025 11:29:32 +1100 Subject: [PATCH 06/36] Refine WP_Query ordering logic to handle 'none' in orderby scenarios and ensure ID is consistently used as a tie-breaker. Update unit tests to reflect changes in expected SQL output for various orderby cases. --- src/wp-includes/class-wp-query.php | 10 +- .../tests/admin/wpPrivacyRequestsTable.php | 4 +- .../tests/query/deterministicOrdering.php | 291 ++++++++++++++++++ 3 files changed, 301 insertions(+), 4 deletions(-) create mode 100644 tests/phpunit/tests/query/deterministicOrdering.php diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 1eb025b2cf35e..c1ddb99095a42 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -2521,7 +2521,10 @@ public function get_posts() { */ $orderby = "{$wpdb->posts}.post_date " . $query_vars['order'] . ', ' . "{$wpdb->posts}.ID " . $query_vars['order']; } - } elseif ( 'none' === $query_vars['orderby'] ) { + // See get_pages(): when sort_column is 'none', the get_pages() function should not generate any ORDER BY clause. + // Should it rather be handled in the get_pages() function? + // src/wp-includes/post.php L6496 + } elseif ( 'none' === $query_vars['orderby'] || ( is_array( $query_vars['orderby'] ) && array_key_exists( 'none', $query_vars['orderby'] ) ) ) { $orderby = ''; } else { /* @@ -2551,6 +2554,7 @@ public function get_posts() { $orderby_array = array(); $needs_deterministic_orderby = false; $has_id_orderby = false; + $id_tie_breaker_order = $query_vars['order']; // Default to global order if ( is_array( $query_vars['orderby'] ) ) { foreach ( $query_vars['orderby'] as $_orderby => $order ) { @@ -2566,6 +2570,8 @@ public function get_posts() { // Check if this field needs deterministic ordering if ( in_array( $_orderby, $fields_requiring_deterministic_orderby, true ) ) { $needs_deterministic_orderby = true; + // Use the order from the array for ID tie-breaker + $id_tie_breaker_order = $this->parse_order( $order ); } elseif ( 'ID' === $_orderby ) { $has_id_orderby = true; } @@ -2594,7 +2600,7 @@ public function get_posts() { // Add ID as tie-breaker if needed and not already present if ( $needs_deterministic_orderby && ! $has_id_orderby ) { - $orderby_array[] = "{$wpdb->posts}.ID " . $query_vars['order']; + $orderby_array[] = "{$wpdb->posts}.ID " . $id_tie_breaker_order; } // Build the final orderby string diff --git a/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php b/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php index bc7f9d3ac93d7..1fc58f88e2059 100644 --- a/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php +++ b/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php @@ -193,14 +193,14 @@ public function data_columns_should_be_sortable() { 'order' => 'ASC', 'orderby' => 'requester', 's' => 'foo', - 'expected' => 'post_title ASC', + 'expected' => 'post_title ASC, ID ASC', ), // Search and order by requested (post_date) ASC. array( 'order' => 'ASC', 'orderby' => 'requested', 's' => 'foo', - 'expected' => 'post_date ASC', + 'expected' => 'post_date ASC, ID ASC', ), ); } diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php new file mode 100644 index 0000000000000..f7c81f63a21db --- /dev/null +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -0,0 +1,291 @@ +post->create( array( + 'post_title' => 'Post A', + 'post_date' => '2023-01-01 10:00:00', + ) ); + $post2 = self::factory()->post->create( array( + 'post_title' => 'Post B', + 'post_date' => '2023-01-01 10:00:00', // Same date as post1 + ) ); + $post3 = self::factory()->post->create( array( + 'post_title' => 'Post C', + 'post_date' => '2023-01-01 10:00:00', // Same date as post1 and post2 + ) ); + + // Test ordering by post_date (should add ID tie-breaker) + $query = new WP_Query( array( + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 10, + ) ); + + // Verify SQL contains ID as secondary sort + $this->assertStringContainsString( 'ORDER BY', $query->request ); + $this->assertStringContainsString( 'post_date ASC', $query->request ); + $this->assertStringContainsString( 'ID ASC', $query->request ); + $this->assertStringNotContainsString( 'ASC ASC', $query->request ); // No double ASC + } + + /** + * Test that deterministic ordering works with post_title. + * + * @ticket 44349 + */ + public function test_deterministic_ordering_with_post_title() { + // Create posts with same title to test deterministic ordering + $post1 = self::factory()->post->create( array( + 'post_title' => 'Same Title', + 'post_date' => '2023-01-01 10:00:00', + ) ); + $post2 = self::factory()->post->create( array( + 'post_title' => 'Same Title', // Same title as post1 + 'post_date' => '2023-01-01 11:00:00', + ) ); + + $query = new WP_Query( array( + 'orderby' => 'post_title', + 'order' => 'ASC', + 'posts_per_page' => 10, + ) ); + + // Verify SQL contains ID as secondary sort + $this->assertStringContainsString( 'post_title ASC', $query->request ); + $this->assertStringContainsString( 'ID ASC', $query->request ); + $this->assertStringNotContainsString( 'ASC ASC', $query->request ); + } + + /** + * Test that deterministic ordering works with DESC order. + * + * @ticket 44349 + */ + public function test_deterministic_ordering_with_desc_order() { + $query = new WP_Query( array( + 'orderby' => 'post_date', + 'order' => 'DESC', + 'posts_per_page' => 10, + ) ); + + // Verify SQL contains ID as secondary sort with DESC + $this->assertStringContainsString( 'post_date DESC', $query->request ); + $this->assertStringContainsString( 'ID DESC', $query->request ); + $this->assertStringNotContainsString( 'DESC DESC', $query->request ); + } + + /** + * Test that deterministic ordering works with array orderby. + * + * @ticket 44349 + */ + public function test_deterministic_ordering_with_array_orderby() { + $query = new WP_Query( array( + 'orderby' => array( + 'post_date' => 'ASC', + 'post_title' => 'ASC', + ), + 'posts_per_page' => 10, + ) ); + + // Verify SQL contains both fields with directions + $this->assertStringContainsString( 'post_date ASC', $query->request ); + $this->assertStringContainsString( 'post_title ASC', $query->request ); + $this->assertStringContainsString( 'ID ASC', $query->request ); + $this->assertStringNotContainsString( 'ASC ASC', $query->request ); + } + + /** + * Test that deterministic ordering doesn't add ID when ID is already present. + * + * @ticket 44349 + */ + public function test_deterministic_ordering_does_not_duplicate_id() { + $query = new WP_Query( array( + 'orderby' => 'ID', + 'order' => 'ASC', + 'posts_per_page' => 10, + ) ); + + // Should not add duplicate ID + $this->assertStringContainsString( 'ID ASC', $query->request ); + $this->assertStringNotContainsString( 'ID ASC, ID ASC', $query->request ); + } + + /** + * Test that deterministic ordering works with fields that don't need it. + * + * @ticket 44349 + */ + public function test_deterministic_ordering_with_non_deterministic_fields() { + $query = new WP_Query( array( + 'orderby' => 'rand', + 'posts_per_page' => 10, + ) ); + + // Should not add ID tie-breaker for rand + $this->assertStringContainsString( 'RAND()', $query->request ); + $this->assertStringNotContainsString( 'ID ASC', $query->request ); + } + + /** + * Test that deterministic ordering works with default ordering. + * + * @ticket 44349 + */ + public function test_deterministic_ordering_with_default_ordering() { + $query = new WP_Query( array( + 'posts_per_page' => 10, + ) ); + + // Default ordering should include ID tie-breaker + $this->assertStringContainsString( 'post_date DESC', $query->request ); + $this->assertStringContainsString( 'ID DESC', $query->request ); + $this->assertStringNotContainsString( 'DESC DESC', $query->request ); + } + + /** + * Test that deterministic ordering prevents duplicate records across pages. + * + * @ticket 44349 + */ + public function test_deterministic_ordering_prevents_duplicates_across_pages() { + // Create multiple posts with same post_date + $posts = array(); + for ( $i = 1; $i <= 10; $i++ ) { + $posts[] = self::factory()->post->create( array( + 'post_title' => "Post $i", + 'post_date' => '2023-01-01 10:00:00', // All same date + ) ); + } + + // Get first page + $query1 = new WP_Query( array( + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 5, + 'paged' => 1, + ) ); + + // Get second page + $query2 = new WP_Query( array( + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 5, + 'paged' => 2, + ) ); + + $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); + $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); + + // No overlap between pages + $this->assertEmpty( array_intersect( $page1_ids, $page2_ids ) ); + + // Total posts should equal sum of both pages + $this->assertEquals( 10, $query1->found_posts ); + $this->assertEquals( 5, count( $page1_ids ) ); + $this->assertEquals( 5, count( $page2_ids ) ); + } + + /** + * Test that deterministic ordering works with search queries. + * + * @ticket 44349 + */ + public function test_deterministic_ordering_with_search() { + // Create posts with searchable content + $post1 = self::factory()->post->create( array( + 'post_title' => 'Test Post 1', + 'post_content' => 'This is a test post', + 'post_date' => '2023-01-01 10:00:00', + ) ); + $post2 = self::factory()->post->create( array( + 'post_title' => 'Test Post 2', + 'post_content' => 'This is another test post', + 'post_date' => '2023-01-01 10:00:00', // Same date + ) ); + + $query = new WP_Query( array( + 's' => 'test', + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 10, + ) ); + + // Should still have deterministic ordering even with search + $this->assertStringContainsString( 'post_date ASC', $query->request ); + $this->assertStringContainsString( 'ID ASC', $query->request ); + $this->assertStringNotContainsString( 'ASC ASC', $query->request ); + } + + /** + * Test that deterministic ordering works with meta queries. + * + * @ticket 44349 + */ + public function test_deterministic_ordering_with_meta_query() { + // Create posts with meta values + $post1 = self::factory()->post->create(); + add_post_meta( $post1, 'test_meta', 'value1' ); + + $post2 = self::factory()->post->create(); + add_post_meta( $post2, 'test_meta', 'value2' ); + + $query = new WP_Query( array( + 'meta_key' => 'test_meta', + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 10, + ) ); + + // Should still have deterministic ordering with meta queries + $this->assertStringContainsString( 'post_date ASC', $query->request ); + $this->assertStringContainsString( 'ID ASC', $query->request ); + $this->assertStringNotContainsString( 'ASC ASC', $query->request ); + } + + /** + * Test that deterministic ordering works with taxonomy queries. + * + * @ticket 44349 + */ + public function test_deterministic_ordering_with_taxonomy_query() { + // Create posts with categories + $post1 = self::factory()->post->create(); + $post2 = self::factory()->post->create(); + + $cat_id = self::factory()->category->create( array( 'name' => 'Test Category' ) ); + wp_set_post_categories( $post1, array( $cat_id ) ); + wp_set_post_categories( $post2, array( $cat_id ) ); + + $query = new WP_Query( array( + 'category_name' => 'test-category', + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 10, + ) ); + + // Should still have deterministic ordering with taxonomy queries + $this->assertStringContainsString( 'post_date ASC', $query->request ); + $this->assertStringContainsString( 'ID ASC', $query->request ); + $this->assertStringNotContainsString( 'ASC ASC', $query->request ); + } +} From e0f75287791109399ab46107f069d06946942763 Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 22 Oct 2025 11:36:20 +1100 Subject: [PATCH 07/36] lint --- src/wp-includes/class-wp-query.php | 6 +- .../tests/query/deterministicOrdering.php | 462 +++++++++--------- 2 files changed, 239 insertions(+), 229 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index c1ddb99095a42..f84baa9f61eed 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -2521,9 +2521,9 @@ public function get_posts() { */ $orderby = "{$wpdb->posts}.post_date " . $query_vars['order'] . ', ' . "{$wpdb->posts}.ID " . $query_vars['order']; } - // See get_pages(): when sort_column is 'none', the get_pages() function should not generate any ORDER BY clause. - // Should it rather be handled in the get_pages() function? - // src/wp-includes/post.php L6496 + // See get_pages(): when sort_column is 'none', the get_pages() function should not generate any ORDER BY clause. + // Should it rather be handled in the get_pages() function? + // src/wp-includes/post.php L6496 } elseif ( 'none' === $query_vars['orderby'] || ( is_array( $query_vars['orderby'] ) && array_key_exists( 'none', $query_vars['orderby'] ) ) ) { $orderby = ''; } else { diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index f7c81f63a21db..132b76beb069f 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -11,67 +11,118 @@ class Tests_Query_DeterministicOrdering extends WP_UnitTestCase { /** - * Test that deterministic ordering adds ID as tie-breaker for fields that can have duplicates. + * Test that deterministic ordering prevents duplicate records across pages. + * + * This is the core test for the bug fix. When multiple posts have the same + * value for a field (like post_date), pagination can show duplicate records + * without deterministic ordering. * * @ticket 44349 */ - public function test_deterministic_ordering_adds_id_tie_breaker() { - global $wpdb; - - // Create posts with same post_date to test deterministic ordering - $post1 = self::factory()->post->create( array( - 'post_title' => 'Post A', - 'post_date' => '2023-01-01 10:00:00', - ) ); - $post2 = self::factory()->post->create( array( - 'post_title' => 'Post B', - 'post_date' => '2023-01-01 10:00:00', // Same date as post1 - ) ); - $post3 = self::factory()->post->create( array( - 'post_title' => 'Post C', - 'post_date' => '2023-01-01 10:00:00', // Same date as post1 and post2 - ) ); - - // Test ordering by post_date (should add ID tie-breaker) - $query = new WP_Query( array( - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 10, - ) ); - - // Verify SQL contains ID as secondary sort - $this->assertStringContainsString( 'ORDER BY', $query->request ); - $this->assertStringContainsString( 'post_date ASC', $query->request ); - $this->assertStringContainsString( 'ID ASC', $query->request ); - $this->assertStringNotContainsString( 'ASC ASC', $query->request ); // No double ASC + public function test_deterministic_ordering_prevents_duplicates_across_pages() { + // Create multiple posts with identical post_date to trigger the bug + $identical_date = '2023-01-01 10:00:00'; + $post_ids = array(); + + for ( $i = 1; $i <= 20; $i++ ) { + $post_ids[] = self::factory()->post->create( + array( + 'post_title' => "Post $i", + 'post_date' => $identical_date, + ) + ); + } + + // Get first page + $query1 = new WP_Query( + array( + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 10, + 'paged' => 1, + ) + ); + + // Get second page + $query2 = new WP_Query( + array( + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 10, + 'paged' => 2, + ) + ); + + $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); + $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); + + // Verify no overlap between pages (no duplicates) + $overlap = array_intersect( $page1_ids, $page2_ids ); + $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts' ); + + // Verify total count is correct + $this->assertEquals( 20, $query1->found_posts, 'Total posts should be 20' ); + $this->assertEquals( 10, count( $page1_ids ), 'First page should have 10 posts' ); + $this->assertEquals( 10, count( $page2_ids ), 'Second page should have 10 posts' ); + + // Verify deterministic ordering: same query should return same results + $query1_repeat = new WP_Query( + array( + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 10, + 'paged' => 1, + ) + ); + $page1_repeat_ids = wp_list_pluck( $query1_repeat->posts, 'ID' ); + + $this->assertEquals( $page1_ids, $page1_repeat_ids, 'Same query should return same results' ); } /** - * Test that deterministic ordering works with post_title. + * Test that deterministic ordering works with post_title field. * * @ticket 44349 */ public function test_deterministic_ordering_with_post_title() { - // Create posts with same title to test deterministic ordering - $post1 = self::factory()->post->create( array( - 'post_title' => 'Same Title', - 'post_date' => '2023-01-01 10:00:00', - ) ); - $post2 = self::factory()->post->create( array( - 'post_title' => 'Same Title', // Same title as post1 - 'post_date' => '2023-01-01 11:00:00', - ) ); - - $query = new WP_Query( array( - 'orderby' => 'post_title', - 'order' => 'ASC', - 'posts_per_page' => 10, - ) ); - - // Verify SQL contains ID as secondary sort - $this->assertStringContainsString( 'post_title ASC', $query->request ); - $this->assertStringContainsString( 'ID ASC', $query->request ); - $this->assertStringNotContainsString( 'ASC ASC', $query->request ); + $identical_title = 'Same Title'; + $post_ids = array(); + + for ( $i = 1; $i <= 15; $i++ ) { + $post_ids[] = self::factory()->post->create( + array( + 'post_title' => $identical_title, + 'post_date' => "2023-01-0$i 10:00:00", + ) + ); + } + + // Get first page + $query1 = new WP_Query( + array( + 'orderby' => 'post_title', + 'order' => 'ASC', + 'posts_per_page' => 8, + 'paged' => 1, + ) + ); + + // Get second page + $query2 = new WP_Query( + array( + 'orderby' => 'post_title', + 'order' => 'ASC', + 'posts_per_page' => 8, + 'paged' => 2, + ) + ); + + $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); + $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); + + // Verify no duplicates across pages + $overlap = array_intersect( $page1_ids, $page2_ids ); + $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts when ordering by title' ); } /** @@ -80,16 +131,44 @@ public function test_deterministic_ordering_with_post_title() { * @ticket 44349 */ public function test_deterministic_ordering_with_desc_order() { - $query = new WP_Query( array( - 'orderby' => 'post_date', - 'order' => 'DESC', - 'posts_per_page' => 10, - ) ); - - // Verify SQL contains ID as secondary sort with DESC - $this->assertStringContainsString( 'post_date DESC', $query->request ); - $this->assertStringContainsString( 'ID DESC', $query->request ); - $this->assertStringNotContainsString( 'DESC DESC', $query->request ); + $identical_date = '2023-01-01 10:00:00'; + $post_ids = array(); + + for ( $i = 1; $i <= 12; $i++ ) { + $post_ids[] = self::factory()->post->create( + array( + 'post_title' => "Post $i", + 'post_date' => $identical_date, + ) + ); + } + + // Get first page with DESC order + $query1 = new WP_Query( + array( + 'orderby' => 'post_date', + 'order' => 'DESC', + 'posts_per_page' => 6, + 'paged' => 1, + ) + ); + + // Get second page with DESC order + $query2 = new WP_Query( + array( + 'orderby' => 'post_date', + 'order' => 'DESC', + 'posts_per_page' => 6, + 'paged' => 2, + ) + ); + + $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); + $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); + + // Verify no duplicates across pages + $overlap = array_intersect( $page1_ids, $page2_ids ); + $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts with DESC order' ); } /** @@ -98,111 +177,78 @@ public function test_deterministic_ordering_with_desc_order() { * @ticket 44349 */ public function test_deterministic_ordering_with_array_orderby() { - $query = new WP_Query( array( - 'orderby' => array( - 'post_date' => 'ASC', - 'post_title' => 'ASC', - ), - 'posts_per_page' => 10, - ) ); - - // Verify SQL contains both fields with directions - $this->assertStringContainsString( 'post_date ASC', $query->request ); - $this->assertStringContainsString( 'post_title ASC', $query->request ); - $this->assertStringContainsString( 'ID ASC', $query->request ); - $this->assertStringNotContainsString( 'ASC ASC', $query->request ); - } + $identical_date = '2023-01-01 10:00:00'; + $post_ids = array(); + + for ( $i = 1; $i <= 16; $i++ ) { + $post_ids[] = self::factory()->post->create( + array( + 'post_title' => "Post $i", + 'post_date' => $identical_date, + ) + ); + } - /** - * Test that deterministic ordering doesn't add ID when ID is already present. - * - * @ticket 44349 - */ - public function test_deterministic_ordering_does_not_duplicate_id() { - $query = new WP_Query( array( - 'orderby' => 'ID', - 'order' => 'ASC', - 'posts_per_page' => 10, - ) ); + // Test with array orderby + $query1 = new WP_Query( + array( + 'orderby' => array( + 'post_date' => 'ASC', + 'post_title' => 'ASC', + ), + 'posts_per_page' => 8, + 'paged' => 1, + ) + ); + + $query2 = new WP_Query( + array( + 'orderby' => array( + 'post_date' => 'ASC', + 'post_title' => 'ASC', + ), + 'posts_per_page' => 8, + 'paged' => 2, + ) + ); - // Should not add duplicate ID - $this->assertStringContainsString( 'ID ASC', $query->request ); - $this->assertStringNotContainsString( 'ID ASC, ID ASC', $query->request ); - } + $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); + $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); - /** - * Test that deterministic ordering works with fields that don't need it. - * - * @ticket 44349 - */ - public function test_deterministic_ordering_with_non_deterministic_fields() { - $query = new WP_Query( array( - 'orderby' => 'rand', - 'posts_per_page' => 10, - ) ); - - // Should not add ID tie-breaker for rand - $this->assertStringContainsString( 'RAND()', $query->request ); - $this->assertStringNotContainsString( 'ID ASC', $query->request ); + // Verify no duplicates across pages + $overlap = array_intersect( $page1_ids, $page2_ids ); + $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts with array orderby' ); } /** - * Test that deterministic ordering works with default ordering. + * Test that deterministic ordering doesn't add ID when ID is already present. * * @ticket 44349 */ - public function test_deterministic_ordering_with_default_ordering() { - $query = new WP_Query( array( - 'posts_per_page' => 10, - ) ); - - // Default ordering should include ID tie-breaker - $this->assertStringContainsString( 'post_date DESC', $query->request ); - $this->assertStringContainsString( 'ID DESC', $query->request ); - $this->assertStringNotContainsString( 'DESC DESC', $query->request ); - } + public function test_deterministic_ordering_does_not_duplicate_id() { + $identical_date = '2023-01-01 10:00:00'; + $post_ids = array(); - /** - * Test that deterministic ordering prevents duplicate records across pages. - * - * @ticket 44349 - */ - public function test_deterministic_ordering_prevents_duplicates_across_pages() { - // Create multiple posts with same post_date - $posts = array(); for ( $i = 1; $i <= 10; $i++ ) { - $posts[] = self::factory()->post->create( array( - 'post_title' => "Post $i", - 'post_date' => '2023-01-01 10:00:00', // All same date - ) ); + $post_ids[] = self::factory()->post->create( + array( + 'post_title' => "Post $i", + 'post_date' => $identical_date, + ) + ); } - // Get first page - $query1 = new WP_Query( array( - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 5, - 'paged' => 1, - ) ); - - // Get second page - $query2 = new WP_Query( array( - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 5, - 'paged' => 2, - ) ); - - $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); - $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); + $query = new WP_Query( + array( + 'orderby' => 'ID', + 'order' => 'ASC', + 'posts_per_page' => 10, + ) + ); - // No overlap between pages - $this->assertEmpty( array_intersect( $page1_ids, $page2_ids ) ); - - // Total posts should equal sum of both pages - $this->assertEquals( 10, $query1->found_posts ); - $this->assertEquals( 5, count( $page1_ids ) ); - $this->assertEquals( 5, count( $page2_ids ) ); + // Should not add duplicate ID ordering + $this->assertStringContainsString( 'ID ASC', $query->request ); + $this->assertStringNotContainsString( 'ID ASC, ID ASC', $query->request ); } /** @@ -211,81 +257,45 @@ public function test_deterministic_ordering_prevents_duplicates_across_pages() { * @ticket 44349 */ public function test_deterministic_ordering_with_search() { - // Create posts with searchable content - $post1 = self::factory()->post->create( array( - 'post_title' => 'Test Post 1', - 'post_content' => 'This is a test post', - 'post_date' => '2023-01-01 10:00:00', - ) ); - $post2 = self::factory()->post->create( array( - 'post_title' => 'Test Post 2', - 'post_content' => 'This is another test post', - 'post_date' => '2023-01-01 10:00:00', // Same date - ) ); - - $query = new WP_Query( array( - 's' => 'test', - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 10, - ) ); - - // Should still have deterministic ordering even with search - $this->assertStringContainsString( 'post_date ASC', $query->request ); - $this->assertStringContainsString( 'ID ASC', $query->request ); - $this->assertStringNotContainsString( 'ASC ASC', $query->request ); - } + $identical_date = '2023-01-01 10:00:00'; + $post_ids = array(); + + for ( $i = 1; $i <= 12; $i++ ) { + $post_ids[] = self::factory()->post->create( + array( + 'post_title' => "Test Post $i", + 'post_content' => 'This is a test post', + 'post_date' => $identical_date, + ) + ); + } - /** - * Test that deterministic ordering works with meta queries. - * - * @ticket 44349 - */ - public function test_deterministic_ordering_with_meta_query() { - // Create posts with meta values - $post1 = self::factory()->post->create(); - add_post_meta( $post1, 'test_meta', 'value1' ); - - $post2 = self::factory()->post->create(); - add_post_meta( $post2, 'test_meta', 'value2' ); - - $query = new WP_Query( array( - 'meta_key' => 'test_meta', - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 10, - ) ); - - // Should still have deterministic ordering with meta queries - $this->assertStringContainsString( 'post_date ASC', $query->request ); - $this->assertStringContainsString( 'ID ASC', $query->request ); - $this->assertStringNotContainsString( 'ASC ASC', $query->request ); - } + // Test with search + $query1 = new WP_Query( + array( + 's' => 'test', + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 6, + 'paged' => 1, + ) + ); + + $query2 = new WP_Query( + array( + 's' => 'test', + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 6, + 'paged' => 2, + ) + ); - /** - * Test that deterministic ordering works with taxonomy queries. - * - * @ticket 44349 - */ - public function test_deterministic_ordering_with_taxonomy_query() { - // Create posts with categories - $post1 = self::factory()->post->create(); - $post2 = self::factory()->post->create(); - - $cat_id = self::factory()->category->create( array( 'name' => 'Test Category' ) ); - wp_set_post_categories( $post1, array( $cat_id ) ); - wp_set_post_categories( $post2, array( $cat_id ) ); - - $query = new WP_Query( array( - 'category_name' => 'test-category', - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 10, - ) ); - - // Should still have deterministic ordering with taxonomy queries - $this->assertStringContainsString( 'post_date ASC', $query->request ); - $this->assertStringContainsString( 'ID ASC', $query->request ); - $this->assertStringNotContainsString( 'ASC ASC', $query->request ); + $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); + $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); + + // Verify no duplicates across pages even with search + $overlap = array_intersect( $page1_ids, $page2_ids ); + $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts even with search' ); } } From 6f8bc1a8052361e69934f096c55f8e87d44a2baf Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 22 Oct 2025 11:38:38 +1100 Subject: [PATCH 08/36] Remove ticket number in tests for now --- .../phpunit/tests/query/deterministicOrdering.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index 132b76beb069f..b46ed02757f03 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -6,7 +6,7 @@ * * @group query * @group ordering - * @ticket 44349 + * @ticket xxxxx */ class Tests_Query_DeterministicOrdering extends WP_UnitTestCase { @@ -17,7 +17,7 @@ class Tests_Query_DeterministicOrdering extends WP_UnitTestCase { * value for a field (like post_date), pagination can show duplicate records * without deterministic ordering. * - * @ticket 44349 + * @ticket xxxxx */ public function test_deterministic_ordering_prevents_duplicates_across_pages() { // Create multiple posts with identical post_date to trigger the bug @@ -82,7 +82,7 @@ public function test_deterministic_ordering_prevents_duplicates_across_pages() { /** * Test that deterministic ordering works with post_title field. * - * @ticket 44349 + * @ticket xxxxx */ public function test_deterministic_ordering_with_post_title() { $identical_title = 'Same Title'; @@ -128,7 +128,7 @@ public function test_deterministic_ordering_with_post_title() { /** * Test that deterministic ordering works with DESC order. * - * @ticket 44349 + * @ticket xxxxx */ public function test_deterministic_ordering_with_desc_order() { $identical_date = '2023-01-01 10:00:00'; @@ -174,7 +174,7 @@ public function test_deterministic_ordering_with_desc_order() { /** * Test that deterministic ordering works with array orderby. * - * @ticket 44349 + * @ticket xxxxx */ public function test_deterministic_ordering_with_array_orderby() { $identical_date = '2023-01-01 10:00:00'; @@ -223,7 +223,7 @@ public function test_deterministic_ordering_with_array_orderby() { /** * Test that deterministic ordering doesn't add ID when ID is already present. * - * @ticket 44349 + * @ticket xxxxx */ public function test_deterministic_ordering_does_not_duplicate_id() { $identical_date = '2023-01-01 10:00:00'; @@ -254,7 +254,7 @@ public function test_deterministic_ordering_does_not_duplicate_id() { /** * Test that deterministic ordering works with search queries. * - * @ticket 44349 + * @ticket xxxxx */ public function test_deterministic_ordering_with_search() { $identical_date = '2023-01-01 10:00:00'; From ebd9bd24f0f6d7682568d1c304c3b8a6af74c9d7 Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 22 Oct 2025 15:58:41 +1100 Subject: [PATCH 09/36] Refactor WP_Query to ensure consistent ordering by appending ID as a secondary sort field in various scenarios. Update unit tests to reflect changes in expected SQL output for orderby cases, enhancing determinism in query results. --- src/wp-includes/class-wp-query.php | 17 ++++++++++++++--- .../tests/rest-api/rest-posts-controller.php | 18 +++++++++--------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index f84baa9f61eed..f094fd03556ef 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -2607,7 +2607,7 @@ public function get_posts() { if ( empty( $orderby_array ) ) { $orderby = "{$wpdb->posts}.post_date " . $query_vars['order'] . ', ' . "{$wpdb->posts}.ID " . $query_vars['order']; } else { - $orderby = implode( ', ', $orderby_array ); + $orderby = trim( implode( ', ', $orderby_array ) ); } } @@ -3323,7 +3323,18 @@ public function get_posts() { } if ( $query_vars['cache_results'] && $id_query_is_cacheable ) { - $new_request = str_replace( $fields, "{$wpdb->posts}.*", $this->request ); + $new_request = $this->request; + // Split SQL into parts. + $parts = explode( 'ORDER BY', $new_request ); + if ( count( $parts ) === 2 ) { + // Replace only in the SELECT part, preserve ORDER BY. + $select_part = str_replace( $fields, "{$wpdb->posts}.*", $parts[0] ); + $new_request = $select_part . 'ORDER BY' . $parts[1]; + } else { + // No ORDER BY clause, safe to replace. + $new_request = str_replace( $fields, "{$wpdb->posts}.*", $new_request ); + } + $cache_key = $this->generate_cache_key( $query_vars, $new_request ); $cache_found = false; @@ -5139,7 +5150,7 @@ protected function generate_cache_key( array $args, $sql ) { // Add a default orderby value of date to ensure same cache key generation. if ( ! isset( $args['orderby'] ) ) { - $args['orderby'] = 'date'; + $args['orderby'] = 'date, ID'; } $placeholder = $wpdb->placeholder_escape(); diff --git a/tests/phpunit/tests/rest-api/rest-posts-controller.php b/tests/phpunit/tests/rest-api/rest-posts-controller.php index 212ddde70dd83..38a43fef488c7 100644 --- a/tests/phpunit/tests/rest-api/rest-posts-controller.php +++ b/tests/phpunit/tests/rest-api/rest-posts-controller.php @@ -488,7 +488,7 @@ public function test_get_items_include_query( $method ) { $this->assertSame( 2, $headers['X-WP-Total'], 'Failed asserting that the number of posts is correct.' ); } - $this->assertPostsOrderedBy( '{posts}.post_date DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_date DESC, {posts}.ID DESC' ); // 'orderby' => 'include'. $request->set_param( 'orderby', 'include' ); @@ -544,7 +544,7 @@ public function test_get_items_orderby_author_query() { $this->assertSame( self::$editor_id, $data[1]['author'] ); $this->assertSame( self::$editor_id, $data[2]['author'] ); - $this->assertPostsOrderedBy( '{posts}.post_author DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_author DESC, {posts}.ID DESC' ); } public function test_get_items_orderby_modified_query() { @@ -568,7 +568,7 @@ public function test_get_items_orderby_modified_query() { $this->assertSame( $id3, $data[1]['id'] ); $this->assertSame( $id2, $data[2]['id'] ); - $this->assertPostsOrderedBy( '{posts}.post_modified DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_modified DESC, {posts}.ID DESC' ); } public function test_get_items_orderby_parent_query() { @@ -606,7 +606,7 @@ public function test_get_items_orderby_parent_query() { $this->assertSame( 0, $data[1]['parent'] ); $this->assertSame( 0, $data[2]['parent'] ); - $this->assertPostsOrderedBy( '{posts}.post_parent DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_parent DESC, {posts}.ID DESC' ); } public function test_get_items_exclude_query() { @@ -976,14 +976,14 @@ public function test_get_items_order_and_orderby() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); $this->assertSame( 'Apple Sauce', $data[0]['title']['rendered'] ); - $this->assertPostsOrderedBy( '{posts}.post_title DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_title DESC, {posts}.ID DESC' ); // 'order' => 'asc'. $request->set_param( 'order', 'asc' ); $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); $this->assertSame( 'Apple Cobbler', $data[0]['title']['rendered'] ); - $this->assertPostsOrderedBy( '{posts}.post_title ASC' ); + $this->assertPostsOrderedBy( '{posts}.post_title ASC, {posts}.ID ASC' ); // 'order' => 'asc,id' should error. $request->set_param( 'order', 'asc,id' ); @@ -1068,7 +1068,7 @@ public function test_get_items_with_orderby_slug() { // Default ORDER is DESC. $this->assertSame( 'xyz', $data[0]['slug'] ); $this->assertSame( 'abc', $data[1]['slug'] ); - $this->assertPostsOrderedBy( '{posts}.post_name DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_name DESC, {posts}.ID DESC' ); } public function test_get_items_with_orderby_slugs() { @@ -1120,7 +1120,7 @@ public function test_get_items_with_orderby_relevance() { $this->assertCount( 2, $data ); $this->assertSame( $id1, $data[0]['id'] ); $this->assertSame( $id2, $data[1]['id'] ); - $this->assertPostsOrderedBy( '{posts}.post_title LIKE \'%relevant%\' DESC, {posts}.post_date DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_title LIKE \'%relevant%\' DESC, {posts}.post_date DESC, {posts}.ID DESC' ); } public function test_get_items_with_orderby_relevance_two_terms() { @@ -1148,7 +1148,7 @@ public function test_get_items_with_orderby_relevance_two_terms() { $this->assertCount( 2, $data ); $this->assertSame( $id1, $data[0]['id'] ); $this->assertSame( $id2, $data[1]['id'] ); - $this->assertPostsOrderedBy( '(CASE WHEN {posts}.post_title LIKE \'%relevant content%\' THEN 1 WHEN {posts}.post_title LIKE \'%relevant%\' AND {posts}.post_title LIKE \'%content%\' THEN 2 WHEN {posts}.post_title LIKE \'%relevant%\' OR {posts}.post_title LIKE \'%content%\' THEN 3 WHEN {posts}.post_excerpt LIKE \'%relevant content%\' THEN 4 WHEN {posts}.post_content LIKE \'%relevant content%\' THEN 5 ELSE 6 END), {posts}.post_date DESC' ); + $this->assertPostsOrderedBy( '(CASE WHEN {posts}.post_title LIKE \'%relevant content%\' THEN 1 WHEN {posts}.post_title LIKE \'%relevant%\' AND {posts}.post_title LIKE \'%content%\' THEN 2 WHEN {posts}.post_title LIKE \'%relevant%\' OR {posts}.post_title LIKE \'%content%\' THEN 3 WHEN {posts}.post_excerpt LIKE \'%relevant content%\' THEN 4 WHEN {posts}.post_content LIKE \'%relevant content%\' THEN 5 ELSE 6 END), {posts}.post_date DESC, {posts}.ID DESC' ); } public function test_get_items_with_orderby_relevance_missing_search() { From addb896aef13ec48263bf1fa22ee0478af78624c Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 22 Oct 2025 16:37:52 +1100 Subject: [PATCH 10/36] Enhance WP_Query ordering logic by normalizing 'date' to 'date, ID' for consistent cache key generation. This change ensures deterministic results when 'date' is specified as the orderby value, improving query consistency. --- src/wp-includes/class-wp-query.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index f094fd03556ef..5629ec4053180 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -5148,9 +5148,13 @@ protected function generate_cache_key( array $args, $sql ) { sort( $args['post_status'] ); } + // Add a default orderby value of date to ensure same cache key generation. // Add a default orderby value of date to ensure same cache key generation. if ( ! isset( $args['orderby'] ) ) { $args['orderby'] = 'date, ID'; + } elseif ( $args['orderby'] === 'date' ) { + // Normalize 'date' to 'date, ID' to match deterministic ordering + $args['orderby'] = 'date, ID'; } $placeholder = $wpdb->placeholder_escape(); From 310360e1a06e67e5538f94466ee9ab630ec0cfc6 Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 22 Oct 2025 16:47:36 +1100 Subject: [PATCH 11/36] lint --- src/wp-includes/class-wp-query.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 5629ec4053180..32583bf50da4c 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -3336,8 +3336,8 @@ public function get_posts() { } $cache_key = $this->generate_cache_key( $query_vars, $new_request ); - $cache_found = false; + if ( null === $this->posts ) { $cached_results = wp_cache_get_salted( $cache_key, 'post-queries', $last_changed ); @@ -5148,12 +5148,14 @@ protected function generate_cache_key( array $args, $sql ) { sort( $args['post_status'] ); } - // Add a default orderby value of date to ensure same cache key generation. - // Add a default orderby value of date to ensure same cache key generation. + + /* + * Ensure deterministic ordering to prevent duplicate records across pages. + * When multiple posts have the same value for a field, add ID as secondary sort to guarantee consistent ordering. + */ if ( ! isset( $args['orderby'] ) ) { $args['orderby'] = 'date, ID'; - } elseif ( $args['orderby'] === 'date' ) { - // Normalize 'date' to 'date, ID' to match deterministic ordering + } elseif ( 'date' === $args['orderby'] ) { $args['orderby'] = 'date, ID'; } From 5a9ef90491128cfd8ae59d38293180e5ec2b98b7 Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 22 Oct 2025 16:48:18 +1100 Subject: [PATCH 12/36] linto --- src/wp-includes/class-wp-query.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 32583bf50da4c..66fe41c8dceb2 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -5148,7 +5148,6 @@ protected function generate_cache_key( array $args, $sql ) { sort( $args['post_status'] ); } - /* * Ensure deterministic ordering to prevent duplicate records across pages. * When multiple posts have the same value for a field, add ID as secondary sort to guarantee consistent ordering. From 9dd9d7b63fb0001173bb78c7c5d33aa9871d9746 Mon Sep 17 00:00:00 2001 From: Ramon Date: Fri, 5 Dec 2025 12:30:17 +1100 Subject: [PATCH 13/36] Fix date formatting in deterministic ordering test to ensure consistent post creation. Updated post_date to use str_pad for zero-padding single-digit days. --- tests/phpunit/tests/query/deterministicOrdering.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index b46ed02757f03..a6bdfb915c6b9 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -92,7 +92,7 @@ public function test_deterministic_ordering_with_post_title() { $post_ids[] = self::factory()->post->create( array( 'post_title' => $identical_title, - 'post_date' => "2023-01-0$i 10:00:00", + 'post_date' => "2023-01-" . str_pad((string) $i, 2, '0', STR_PAD_LEFT) . " 10:00:00", ) ); } From f2963de80e63fd4c984a082af63a48e9a261e994 Mon Sep 17 00:00:00 2001 From: Ramon Date: Fri, 5 Dec 2025 23:04:30 +1100 Subject: [PATCH 14/36] Refactor deterministic ordering tests to utilize shared fixtures for post creation. Introduced separate arrays for posts with identical dates, titles, and menu orders to enhance test clarity and maintainability. Updated queries to reference these shared fixtures, ensuring consistent results across pagination and ordering scenarios. --- .../tests/query/deterministicOrdering.php | 271 +++++++++++++----- 1 file changed, 200 insertions(+), 71 deletions(-) diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index a6bdfb915c6b9..0c74fb1ed886c 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -11,31 +11,135 @@ class Tests_Query_DeterministicOrdering extends WP_UnitTestCase { /** - * Test that deterministic ordering prevents duplicate records across pages. + * Post IDs for posts with identical dates (for date ordering tests). * - * This is the core test for the bug fix. When multiple posts have the same - * value for a field (like post_date), pagination can show duplicate records - * without deterministic ordering. + * @var array + */ + protected static $date_identical_post_ids = array(); + + /** + * Post IDs for posts with identical titles (for title ordering tests). * - * @ticket xxxxx + * @var array */ - public function test_deterministic_ordering_prevents_duplicates_across_pages() { - // Create multiple posts with identical post_date to trigger the bug - $identical_date = '2023-01-01 10:00:00'; - $post_ids = array(); + protected static $title_identical_post_ids = array(); + + /** + * Post IDs for search tests. + * + * @var array + */ + protected static $search_post_ids = array(); + + /** + * Post IDs for menu_order tests. + * + * @var array + */ + protected static $menu_order_post_ids = array(); + + /** + * Set up shared fixtures for all tests. + */ + public static function set_up_before_class() { + parent::set_up_before_class(); + // Register custom post types for test isolation. + register_post_type( + 'wptests_time_ident', + array( + 'public' => true, + ) + ); + + register_post_type( + 'wptests_title_ident', + array( + 'public' => true, + ) + ); + + // Create posts with identical dates for date ordering tests. + $identical_date = '2023-01-01 10:00:00'; for ( $i = 1; $i <= 20; $i++ ) { - $post_ids[] = self::factory()->post->create( + self::$date_identical_post_ids[] = self::factory()->post->create( array( + 'post_type' => 'wptests_time_ident', 'post_title' => "Post $i", 'post_date' => $identical_date, ) ); } + // Create posts with identical titles for title ordering tests. + $identical_title = 'Same Title'; + for ( $i = 1; $i <= 15; $i++ ) { + self::$title_identical_post_ids[] = self::factory()->post->create( + array( + 'post_type' => 'wptests_title_ident', + 'post_title' => $identical_title, + 'post_date' => '2023-01-' . str_pad( (string) $i, 2, '0', STR_PAD_LEFT ) . ' 10:00:00', + ) + ); + } + + // Create posts for search tests. + $identical_date = '2023-01-01 10:00:00'; + for ( $i = 1; $i <= 12; $i++ ) { + self::$search_post_ids[] = self::factory()->post->create( + array( + 'post_type' => 'wptests_time_ident', + 'post_title' => "Test Post $i", + 'post_content' => 'This is a test post', + 'post_date' => $identical_date, + ) + ); + } + + // Create pages with identical menu_order for menu_order tests. + for ( $i = 1; $i <= 20; $i++ ) { + self::$menu_order_post_ids[] = self::factory()->post->create( + array( + 'post_type' => 'page', + 'post_title' => "Page $i", + 'menu_order' => 0, // All pages have same menu_order + ) + ); + } + } + + /** + * Clean up after all tests. + */ + public static function tear_down_after_class() { + _unregister_post_type( 'wptests_time_ident' ); + _unregister_post_type( 'wptests_title_ident' ); + + self::$date_identical_post_ids = array(); + self::$title_identical_post_ids = array(); + self::$search_post_ids = array(); + self::$menu_order_post_ids = array(); + + parent::tear_down_after_class(); + } + + /** + * Test that deterministic ordering prevents duplicate records across pages. + * + * This is the core test for the bug fix. When multiple posts have the same + * value for a field (like post_date), pagination can show duplicate records + * without deterministic ordering. + * + * @ticket xxxxx + */ + public function test_deterministic_ordering_prevents_duplicates_across_pages() { + // Use shared fixtures with identical post_date + // Get first page $query1 = new WP_Query( array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, 'orderby' => 'post_date', 'order' => 'ASC', 'posts_per_page' => 10, @@ -46,6 +150,8 @@ public function test_deterministic_ordering_prevents_duplicates_across_pages() { // Get second page $query2 = new WP_Query( array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, 'orderby' => 'post_date', 'order' => 'ASC', 'posts_per_page' => 10, @@ -68,6 +174,8 @@ public function test_deterministic_ordering_prevents_duplicates_across_pages() { // Verify deterministic ordering: same query should return same results $query1_repeat = new WP_Query( array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, 'orderby' => 'post_date', 'order' => 'ASC', 'posts_per_page' => 10, @@ -85,21 +193,12 @@ public function test_deterministic_ordering_prevents_duplicates_across_pages() { * @ticket xxxxx */ public function test_deterministic_ordering_with_post_title() { - $identical_title = 'Same Title'; - $post_ids = array(); - - for ( $i = 1; $i <= 15; $i++ ) { - $post_ids[] = self::factory()->post->create( - array( - 'post_title' => $identical_title, - 'post_date' => "2023-01-" . str_pad((string) $i, 2, '0', STR_PAD_LEFT) . " 10:00:00", - ) - ); - } - + // Use shared fixtures with identical post_title // Get first page $query1 = new WP_Query( array( + 'post_type' => 'wptests_title_ident', + 'post__in' => self::$title_identical_post_ids, 'orderby' => 'post_title', 'order' => 'ASC', 'posts_per_page' => 8, @@ -110,6 +209,8 @@ public function test_deterministic_ordering_with_post_title() { // Get second page $query2 = new WP_Query( array( + 'post_type' => 'wptests_title_ident', + 'post__in' => self::$title_identical_post_ids, 'orderby' => 'post_title', 'order' => 'ASC', 'posts_per_page' => 8, @@ -131,21 +232,12 @@ public function test_deterministic_ordering_with_post_title() { * @ticket xxxxx */ public function test_deterministic_ordering_with_desc_order() { - $identical_date = '2023-01-01 10:00:00'; - $post_ids = array(); - - for ( $i = 1; $i <= 12; $i++ ) { - $post_ids[] = self::factory()->post->create( - array( - 'post_title' => "Post $i", - 'post_date' => $identical_date, - ) - ); - } - + // Use shared fixtures with identical post_date // Get first page with DESC order $query1 = new WP_Query( array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, 'orderby' => 'post_date', 'order' => 'DESC', 'posts_per_page' => 6, @@ -156,6 +248,8 @@ public function test_deterministic_ordering_with_desc_order() { // Get second page with DESC order $query2 = new WP_Query( array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, 'orderby' => 'post_date', 'order' => 'DESC', 'posts_per_page' => 6, @@ -177,21 +271,12 @@ public function test_deterministic_ordering_with_desc_order() { * @ticket xxxxx */ public function test_deterministic_ordering_with_array_orderby() { - $identical_date = '2023-01-01 10:00:00'; - $post_ids = array(); - - for ( $i = 1; $i <= 16; $i++ ) { - $post_ids[] = self::factory()->post->create( - array( - 'post_title' => "Post $i", - 'post_date' => $identical_date, - ) - ); - } - + // Use shared fixtures with identical post_date // Test with array orderby $query1 = new WP_Query( array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, 'orderby' => array( 'post_date' => 'ASC', 'post_title' => 'ASC', @@ -203,6 +288,8 @@ public function test_deterministic_ordering_with_array_orderby() { $query2 = new WP_Query( array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, 'orderby' => array( 'post_date' => 'ASC', 'post_title' => 'ASC', @@ -226,20 +313,11 @@ public function test_deterministic_ordering_with_array_orderby() { * @ticket xxxxx */ public function test_deterministic_ordering_does_not_duplicate_id() { - $identical_date = '2023-01-01 10:00:00'; - $post_ids = array(); - - for ( $i = 1; $i <= 10; $i++ ) { - $post_ids[] = self::factory()->post->create( - array( - 'post_title' => "Post $i", - 'post_date' => $identical_date, - ) - ); - } - + // Use shared fixtures with identical post_date $query = new WP_Query( array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, 'orderby' => 'ID', 'order' => 'ASC', 'posts_per_page' => 10, @@ -257,22 +335,12 @@ public function test_deterministic_ordering_does_not_duplicate_id() { * @ticket xxxxx */ public function test_deterministic_ordering_with_search() { - $identical_date = '2023-01-01 10:00:00'; - $post_ids = array(); - - for ( $i = 1; $i <= 12; $i++ ) { - $post_ids[] = self::factory()->post->create( - array( - 'post_title' => "Test Post $i", - 'post_content' => 'This is a test post', - 'post_date' => $identical_date, - ) - ); - } - + // Use shared fixtures for search tests // Test with search $query1 = new WP_Query( array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$search_post_ids, 's' => 'test', 'orderby' => 'post_date', 'order' => 'ASC', @@ -283,6 +351,8 @@ public function test_deterministic_ordering_with_search() { $query2 = new WP_Query( array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$search_post_ids, 's' => 'test', 'orderby' => 'post_date', 'order' => 'ASC', @@ -298,4 +368,63 @@ public function test_deterministic_ordering_with_search() { $overlap = array_intersect( $page1_ids, $page2_ids ); $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts even with search' ); } + + /** + * Test that deterministic ordering works with menu_order field. + * + * @ticket xxxxx + */ + public function test_deterministic_ordering_with_menu_order() { + // Use shared fixtures with identical menu_order + // Get first page + $query1 = new WP_Query( + array( + 'post_type' => 'page', + 'post__in' => self::$menu_order_post_ids, + 'orderby' => 'menu_order', + 'order' => 'ASC', + 'posts_per_page' => 10, + 'paged' => 1, + ) + ); + + // Get second page + $query2 = new WP_Query( + array( + 'post_type' => 'page', + 'post__in' => self::$menu_order_post_ids, + 'orderby' => 'menu_order', + 'order' => 'ASC', + 'posts_per_page' => 10, + 'paged' => 2, + ) + ); + + $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); + $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); + + // Verify no overlap between pages (no duplicates) + $overlap = array_intersect( $page1_ids, $page2_ids ); + $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts when ordering by menu_order' ); + + // Verify total count is correct + $this->assertEquals( 20, $query1->found_posts, 'Total pages should be 20' ); + $this->assertEquals( 10, count( $page1_ids ), 'First page should have 10 pages' ); + $this->assertEquals( 10, count( $page2_ids ), 'Second page should have 10 pages' ); + + // Verify deterministic ordering: same query should return same results + $query1_repeat = new WP_Query( + array( + 'post_type' => 'page', + 'post__in' => self::$menu_order_post_ids, + 'orderby' => 'menu_order', + 'order' => 'ASC', + 'posts_per_page' => 10, + 'paged' => 1, + ) + ); + $page1_repeat_ids = wp_list_pluck( $query1_repeat->posts, 'ID' ); + + $this->assertEquals( $page1_ids, $page1_repeat_ids, 'Same query should return same results when ordering by menu_order' ); + } } From c29cb0ffccb3978348652c1960a54d5412658c50 Mon Sep 17 00:00:00 2001 From: Ramon Date: Fri, 5 Dec 2025 23:33:19 +1100 Subject: [PATCH 15/36] Refactor WP_Query ordering logic to implement a blacklist approach for deterministic ordering. Updated comments for clarity and introduced a new test for metadata ordering to ensure no duplicates across paginated results. --- src/wp-includes/class-wp-query.php | 38 ++++------ .../tests/query/deterministicOrdering.php | 70 +++++++++++++++++++ 2 files changed, 84 insertions(+), 24 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 66fe41c8dceb2..6a56caa3751a3 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -2531,24 +2531,14 @@ public function get_posts() { * Ensure deterministic ordering to prevent duplicate records across pages. * When multiple posts have the same value for a field, add ID as secondary sort to guarantee consistent ordering. * Note: this is to circumvent a bug that is currently being tracked in https://core.trac.wordpress.org/ticket/44349. + * + * Use a blacklist approach: add ID as tie-breaker for all orderby fields except those that are + * already deterministic (ID itself, random ordering, or search relevance). */ - $fields_requiring_deterministic_orderby = array( - 'post_name', - 'post_author', - 'post_date', - 'post_title', - 'post_modified', - 'post_parent', - 'post_type', - 'name', - 'author', - 'date', - 'title', - 'modified', - 'parent', - 'type', - 'menu_order', - 'comment_count', + $fields_excluding_deterministic_orderby = array( + 'ID', + 'rand', + 'relevance', ); $orderby_array = array(); @@ -2567,10 +2557,10 @@ public function get_posts() { $orderby_array[] = $parsed . ' ' . $this->parse_order( $order ); - // Check if this field needs deterministic ordering - if ( in_array( $_orderby, $fields_requiring_deterministic_orderby, true ) ) { + // Check if this field should have deterministic ordering (not in blacklist). + if ( ! in_array( $_orderby, $fields_excluding_deterministic_orderby, true ) ) { $needs_deterministic_orderby = true; - // Use the order from the array for ID tie-breaker + // Use the order from the array for ID tie-breaker. $id_tie_breaker_order = $this->parse_order( $order ); } elseif ( 'ID' === $_orderby ) { $has_id_orderby = true; @@ -2589,8 +2579,8 @@ public function get_posts() { $orderby_array[] = $parsed . ' ' . $query_vars['order']; - // Check if this field needs deterministic ordering - if ( in_array( $orderby, $fields_requiring_deterministic_orderby, true ) ) { + // Check if this field should have deterministic ordering (not in blacklist). + if ( ! in_array( $orderby, $fields_excluding_deterministic_orderby, true ) ) { $needs_deterministic_orderby = true; } elseif ( 'ID' === $orderby ) { $has_id_orderby = true; @@ -2598,12 +2588,12 @@ public function get_posts() { } } - // Add ID as tie-breaker if needed and not already present + // Add ID as tie-breaker if needed and not already present. if ( $needs_deterministic_orderby && ! $has_id_orderby ) { $orderby_array[] = "{$wpdb->posts}.ID " . $id_tie_breaker_order; } - // Build the final orderby string + // Build the final orderby string. if ( empty( $orderby_array ) ) { $orderby = "{$wpdb->posts}.post_date " . $query_vars['order'] . ', ' . "{$wpdb->posts}.ID " . $query_vars['order']; } else { diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index 0c74fb1ed886c..264d964aee23f 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -427,4 +427,74 @@ public function test_deterministic_ordering_with_menu_order() { $this->assertEquals( $page1_ids, $page1_repeat_ids, 'Same query should return same results when ordering by menu_order' ); } + + /** + * Test that deterministic ordering works with metadata ordering. + * + * @ticket xxxxx + */ + public function test_deterministic_ordering_with_metadata() { + $post_ids = array(); + + // Create posts with identical meta values to trigger the bug + $identical_meta_value = 'same_price'; + for ( $i = 1; $i <= 20; $i++ ) { + $post_id = self::factory()->post->create( + array( + 'post_type' => 'wptests_time_ident', + 'post_title' => "Post $i", + ) + ); + add_post_meta( $post_id, 'price', $identical_meta_value ); + $post_ids[] = $post_id; + } + + // Get first page ordering by metadata + $query1 = new WP_Query( + array( + 'post_type' => 'wptests_time_ident', + 'post__in' => $post_ids, + 'meta_query' => array( + 'price_key' => array( + 'key' => 'price', + 'compare' => 'EXISTS', + ), + ), + 'orderby' => 'price_key', + 'order' => 'ASC', + 'posts_per_page' => 10, + 'paged' => 1, + ) + ); + + // Get second page ordering by metadata + $query2 = new WP_Query( + array( + 'post_type' => 'wptests_time_ident', + 'post__in' => $post_ids, + 'meta_query' => array( + 'price_key' => array( + 'key' => 'price', + 'compare' => 'EXISTS', + ), + ), + 'orderby' => 'price_key', + 'order' => 'ASC', + 'posts_per_page' => 10, + 'paged' => 2, + ) + ); + + $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); + $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); + + // Verify no overlap between pages (no duplicates) + $overlap = array_intersect( $page1_ids, $page2_ids ); + $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts when ordering by metadata' ); + + // Verify total count is correct + $this->assertEquals( 20, $query1->found_posts, 'Total posts should be 20' ); + $this->assertEquals( 10, count( $page1_ids ), 'First page should have 10 posts' ); + $this->assertEquals( 10, count( $page2_ids ), 'Second page should have 10 posts' ); + } } From 828a90e7cef1be01b5f2b4a393b4da33c5d6c39a Mon Sep 17 00:00:00 2001 From: Ramon Date: Fri, 5 Dec 2025 23:50:52 +1100 Subject: [PATCH 16/36] Add search relevance tests to deterministic ordering suite Introduced new tests to verify deterministic ordering when posts are ordered by search relevance. Created shared fixtures for posts with identical content to ensure consistent relevance scores, preventing duplicates across paginated results. Updated the test suite to include scenarios for both explicit and empty orderby parameters, ensuring robust coverage of search-related ordering behavior. --- .../tests/query/deterministicOrdering.php | 144 +++++++++++++++++- 1 file changed, 140 insertions(+), 4 deletions(-) diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index 264d964aee23f..1f6941e9fbf6e 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -38,6 +38,13 @@ class Tests_Query_DeterministicOrdering extends WP_UnitTestCase { */ protected static $menu_order_post_ids = array(); + /** + * Post IDs for search relevance tests. + * + * @var array + */ + protected static $search_relevance_post_ids = array(); + /** * Set up shared fixtures for all tests. */ @@ -106,6 +113,20 @@ public static function set_up_before_class() { ) ); } + + // Create posts for search relevance tests. + // All posts will have the same content to ensure same relevance scores. + $identical_content = 'This is a search test post with identical content'; + for ( $i = 1; $i <= 20; $i++ ) { + self::$search_relevance_post_ids[] = self::factory()->post->create( + array( + 'post_type' => 'wptests_time_ident', + 'post_title' => "Search Post $i", + 'post_content' => $identical_content, + 'post_excerpt' => $identical_content, + ) + ); + } } /** @@ -115,10 +136,11 @@ public static function tear_down_after_class() { _unregister_post_type( 'wptests_time_ident' ); _unregister_post_type( 'wptests_title_ident' ); - self::$date_identical_post_ids = array(); - self::$title_identical_post_ids = array(); - self::$search_post_ids = array(); - self::$menu_order_post_ids = array(); + self::$date_identical_post_ids = array(); + self::$title_identical_post_ids = array(); + self::$search_post_ids = array(); + self::$menu_order_post_ids = array(); + self::$search_relevance_post_ids = array(); parent::tear_down_after_class(); } @@ -497,4 +519,118 @@ public function test_deterministic_ordering_with_metadata() { $this->assertEquals( 10, count( $page1_ids ), 'First page should have 10 posts' ); $this->assertEquals( 10, count( $page2_ids ), 'Second page should have 10 posts' ); } + + /** + * Test that deterministic ordering works with search relevance ordering. + * + * When ordering by search relevance, multiple posts can have the same relevance score, + * causing duplicate records across pages without deterministic ordering. + * + * @ticket xxxxx + */ + public function test_deterministic_ordering_with_search_relevance() { + // Use shared fixtures with identical content (same relevance scores) + // Get first page ordering by relevance + $query1 = new WP_Query( + array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$search_relevance_post_ids, + 's' => 'search test', + 'orderby' => 'relevance', + 'order' => 'DESC', + 'posts_per_page' => 10, + 'paged' => 1, + ) + ); + + // Get second page ordering by relevance + $query2 = new WP_Query( + array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$search_relevance_post_ids, + 's' => 'search test', + 'orderby' => 'relevance', + 'order' => 'DESC', + 'posts_per_page' => 10, + 'paged' => 2, + ) + ); + + $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); + $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); + + // Verify no overlap between pages (no duplicates) + $overlap = array_intersect( $page1_ids, $page2_ids ); + $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts when ordering by search relevance' ); + + // Verify total count is correct + $this->assertEquals( 20, $query1->found_posts, 'Total posts should be 20' ); + $this->assertEquals( 10, count( $page1_ids ), 'First page should have 10 posts' ); + $this->assertEquals( 10, count( $page2_ids ), 'Second page should have 10 posts' ); + + // Verify deterministic ordering: same query should return same results + $query1_repeat = new WP_Query( + array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$search_relevance_post_ids, + 's' => 'search test', + 'orderby' => 'relevance', + 'order' => 'DESC', + 'posts_per_page' => 10, + 'paged' => 1, + ) + ); + $page1_repeat_ids = wp_list_pluck( $query1_repeat->posts, 'ID' ); + + $this->assertEquals( $page1_ids, $page1_repeat_ids, 'Same query should return same results when ordering by search relevance' ); + } + + /** + * Test that deterministic ordering works with search when orderby is empty (defaults to relevance). + * + * When orderby is empty and search is present, WordPress orders by relevance. + * Multiple posts can have the same relevance score, causing duplicate records across pages. + * + * @ticket xxxxx + */ + public function test_deterministic_ordering_with_search_empty_orderby() { + // Use shared fixtures with identical content (same relevance scores) + // Get first page with empty orderby (defaults to relevance) + $query1 = new WP_Query( + array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$search_relevance_post_ids, + 's' => 'search test', + 'orderby' => '', // Empty orderby with search defaults to relevance + 'order' => 'DESC', + 'posts_per_page' => 10, + 'paged' => 1, + ) + ); + + // Get second page with empty orderby + $query2 = new WP_Query( + array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$search_relevance_post_ids, + 's' => 'search test', + 'orderby' => '', // Empty orderby with search defaults to relevance + 'order' => 'DESC', + 'posts_per_page' => 10, + 'paged' => 2, + ) + ); + + $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); + $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); + + // Verify no overlap between pages (no duplicates) + $overlap = array_intersect( $page1_ids, $page2_ids ); + $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts when ordering by search relevance (empty orderby)' ); + + // Verify total count is correct + $this->assertEquals( 20, $query1->found_posts, 'Total posts should be 20' ); + $this->assertEquals( 10, count( $page1_ids ), 'First page should have 10 posts' ); + $this->assertEquals( 10, count( $page2_ids ), 'Second page should have 10 posts' ); + } } From e9df9e1b94655df4d4acdac2ba04deecec0a2814 Mon Sep 17 00:00:00 2001 From: Ramon Date: Sat, 6 Dec 2025 00:15:08 +1100 Subject: [PATCH 17/36] Enhance WP_Query ordering by adding new fields to the orderby array. Included 'post__in', 'post_name__in', 'post_parent__in', and 'include' to improve query flexibility and support additional ordering scenarios. --- src/wp-includes/class-wp-query.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 6a56caa3751a3..4e440f81a3cfa 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -2539,6 +2539,10 @@ public function get_posts() { 'ID', 'rand', 'relevance', + 'post__in', + 'post_name__in', + 'post_parent__in', + 'include', ); $orderby_array = array(); From 3d355d76f2557c12f32e4859f6773aa40f692096 Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 31 Dec 2025 15:52:01 +1100 Subject: [PATCH 18/36] Implement deterministic ordering in WP_Query by adding ID tie-breaker after filters. Updated logic to ensure filters receive the original orderby value, maintaining backward compatibility. Added tests to verify correct behavior when filters modify orderby and prevent duplicate posts across paginated results. --- src/wp-includes/class-wp-query.php | 63 +++-- .../tests/query/deterministicOrdering.php | 219 ++++++++++++++++++ 2 files changed, 263 insertions(+), 19 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 4e440f81a3cfa..47f9f2adc3195 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -2506,6 +2506,13 @@ public function get_posts() { } // Order by. + // Store metadata for deterministic ordering to be applied after filters. + $deterministic_orderby_meta = array( + 'needed' => false, + 'has_id' => false, + 'order' => $query_vars['order'], + ); + if ( empty( $query_vars['orderby'] ) ) { /* * Boolean false or empty array blanks out ORDER BY, @@ -2518,13 +2525,16 @@ public function get_posts() { * Ensure deterministic ordering to prevent duplicate records across pages. * When multiple posts have the same value for a field, add ID as secondary sort to guarantee consistent ordering. * Note: this is to circumvent a bug that is currently being tracked in https://core.trac.wordpress.org/ticket/44349. + * + * Build base orderby without ID tie-breaker for filters, then add it after filters. */ - $orderby = "{$wpdb->posts}.post_date " . $query_vars['order'] . ', ' . "{$wpdb->posts}.ID " . $query_vars['order']; + $orderby = "{$wpdb->posts}.post_date " . $query_vars['order']; + $deterministic_orderby_meta['needed'] = true; } // See get_pages(): when sort_column is 'none', the get_pages() function should not generate any ORDER BY clause. // Should it rather be handled in the get_pages() function? // src/wp-includes/post.php L6496 - } elseif ( 'none' === $query_vars['orderby'] || ( is_array( $query_vars['orderby'] ) && array_key_exists( 'none', $query_vars['orderby'] ) ) ) { + } elseif ( 'none' === $query_vars['orderby'] || isset( $query_vars['orderby']['none'] ) ) { $orderby = ''; } else { /* @@ -2534,6 +2544,8 @@ public function get_posts() { * * Use a blacklist approach: add ID as tie-breaker for all orderby fields except those that are * already deterministic (ID itself, random ordering, or search relevance). + * + * Build base orderby without ID tie-breaker for filters, then add it after filters. */ $fields_excluding_deterministic_orderby = array( 'ID', @@ -2545,10 +2557,7 @@ public function get_posts() { 'include', ); - $orderby_array = array(); - $needs_deterministic_orderby = false; - $has_id_orderby = false; - $id_tie_breaker_order = $query_vars['order']; // Default to global order + $orderby_array = array(); if ( is_array( $query_vars['orderby'] ) ) { foreach ( $query_vars['orderby'] as $_orderby => $order ) { @@ -2563,11 +2572,11 @@ public function get_posts() { // Check if this field should have deterministic ordering (not in blacklist). if ( ! in_array( $_orderby, $fields_excluding_deterministic_orderby, true ) ) { - $needs_deterministic_orderby = true; + $deterministic_orderby_meta['needed'] = true; // Use the order from the array for ID tie-breaker. - $id_tie_breaker_order = $this->parse_order( $order ); + $deterministic_orderby_meta['order'] = $this->parse_order( $order ); } elseif ( 'ID' === $_orderby ) { - $has_id_orderby = true; + $deterministic_orderby_meta['has_id'] = true; } } } else { @@ -2585,21 +2594,17 @@ public function get_posts() { // Check if this field should have deterministic ordering (not in blacklist). if ( ! in_array( $orderby, $fields_excluding_deterministic_orderby, true ) ) { - $needs_deterministic_orderby = true; + $deterministic_orderby_meta['needed'] = true; } elseif ( 'ID' === $orderby ) { - $has_id_orderby = true; + $deterministic_orderby_meta['has_id'] = true; } } } - // Add ID as tie-breaker if needed and not already present. - if ( $needs_deterministic_orderby && ! $has_id_orderby ) { - $orderby_array[] = "{$wpdb->posts}.ID " . $id_tie_breaker_order; - } - - // Build the final orderby string. + // Build the base orderby string (without ID tie-breaker) for filters. if ( empty( $orderby_array ) ) { - $orderby = "{$wpdb->posts}.post_date " . $query_vars['order'] . ', ' . "{$wpdb->posts}.ID " . $query_vars['order']; + $orderby = "{$wpdb->posts}.post_date " . $query_vars['order']; + $deterministic_orderby_meta['needed'] = true; } else { $orderby = trim( implode( ', ', $orderby_array ) ); } @@ -3218,12 +3223,31 @@ public function get_posts() { $where = $clauses['where'] ?? ''; $groupby = $clauses['groupby'] ?? ''; $join = $clauses['join'] ?? ''; - $orderby = $clauses['orderby'] ?? ''; + // Preserve orderby from posts_orderby_request if posts_clauses_request doesn't provide one. + $orderby = $clauses['orderby'] ?? $orderby; $distinct = $clauses['distinct'] ?? ''; $fields = $clauses['fields'] ?? ''; $limits = $clauses['limits'] ?? ''; } + /* + * Ensure deterministic ordering to prevent duplicate records across pages. + * Add ID tie-breaker after filters have been applied, so filters receive + * the original orderby value (for backward compatibility) and the tie-breaker + * is preserved even if filters modify the orderby. + * + * Note: this is to circumvent a bug that is currently being tracked in + * https://core.trac.wordpress.org/ticket/44349. + */ + if ( ! empty( $orderby ) && $deterministic_orderby_meta['needed'] ) { + // Check if ID tie-breaker is already present in the orderby string. + $id_tie_breaker_pattern = '/\b' . preg_quote( $wpdb->posts, '/' ) . '\.ID\b/i'; + if ( ! preg_match( $id_tie_breaker_pattern, $orderby ) ) { + // Add ID as tie-breaker at the end. + $orderby .= ', ' . "{$wpdb->posts}.ID " . $deterministic_orderby_meta['order']; + } + } + if ( ! empty( $groupby ) ) { $groupby = 'GROUP BY ' . $groupby; } @@ -5225,3 +5249,4 @@ public function lazyload_comment_meta( $check, $comment_id ) { return $check; } } + diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index 1f6941e9fbf6e..e4a3cedc47619 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -633,4 +633,223 @@ public function test_deterministic_ordering_with_search_empty_orderby() { $this->assertEquals( 10, count( $page1_ids ), 'First page should have 10 posts' ); $this->assertEquals( 10, count( $page2_ids ), 'Second page should have 10 posts' ); } + + /** + * Test that filters receive the original orderby value (without ID tie-breaker). + * + * This ensures backward compatibility - filters should receive the same orderby + * value they received before the deterministic ordering changes. + * + * @ticket xxxxx + */ + public function test_filters_receive_original_orderby() { + global $wpdb; + + $received_orderby = ''; + + // Capture the orderby value received by the filter. + $filter_callback = function( $orderby ) use ( &$received_orderby ) { + $received_orderby = $orderby; + return $orderby; + }; + + add_filter( 'posts_orderby', $filter_callback ); + + $query = new WP_Query( + array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 10, + ) + ); + + remove_filter( 'posts_orderby', $filter_callback ); + + // Filter should receive orderby without ID tie-breaker. + $expected_orderby = "{$wpdb->posts}.post_date ASC"; + $this->assertEquals( $expected_orderby, $received_orderby, 'Filter should receive original orderby without ID tie-breaker' ); + + // But the final query should still have ID tie-breaker for deterministic ordering. + $this->assertStringContainsString( 'ID ASC', $query->request, 'Final query should have ID tie-breaker' ); + } + + /** + * Test that posts_clauses filter receives original orderby (without ID tie-breaker). + * + * @ticket xxxxx + */ + public function test_posts_clauses_filter_receives_original_orderby() { + global $wpdb; + + $received_orderby = ''; + + // Capture the orderby value received by the filter. + $filter_callback = function( $clauses ) use ( &$received_orderby ) { + $received_orderby = $clauses['orderby'] ?? ''; + return $clauses; + }; + + add_filter( 'posts_clauses', $filter_callback ); + + $query = new WP_Query( + array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 10, + ) + ); + + remove_filter( 'posts_clauses', $filter_callback ); + + // Filter should receive orderby without ID tie-breaker. + $expected_orderby = "{$wpdb->posts}.post_date ASC"; + $this->assertEquals( $expected_orderby, $received_orderby, 'posts_clauses filter should receive original orderby without ID tie-breaker' ); + + // But the final query should still have ID tie-breaker. + $this->assertStringContainsString( 'ID ASC', $query->request, 'Final query should have ID tie-breaker' ); + } + + /** + * Test that deterministic ordering works when filters modify orderby. + * + * Even if a filter modifies the orderby, the ID tie-breaker should still + * be added after the filter to ensure deterministic ordering. + * + * @ticket xxxxx + */ + public function test_deterministic_ordering_works_after_filter_modifies_orderby() { + // Filter that modifies the orderby. + $filter_callback = function( $orderby ) { + // Add a custom field to the orderby. + global $wpdb; + return $orderby . ', ' . "{$wpdb->posts}.post_title ASC"; + }; + + add_filter( 'posts_orderby', $filter_callback ); + + $query1 = new WP_Query( + array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 10, + 'paged' => 1, + ) + ); + + $query2 = new WP_Query( + array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 10, + 'paged' => 2, + ) + ); + + remove_filter( 'posts_orderby', $filter_callback ); + + $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); + $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); + + // Verify no duplicates across pages even when filter modifies orderby. + $overlap = array_intersect( $page1_ids, $page2_ids ); + $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts even when filter modifies orderby' ); + + // Verify ID tie-breaker is still present in the final query. + $this->assertStringContainsString( 'ID ASC', $query1->request, 'ID tie-breaker should be present after filter modifies orderby' ); + } + + /** + * Test that deterministic ordering works when posts_clauses filter modifies orderby. + * + * @ticket xxxxx + */ + public function test_deterministic_ordering_works_after_posts_clauses_modifies_orderby() { + // Filter that modifies the orderby via posts_clauses. + $filter_callback = function( $clauses ) { + global $wpdb; + // Modify orderby to add post_title. + $clauses['orderby'] = "{$wpdb->posts}.post_date ASC, {$wpdb->posts}.post_title ASC"; + return $clauses; + }; + + add_filter( 'posts_clauses', $filter_callback ); + + $query1 = new WP_Query( + array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 10, + 'paged' => 1, + ) + ); + + $query2 = new WP_Query( + array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 10, + 'paged' => 2, + ) + ); + + remove_filter( 'posts_clauses', $filter_callback ); + + $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); + $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); + + // Verify no duplicates across pages. + $overlap = array_intersect( $page1_ids, $page2_ids ); + $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts even when posts_clauses modifies orderby' ); + + // Verify ID tie-breaker is still present. + $this->assertStringContainsString( 'ID ASC', $query1->request, 'ID tie-breaker should be present after posts_clauses modifies orderby' ); + } + + /** + * Test that ID tie-breaker is not duplicated when filter already includes ID. + * + * If a filter adds ID to the orderby, we should not add it again. + * + * @ticket xxxxx + */ + public function test_id_tie_breaker_not_duplicated_when_filter_includes_id() { + global $wpdb; + + // Filter that already includes ID in orderby. + $filter_callback = function( $orderby ) use ( $wpdb ) { + return "{$wpdb->posts}.post_date ASC, {$wpdb->posts}.ID ASC"; + }; + + add_filter( 'posts_orderby', $filter_callback ); + + $query = new WP_Query( + array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 10, + ) + ); + + remove_filter( 'posts_orderby', $filter_callback ); + + // Should not have duplicate ID ordering. + $this->assertStringContainsString( 'ID ASC', $query->request, 'ID should be present' ); + // Count occurrences of "ID ASC" - should be exactly 1. + $id_count = substr_count( $query->request, 'ID ASC' ); + $this->assertEquals( 1, $id_count, 'ID should not be duplicated when filter already includes it' ); + } } From fbe02eeab88e4527993ae3ed8560e206b4f66fb0 Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 31 Dec 2025 16:39:52 +1100 Subject: [PATCH 19/36] lint --- src/wp-includes/class-wp-query.php | 23 +++++++++---------- .../tests/query/deterministicOrdering.php | 10 ++++---- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 47f9f2adc3195..bd7550e412e97 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -2508,9 +2508,9 @@ public function get_posts() { // Order by. // Store metadata for deterministic ordering to be applied after filters. $deterministic_orderby_meta = array( - 'needed' => false, - 'has_id' => false, - 'order' => $query_vars['order'], + 'needed' => false, + 'has_id' => false, + 'order' => $query_vars['order'], ); if ( empty( $query_vars['orderby'] ) ) { @@ -2525,10 +2525,10 @@ public function get_posts() { * Ensure deterministic ordering to prevent duplicate records across pages. * When multiple posts have the same value for a field, add ID as secondary sort to guarantee consistent ordering. * Note: this is to circumvent a bug that is currently being tracked in https://core.trac.wordpress.org/ticket/44349. - * + * * Build base orderby without ID tie-breaker for filters, then add it after filters. */ - $orderby = "{$wpdb->posts}.post_date " . $query_vars['order']; + $orderby = "{$wpdb->posts}.post_date " . $query_vars['order']; $deterministic_orderby_meta['needed'] = true; } // See get_pages(): when sort_column is 'none', the get_pages() function should not generate any ORDER BY clause. @@ -2544,7 +2544,7 @@ public function get_posts() { * * Use a blacklist approach: add ID as tie-breaker for all orderby fields except those that are * already deterministic (ID itself, random ordering, or search relevance). - * + * * Build base orderby without ID tie-breaker for filters, then add it after filters. */ $fields_excluding_deterministic_orderby = array( @@ -2603,7 +2603,7 @@ public function get_posts() { // Build the base orderby string (without ID tie-breaker) for filters. if ( empty( $orderby_array ) ) { - $orderby = "{$wpdb->posts}.post_date " . $query_vars['order']; + $orderby = "{$wpdb->posts}.post_date " . $query_vars['order']; $deterministic_orderby_meta['needed'] = true; } else { $orderby = trim( implode( ', ', $orderby_array ) ); @@ -3220,9 +3220,9 @@ public function get_posts() { */ $clauses = (array) apply_filters_ref_array( 'posts_clauses_request', array( compact( $pieces ), &$this ) ); - $where = $clauses['where'] ?? ''; - $groupby = $clauses['groupby'] ?? ''; - $join = $clauses['join'] ?? ''; + $where = $clauses['where'] ?? ''; + $groupby = $clauses['groupby'] ?? ''; + $join = $clauses['join'] ?? ''; // Preserve orderby from posts_orderby_request if posts_clauses_request doesn't provide one. $orderby = $clauses['orderby'] ?? $orderby; $distinct = $clauses['distinct'] ?? ''; @@ -3235,7 +3235,7 @@ public function get_posts() { * Add ID tie-breaker after filters have been applied, so filters receive * the original orderby value (for backward compatibility) and the tie-breaker * is preserved even if filters modify the orderby. - * + * * Note: this is to circumvent a bug that is currently being tracked in * https://core.trac.wordpress.org/ticket/44349. */ @@ -5249,4 +5249,3 @@ public function lazyload_comment_meta( $check, $comment_id ) { return $check; } } - diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index e4a3cedc47619..a2d8d943f17ba 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -648,7 +648,7 @@ public function test_filters_receive_original_orderby() { $received_orderby = ''; // Capture the orderby value received by the filter. - $filter_callback = function( $orderby ) use ( &$received_orderby ) { + $filter_callback = function ( $orderby ) use ( &$received_orderby ) { $received_orderby = $orderby; return $orderby; }; @@ -686,7 +686,7 @@ public function test_posts_clauses_filter_receives_original_orderby() { $received_orderby = ''; // Capture the orderby value received by the filter. - $filter_callback = function( $clauses ) use ( &$received_orderby ) { + $filter_callback = function ( $clauses ) use ( &$received_orderby ) { $received_orderby = $clauses['orderby'] ?? ''; return $clauses; }; @@ -723,7 +723,7 @@ public function test_posts_clauses_filter_receives_original_orderby() { */ public function test_deterministic_ordering_works_after_filter_modifies_orderby() { // Filter that modifies the orderby. - $filter_callback = function( $orderby ) { + $filter_callback = function ( $orderby ) { // Add a custom field to the orderby. global $wpdb; return $orderby . ', ' . "{$wpdb->posts}.post_title ASC"; @@ -773,7 +773,7 @@ public function test_deterministic_ordering_works_after_filter_modifies_orderby( */ public function test_deterministic_ordering_works_after_posts_clauses_modifies_orderby() { // Filter that modifies the orderby via posts_clauses. - $filter_callback = function( $clauses ) { + $filter_callback = function ( $clauses ) { global $wpdb; // Modify orderby to add post_title. $clauses['orderby'] = "{$wpdb->posts}.post_date ASC, {$wpdb->posts}.post_title ASC"; @@ -828,7 +828,7 @@ public function test_id_tie_breaker_not_duplicated_when_filter_includes_id() { global $wpdb; // Filter that already includes ID in orderby. - $filter_callback = function( $orderby ) use ( $wpdb ) { + $filter_callback = function ( $orderby ) use ( $wpdb ) { return "{$wpdb->posts}.post_date ASC, {$wpdb->posts}.ID ASC"; }; From 8ab6a795968864acfbfa8b99cff838d361812f19 Mon Sep 17 00:00:00 2001 From: Ramon Date: Fri, 2 Jan 2026 10:34:28 +1100 Subject: [PATCH 20/36] Refactor REST API post ordering tests to remove ID tie-breaker from assertions. Added a new test to verify the inclusion of ID tie-breaker in the final SQL query for deterministic ordering, ensuring backward compatibility with filters. --- .../tests/rest-api/rest-posts-controller.php | 92 +++++++++++++++++-- 1 file changed, 83 insertions(+), 9 deletions(-) diff --git a/tests/phpunit/tests/rest-api/rest-posts-controller.php b/tests/phpunit/tests/rest-api/rest-posts-controller.php index 38a43fef488c7..6d21498848ff7 100644 --- a/tests/phpunit/tests/rest-api/rest-posts-controller.php +++ b/tests/phpunit/tests/rest-api/rest-posts-controller.php @@ -488,7 +488,7 @@ public function test_get_items_include_query( $method ) { $this->assertSame( 2, $headers['X-WP-Total'], 'Failed asserting that the number of posts is correct.' ); } - $this->assertPostsOrderedBy( '{posts}.post_date DESC, {posts}.ID DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_date DESC' ); // 'orderby' => 'include'. $request->set_param( 'orderby', 'include' ); @@ -544,7 +544,7 @@ public function test_get_items_orderby_author_query() { $this->assertSame( self::$editor_id, $data[1]['author'] ); $this->assertSame( self::$editor_id, $data[2]['author'] ); - $this->assertPostsOrderedBy( '{posts}.post_author DESC, {posts}.ID DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_author DESC' ); } public function test_get_items_orderby_modified_query() { @@ -568,7 +568,7 @@ public function test_get_items_orderby_modified_query() { $this->assertSame( $id3, $data[1]['id'] ); $this->assertSame( $id2, $data[2]['id'] ); - $this->assertPostsOrderedBy( '{posts}.post_modified DESC, {posts}.ID DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_modified DESC' ); } public function test_get_items_orderby_parent_query() { @@ -606,7 +606,7 @@ public function test_get_items_orderby_parent_query() { $this->assertSame( 0, $data[1]['parent'] ); $this->assertSame( 0, $data[2]['parent'] ); - $this->assertPostsOrderedBy( '{posts}.post_parent DESC, {posts}.ID DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_parent DESC' ); } public function test_get_items_exclude_query() { @@ -976,14 +976,14 @@ public function test_get_items_order_and_orderby() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); $this->assertSame( 'Apple Sauce', $data[0]['title']['rendered'] ); - $this->assertPostsOrderedBy( '{posts}.post_title DESC, {posts}.ID DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_title DESC' ); // 'order' => 'asc'. $request->set_param( 'order', 'asc' ); $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); $this->assertSame( 'Apple Cobbler', $data[0]['title']['rendered'] ); - $this->assertPostsOrderedBy( '{posts}.post_title ASC, {posts}.ID ASC' ); + $this->assertPostsOrderedBy( '{posts}.post_title ASC' ); // 'order' => 'asc,id' should error. $request->set_param( 'order', 'asc,id' ); @@ -1068,7 +1068,7 @@ public function test_get_items_with_orderby_slug() { // Default ORDER is DESC. $this->assertSame( 'xyz', $data[0]['slug'] ); $this->assertSame( 'abc', $data[1]['slug'] ); - $this->assertPostsOrderedBy( '{posts}.post_name DESC, {posts}.ID DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_name DESC' ); } public function test_get_items_with_orderby_slugs() { @@ -1120,7 +1120,7 @@ public function test_get_items_with_orderby_relevance() { $this->assertCount( 2, $data ); $this->assertSame( $id1, $data[0]['id'] ); $this->assertSame( $id2, $data[1]['id'] ); - $this->assertPostsOrderedBy( '{posts}.post_title LIKE \'%relevant%\' DESC, {posts}.post_date DESC, {posts}.ID DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_title LIKE \'%relevant%\' DESC, {posts}.post_date DESC' ); } public function test_get_items_with_orderby_relevance_two_terms() { @@ -1148,7 +1148,7 @@ public function test_get_items_with_orderby_relevance_two_terms() { $this->assertCount( 2, $data ); $this->assertSame( $id1, $data[0]['id'] ); $this->assertSame( $id2, $data[1]['id'] ); - $this->assertPostsOrderedBy( '(CASE WHEN {posts}.post_title LIKE \'%relevant content%\' THEN 1 WHEN {posts}.post_title LIKE \'%relevant%\' AND {posts}.post_title LIKE \'%content%\' THEN 2 WHEN {posts}.post_title LIKE \'%relevant%\' OR {posts}.post_title LIKE \'%content%\' THEN 3 WHEN {posts}.post_excerpt LIKE \'%relevant content%\' THEN 4 WHEN {posts}.post_content LIKE \'%relevant content%\' THEN 5 ELSE 6 END), {posts}.post_date DESC, {posts}.ID DESC' ); + $this->assertPostsOrderedBy( '(CASE WHEN {posts}.post_title LIKE \'%relevant content%\' THEN 1 WHEN {posts}.post_title LIKE \'%relevant%\' AND {posts}.post_title LIKE \'%content%\' THEN 2 WHEN {posts}.post_title LIKE \'%relevant%\' OR {posts}.post_title LIKE \'%content%\' THEN 3 WHEN {posts}.post_excerpt LIKE \'%relevant content%\' THEN 4 WHEN {posts}.post_content LIKE \'%relevant content%\' THEN 5 ELSE 6 END), {posts}.post_date DESC' ); } public function test_get_items_with_orderby_relevance_missing_search() { @@ -1158,6 +1158,80 @@ public function test_get_items_with_orderby_relevance_missing_search() { $this->assertErrorResponse( 'rest_no_search_term_defined', $response, 400 ); } + /** + * Test that ID tie-breaker is added to final SQL query for deterministic ordering. + * + * This test verifies that the ID tie-breaker is present in the final SQL query, + * even though filters receive the orderby without ID (for backward compatibility). + * + * @ticket xxxxx + */ + public function test_id_tie_breaker_in_final_sql_query() { + global $wpdb; + + $identical_date = '2023-01-01 10:00:00'; + $post_ids = array(); + for ( $i = 1; $i <= 5; $i++ ) { + $post_ids[] = self::factory()->post->create( + array( + 'post_status' => 'publish', + 'post_date' => $identical_date, + ) + ); + } + + /* + * Capture the WP_Query instance via posts_clauses filter. + * We use the same hook as other tests in this class (save_posts_clauses), + * but we need to capture the query instance to access $query->request after execution. + * The existing save_posts_clauses method stores clauses but not the query instance. + */ + $captured_query = null; + $filter_callback = function ( $clauses, $query ) use ( &$captured_query ) { + /* + * Short-circuit: only capture the query on the first call. + * The posts_clauses filter may be called multiple times (e.g., for main query + * and sub-queries), but we only need the main query instance once. + */ + if ( null === $captured_query ) { + $captured_query = $query; + } + return $clauses; + }; + add_filter( 'posts_clauses', $filter_callback, 10, 2 ); + + $request = new WP_REST_Request( 'GET', '/wp/v2/posts' ); + $request->set_param( 'order', 'desc' ); + $request->set_param( 'per_page', 100 ); + + $response = rest_get_server()->dispatch( $request ); + + remove_filter( 'posts_clauses', $filter_callback ); + + $this->assertSame( 200, $response->get_status() ); + $this->assertNotNull( $captured_query, 'WP_Query should be captured' ); + $this->assertInstanceOf( 'WP_Query', $captured_query, 'Captured query should be a WP_Query instance' ); + + /** @var WP_Query $captured_query */ + $sql = $captured_query->request; + $posts_table = preg_quote( $wpdb->posts, '/' ); + + $orderby_pattern = '/ORDER\s+BY\s+.*' . $posts_table . '\.ID\s+(?:ASC|DESC)/i'; + $this->assertMatchesRegularExpression( + $orderby_pattern, + $sql, + 'Final SQL query should include ID tie-breaker in ORDER BY clause' + ); + + $this->assertCount( 1, $this->posts_clauses ); + $filter_orderby = $this->posts_clauses[0]['orderby']; + $this->assertStringNotContainsString( + 'ID', + $filter_orderby, + 'Filters should receive orderby without ID tie-breaker for backward compatibility' + ); + } + public function test_get_items_offset_query() { $request = new WP_REST_Request( 'GET', '/wp/v2/posts' ); $request->set_param( 'per_page', self::$per_page ); From 81832285186246b418c579f3717b35d319c6f695 Mon Sep 17 00:00:00 2001 From: Ramon Date: Fri, 2 Jan 2026 10:48:25 +1100 Subject: [PATCH 21/36] Refactor WP_Query to preserve filter modifications to orderby. Adjusted related tests to verify that filter modifications are respected. --- src/wp-includes/class-wp-query.php | 26 +++++-- .../tests/query/deterministicOrdering.php | 75 ++++++------------- 2 files changed, 41 insertions(+), 60 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index bd7550e412e97..b5becf0948364 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -2505,12 +2505,15 @@ public function get_posts() { $query_vars['order'] = ''; } - // Order by. - // Store metadata for deterministic ordering to be applied after filters. + /* + * Order by. + * Store metadata for deterministic ordering to be applied after filters. + */ $deterministic_orderby_meta = array( - 'needed' => false, - 'has_id' => false, - 'order' => $query_vars['order'], + 'needed' => false, + 'has_id' => false, + 'order' => $query_vars['order'], + 'original' => '', // Store original orderby to detect filter modifications. ); if ( empty( $query_vars['orderby'] ) ) { @@ -2634,6 +2637,11 @@ public function get_posts() { } } + // Store the original orderby after all core modifications but before filters modify it. + if ( $deterministic_orderby_meta['needed'] ) { + $deterministic_orderby_meta['original'] = $orderby; + } + if ( is_array( $post_type ) && count( $post_type ) > 1 ) { $post_type_cap = 'multiple_post_type'; } else { @@ -3240,9 +3248,11 @@ public function get_posts() { * https://core.trac.wordpress.org/ticket/44349. */ if ( ! empty( $orderby ) && $deterministic_orderby_meta['needed'] ) { - // Check if ID tie-breaker is already present in the orderby string. - $id_tie_breaker_pattern = '/\b' . preg_quote( $wpdb->posts, '/' ) . '\.ID\b/i'; - if ( ! preg_match( $id_tie_breaker_pattern, $orderby ) ) { + /* + * Only add ID tie-breaker if no filter modified the orderby. + * If a filter modified it, we assume they know what they're doing and don't interfere. + */ + if ( ! empty( $deterministic_orderby_meta['original'] ) && $orderby === $deterministic_orderby_meta['original'] ) { // Add ID as tie-breaker at the end. $orderby .= ', ' . "{$wpdb->posts}.ID " . $deterministic_orderby_meta['order']; } diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index a2d8d943f17ba..9c98b8f0c8d1f 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -714,67 +714,54 @@ public function test_posts_clauses_filter_receives_original_orderby() { } /** - * Test that deterministic ordering works when filters modify orderby. + * Test that filter modifications to orderby are preserved. * - * Even if a filter modifies the orderby, the ID tie-breaker should still - * be added after the filter to ensure deterministic ordering. + * When a filter modifies the orderby, the modification should be preserved + * and we should not add the ID tie-breaker (we assume the filter knows what it's doing). * * @ticket xxxxx */ - public function test_deterministic_ordering_works_after_filter_modifies_orderby() { - // Filter that modifies the orderby. + public function test_filter_modifications_to_orderby_are_preserved() { + // Filter that modifies the orderby by adding post_title. $filter_callback = function ( $orderby ) { - // Add a custom field to the orderby. global $wpdb; return $orderby . ', ' . "{$wpdb->posts}.post_title ASC"; }; add_filter( 'posts_orderby', $filter_callback ); - $query1 = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$date_identical_post_ids, - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 10, - 'paged' => 1, - ) - ); - - $query2 = new WP_Query( + $query = new WP_Query( array( 'post_type' => 'wptests_time_ident', 'post__in' => self::$date_identical_post_ids, 'orderby' => 'post_date', 'order' => 'ASC', 'posts_per_page' => 10, - 'paged' => 2, ) ); remove_filter( 'posts_orderby', $filter_callback ); - $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); - $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); - - // Verify no duplicates across pages even when filter modifies orderby. - $overlap = array_intersect( $page1_ids, $page2_ids ); - $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts even when filter modifies orderby' ); + // Verify filter modification is preserved in the final query. + $this->assertStringContainsString( 'post_title ASC', $query->request, 'Filter modification to orderby should be preserved' ); - // Verify ID tie-breaker is still present in the final query. - $this->assertStringContainsString( 'ID ASC', $query1->request, 'ID tie-breaker should be present after filter modifies orderby' ); + // Verify ID tie-breaker is NOT added when filter modifies orderby. + $this->assertStringNotContainsString( ', ' . $GLOBALS['wpdb']->posts . '.ID ASC', $query->request, 'ID tie-breaker should not be added when filter modifies orderby' ); } /** - * Test that deterministic ordering works when posts_clauses filter modifies orderby. + * Test that posts_clauses filter modifications to orderby are preserved. + * + * When a posts_clauses filter modifies the orderby, the modification should be preserved + * and we should not add the ID tie-breaker (we assume the filter knows what it's doing). * * @ticket xxxxx */ - public function test_deterministic_ordering_works_after_posts_clauses_modifies_orderby() { + public function test_posts_clauses_filter_modifications_to_orderby_are_preserved() { + global $wpdb; + // Filter that modifies the orderby via posts_clauses. - $filter_callback = function ( $clauses ) { - global $wpdb; + $filter_callback = function ( $clauses ) use ( $wpdb ) { // Modify orderby to add post_title. $clauses['orderby'] = "{$wpdb->posts}.post_date ASC, {$wpdb->posts}.post_title ASC"; return $clauses; @@ -782,39 +769,23 @@ public function test_deterministic_ordering_works_after_posts_clauses_modifies_o add_filter( 'posts_clauses', $filter_callback ); - $query1 = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$date_identical_post_ids, - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 10, - 'paged' => 1, - ) - ); - - $query2 = new WP_Query( + $query = new WP_Query( array( 'post_type' => 'wptests_time_ident', 'post__in' => self::$date_identical_post_ids, 'orderby' => 'post_date', 'order' => 'ASC', 'posts_per_page' => 10, - 'paged' => 2, ) ); remove_filter( 'posts_clauses', $filter_callback ); - $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); - $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); - - // Verify no duplicates across pages. - $overlap = array_intersect( $page1_ids, $page2_ids ); - $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts even when posts_clauses modifies orderby' ); + // Verify filter modification is preserved in the final query. + $this->assertStringContainsString( 'post_title ASC', $query->request, 'posts_clauses filter modification to orderby should be preserved' ); - // Verify ID tie-breaker is still present. - $this->assertStringContainsString( 'ID ASC', $query1->request, 'ID tie-breaker should be present after posts_clauses modifies orderby' ); + // Verify ID tie-breaker is NOT added when filter modifies orderby. + $this->assertStringNotContainsString( ', ' . $wpdb->posts . '.ID ASC', $query->request, 'ID tie-breaker should not be added when posts_clauses filter modifies orderby' ); } /** From 4896bcab4f44d6a8047ac55b5a7078d74cfef88d Mon Sep 17 00:00:00 2001 From: Ramon Date: Fri, 2 Jan 2026 11:06:05 +1100 Subject: [PATCH 22/36] Separate units in tests for posts_orderby and posts_clauses filter behavior. --- .../tests/query/deterministicOrdering.php | 75 ++++++++++++++++--- 1 file changed, 65 insertions(+), 10 deletions(-) diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index 9c98b8f0c8d1f..eeebe134ed26e 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -635,19 +635,18 @@ public function test_deterministic_ordering_with_search_empty_orderby() { } /** - * Test that filters receive the original orderby value (without ID tie-breaker). + * Test that posts_orderby filter receives original orderby value. * * This ensures backward compatibility - filters should receive the same orderby * value they received before the deterministic ordering changes. * * @ticket xxxxx */ - public function test_filters_receive_original_orderby() { + public function test_posts_orderby_filter_receives_original_orderby() { global $wpdb; $received_orderby = ''; - // Capture the orderby value received by the filter. $filter_callback = function ( $orderby ) use ( &$received_orderby ) { $received_orderby = $orderby; return $orderby; @@ -669,14 +668,44 @@ public function test_filters_receive_original_orderby() { // Filter should receive orderby without ID tie-breaker. $expected_orderby = "{$wpdb->posts}.post_date ASC"; - $this->assertEquals( $expected_orderby, $received_orderby, 'Filter should receive original orderby without ID tie-breaker' ); + $this->assertEquals( $expected_orderby, $received_orderby, 'posts_orderby filter should receive original orderby without ID tie-breaker' ); + } + + /** + * Test that ID tie-breaker is added when posts_orderby filter does not modify orderby. + * + * @ticket xxxxx + */ + public function test_id_tie_breaker_added_when_posts_orderby_filter_does_not_modify() { + global $wpdb; - // But the final query should still have ID tie-breaker for deterministic ordering. - $this->assertStringContainsString( 'ID ASC', $query->request, 'Final query should have ID tie-breaker' ); + $filter_callback = function ( $orderby ) { + return $orderby; // Return unchanged. + }; + + add_filter( 'posts_orderby', $filter_callback ); + + $query = new WP_Query( + array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 10, + ) + ); + + remove_filter( 'posts_orderby', $filter_callback ); + + // Since filter did NOT modify orderby, ID tie-breaker SHOULD be added. + $this->assertStringContainsString( ', ' . $wpdb->posts . '.ID ASC', $query->request, 'ID tie-breaker should be added when posts_orderby filter does not modify orderby' ); } /** - * Test that posts_clauses filter receives original orderby (without ID tie-breaker). + * Test that posts_clauses filter receives original orderby value. + * + * This ensures backward compatibility - filters should receive the same orderby + * value they received before the deterministic ordering changes. * * @ticket xxxxx */ @@ -685,7 +714,6 @@ public function test_posts_clauses_filter_receives_original_orderby() { $received_orderby = ''; - // Capture the orderby value received by the filter. $filter_callback = function ( $clauses ) use ( &$received_orderby ) { $received_orderby = $clauses['orderby'] ?? ''; return $clauses; @@ -708,9 +736,36 @@ public function test_posts_clauses_filter_receives_original_orderby() { // Filter should receive orderby without ID tie-breaker. $expected_orderby = "{$wpdb->posts}.post_date ASC"; $this->assertEquals( $expected_orderby, $received_orderby, 'posts_clauses filter should receive original orderby without ID tie-breaker' ); + } + + /** + * Test that ID tie-breaker is added when posts_clauses filter does not modify orderby. + * + * @ticket xxxxx + */ + public function test_id_tie_breaker_added_when_posts_clauses_filter_does_not_modify() { + global $wpdb; + + $filter_callback = function ( $clauses ) { + return $clauses; // Return unchanged. + }; + + add_filter( 'posts_clauses', $filter_callback ); + + $query = new WP_Query( + array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, + 'orderby' => 'post_date', + 'order' => 'ASC', + 'posts_per_page' => 10, + ) + ); + + remove_filter( 'posts_clauses', $filter_callback ); - // But the final query should still have ID tie-breaker. - $this->assertStringContainsString( 'ID ASC', $query->request, 'Final query should have ID tie-breaker' ); + // Since filter did NOT modify orderby, ID tie-breaker SHOULD be added. + $this->assertStringContainsString( ', ' . $wpdb->posts . '.ID ASC', $query->request, 'ID tie-breaker should be added when posts_clauses filter does not modify orderby' ); } /** From 3bf4883afaaaf12d75407ced7c5ebb06d33b2c74 Mon Sep 17 00:00:00 2001 From: Ramon Date: Tue, 1 Sep 2026 17:05:23 +1000 Subject: [PATCH 23/36] Query: build the ID tie-breaker before the query clause filters run. Posts that share a value for the column being sorted on have no fixed order, so paginated queries can show the same post on two pages and skip another entirely. Appending the ID to ORDER BY breaks those ties. The tie-breaker is now built into the ORDER BY alongside every other clause, before `posts_orderby`, `posts_clauses` and their `_request` counterparts run, matching how WP_Comment_Query has added its own comment_ID tie-breaker since 4.4. Previously it was appended after those filters, and only when the filtered value still matched the string built earlier. That comparison was exact, so a filter returning the same clause with a trailing space silently switched the tie-breaker off. Building it up front removes the comparison, and filters now receive the ORDER BY that actually runs. Ordering that already ends in a unique value gets no tie-breaker: ID, post__in, post_name__in, post_parent__in, rand, and seeded RAND(n). The last of these previously had the ID appended, which defeated the point of passing a seed. Also stop dropping the ORDER BY when a `posts_clauses_request` filter returns no 'orderby' of its own. It now keeps the clause built earlier, including anything `posts_orderby_request` did to it, rather than leaving the query unordered and undoing the tie-breaker. `get_pages()` passes its 'sort_column' as array( 'none' => $sort_order ), so the 'none' check accepts that form as well as the bare string. Props ramonopoly, peterwilsoncc, azaozz. See #44349. --- src/wp-includes/class-wp-query.php | 172 +++++++++--------- .../tests/query/deterministicOrdering.php | 23 ++- .../tests/rest-api/rest-posts-controller.php | 92 +--------- 3 files changed, 113 insertions(+), 174 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index b5becf0948364..c6d80d381ac81 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -1835,6 +1835,56 @@ protected function parse_order( $order ) { } } + /** + * Determines whether an 'orderby' value can never produce two equal rows. + * + * Ordering by ID, by an explicit list of IDs, or at random already gives every + * post a distinct position, so no tie-breaker is needed on top of it. + * + * @since 7.0.0 + * + * @param string $orderby Single 'orderby' value, before it is parsed into SQL. + * @return bool Whether the value orders posts uniquely. + */ + protected function is_unique_orderby( $orderby ) { + $unique_orderby = array( + 'ID', + 'rand', + 'post__in', + 'post_name__in', + 'post_parent__in', + ); + + if ( in_array( $orderby, $unique_orderby, true ) ) { + return true; + } + + // Seeded random, for example 'RAND(5)', is handled by parse_orderby(). + return 1 === preg_match( '/^RAND\(\s*[0-9]*\s*\)$/i', $orderby ); + } + + /** + * Works out which direction the ID tie-breaker should sort in. + * + * The tie-breaker follows the direction of the last clause it is appended to, so + * that reversing a query's 'order' also reverses the sequence of tied posts. + * + * @since 7.0.0 + * + * @param string[] $orderby_array ORDER BY clauses built so far, each ending in a direction. + * @param string $order The query's 'order' value, used when no clause carries one. + * @return string Either 'ASC' or 'DESC'. + */ + protected function parse_tiebreaker_order( $orderby_array, $order ) { + $last_clause = end( $orderby_array ); + + if ( is_string( $last_clause ) && preg_match( '/\s(ASC|DESC)\s*$/i', $last_clause, $matches ) ) { + return strtoupper( $matches[1] ); + } + + return $this->parse_order( $order ); + } + /** * Sets the 404 property and saves whether query is feed. * @@ -2505,17 +2555,7 @@ public function get_posts() { $query_vars['order'] = ''; } - /* - * Order by. - * Store metadata for deterministic ordering to be applied after filters. - */ - $deterministic_orderby_meta = array( - 'needed' => false, - 'has_id' => false, - 'order' => $query_vars['order'], - 'original' => '', // Store original orderby to detect filter modifications. - ); - + // Order by. if ( empty( $query_vars['orderby'] ) ) { /* * Boolean false or empty array blanks out ORDER BY, @@ -2525,42 +2565,29 @@ public function get_posts() { $orderby = ''; } else { /* - * Ensure deterministic ordering to prevent duplicate records across pages. - * When multiple posts have the same value for a field, add ID as secondary sort to guarantee consistent ordering. - * Note: this is to circumvent a bug that is currently being tracked in https://core.trac.wordpress.org/ticket/44349. - * - * Build base orderby without ID tie-breaker for filters, then add it after filters. + * Sorting by post_date alone is not stable: posts sharing a date can be + * returned in a different sequence each time the query runs, so a post + * can appear on two pages at once or be skipped entirely. Appending the + * ID breaks those ties and gives every page a fixed sequence. */ - $orderby = "{$wpdb->posts}.post_date " . $query_vars['order']; - $deterministic_orderby_meta['needed'] = true; + $orderby = "{$wpdb->posts}.post_date {$query_vars['order']}, {$wpdb->posts}.ID {$query_vars['order']}"; } - // See get_pages(): when sort_column is 'none', the get_pages() function should not generate any ORDER BY clause. - // Should it rather be handled in the get_pages() function? - // src/wp-includes/post.php L6496 } elseif ( 'none' === $query_vars['orderby'] || isset( $query_vars['orderby']['none'] ) ) { + /* + * 'none' blanks out ORDER BY. It arrives as a bare string from WP_Query, and + * as an array key from get_pages(), which turns its 'sort_column' into + * array( 'none' => $sort_order ). + */ $orderby = ''; } else { + $orderby_array = array(); + /* - * Ensure deterministic ordering to prevent duplicate records across pages. - * When multiple posts have the same value for a field, add ID as secondary sort to guarantee consistent ordering. - * Note: this is to circumvent a bug that is currently being tracked in https://core.trac.wordpress.org/ticket/44349. - * - * Use a blacklist approach: add ID as tie-breaker for all orderby fields except those that are - * already deterministic (ID itself, random ordering, or search relevance). - * - * Build base orderby without ID tie-breaker for filters, then add it after filters. + * Tracks whether the requested ordering already ends in a unique value. + * Ordering by ID, by an explicit list of IDs, or at random cannot produce + * ties, so those need no tie-breaker appended. */ - $fields_excluding_deterministic_orderby = array( - 'ID', - 'rand', - 'relevance', - 'post__in', - 'post_name__in', - 'post_parent__in', - 'include', - ); - - $orderby_array = array(); + $found_unique_orderby = false; if ( is_array( $query_vars['orderby'] ) ) { foreach ( $query_vars['orderby'] as $_orderby => $order ) { @@ -2573,13 +2600,8 @@ public function get_posts() { $orderby_array[] = $parsed . ' ' . $this->parse_order( $order ); - // Check if this field should have deterministic ordering (not in blacklist). - if ( ! in_array( $_orderby, $fields_excluding_deterministic_orderby, true ) ) { - $deterministic_orderby_meta['needed'] = true; - // Use the order from the array for ID tie-breaker. - $deterministic_orderby_meta['order'] = $this->parse_order( $order ); - } elseif ( 'ID' === $_orderby ) { - $deterministic_orderby_meta['has_id'] = true; + if ( $this->is_unique_orderby( $orderby ) ) { + $found_unique_orderby = true; } } } else { @@ -2595,22 +2617,27 @@ public function get_posts() { $orderby_array[] = $parsed . ' ' . $query_vars['order']; - // Check if this field should have deterministic ordering (not in blacklist). - if ( ! in_array( $orderby, $fields_excluding_deterministic_orderby, true ) ) { - $deterministic_orderby_meta['needed'] = true; - } elseif ( 'ID' === $orderby ) { - $deterministic_orderby_meta['has_id'] = true; + if ( $this->is_unique_orderby( $orderby ) ) { + $found_unique_orderby = true; } } } - // Build the base orderby string (without ID tie-breaker) for filters. if ( empty( $orderby_array ) ) { - $orderby = "{$wpdb->posts}.post_date " . $query_vars['order']; - $deterministic_orderby_meta['needed'] = true; - } else { - $orderby = trim( implode( ', ', $orderby_array ) ); + $orderby_array[] = "{$wpdb->posts}.post_date " . $query_vars['order']; + } + + /* + * Posts sharing the same value for the requested column can be returned in + * a different sequence each time the query runs, so a post can appear on two + * pages at once or be skipped entirely. Appending the ID breaks those ties. + * Skipped when the ordering already ends in a unique value. + */ + if ( ! $found_unique_orderby ) { + $orderby_array[] = "{$wpdb->posts}.ID " . $this->parse_tiebreaker_order( $orderby_array, $query_vars['order'] ); } + + $orderby = trim( implode( ', ', $orderby_array ) ); } // Order search results by relevance only when another "orderby" is not specified in the query. @@ -2637,11 +2664,6 @@ public function get_posts() { } } - // Store the original orderby after all core modifications but before filters modify it. - if ( $deterministic_orderby_meta['needed'] ) { - $deterministic_orderby_meta['original'] = $orderby; - } - if ( is_array( $post_type ) && count( $post_type ) > 1 ) { $post_type_cap = 'multiple_post_type'; } else { @@ -3231,33 +3253,17 @@ public function get_posts() { $where = $clauses['where'] ?? ''; $groupby = $clauses['groupby'] ?? ''; $join = $clauses['join'] ?? ''; - // Preserve orderby from posts_orderby_request if posts_clauses_request doesn't provide one. + /* + * Keep the ORDER BY built above, and any change the 'posts_orderby_request' + * filter made to it, when this filter returns no 'orderby' of its own. + * Dropping it here would leave the query with no ORDER BY at all. + */ $orderby = $clauses['orderby'] ?? $orderby; $distinct = $clauses['distinct'] ?? ''; $fields = $clauses['fields'] ?? ''; $limits = $clauses['limits'] ?? ''; } - /* - * Ensure deterministic ordering to prevent duplicate records across pages. - * Add ID tie-breaker after filters have been applied, so filters receive - * the original orderby value (for backward compatibility) and the tie-breaker - * is preserved even if filters modify the orderby. - * - * Note: this is to circumvent a bug that is currently being tracked in - * https://core.trac.wordpress.org/ticket/44349. - */ - if ( ! empty( $orderby ) && $deterministic_orderby_meta['needed'] ) { - /* - * Only add ID tie-breaker if no filter modified the orderby. - * If a filter modified it, we assume they know what they're doing and don't interfere. - */ - if ( ! empty( $deterministic_orderby_meta['original'] ) && $orderby === $deterministic_orderby_meta['original'] ) { - // Add ID as tie-breaker at the end. - $orderby .= ', ' . "{$wpdb->posts}.ID " . $deterministic_orderby_meta['order']; - } - } - if ( ! empty( $groupby ) ) { $groupby = 'GROUP BY ' . $groupby; } diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index eeebe134ed26e..6f7ef2275addc 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -666,9 +666,9 @@ public function test_posts_orderby_filter_receives_original_orderby() { remove_filter( 'posts_orderby', $filter_callback ); - // Filter should receive orderby without ID tie-breaker. - $expected_orderby = "{$wpdb->posts}.post_date ASC"; - $this->assertEquals( $expected_orderby, $received_orderby, 'posts_orderby filter should receive original orderby without ID tie-breaker' ); + // The filter receives the complete ORDER BY, tie-breaker included. + $expected_orderby = "{$wpdb->posts}.post_date ASC, {$wpdb->posts}.ID ASC"; + $this->assertSame( $expected_orderby, $received_orderby, 'posts_orderby filter should receive the ORDER BY that will run, including the ID tie-breaker' ); } /** @@ -733,9 +733,9 @@ public function test_posts_clauses_filter_receives_original_orderby() { remove_filter( 'posts_clauses', $filter_callback ); - // Filter should receive orderby without ID tie-breaker. - $expected_orderby = "{$wpdb->posts}.post_date ASC"; - $this->assertEquals( $expected_orderby, $received_orderby, 'posts_clauses filter should receive original orderby without ID tie-breaker' ); + // The filter receives the complete ORDER BY, tie-breaker included. + $expected_orderby = "{$wpdb->posts}.post_date ASC, {$wpdb->posts}.ID ASC"; + $this->assertSame( $expected_orderby, $received_orderby, 'posts_clauses filter should receive the ORDER BY that will run, including the ID tie-breaker' ); } /** @@ -800,8 +800,15 @@ public function test_filter_modifications_to_orderby_are_preserved() { // Verify filter modification is preserved in the final query. $this->assertStringContainsString( 'post_title ASC', $query->request, 'Filter modification to orderby should be preserved' ); - // Verify ID tie-breaker is NOT added when filter modifies orderby. - $this->assertStringNotContainsString( ', ' . $GLOBALS['wpdb']->posts . '.ID ASC', $query->request, 'ID tie-breaker should not be added when filter modifies orderby' ); + /* + * The filter appended to a clause that already carried the tie-breaker, so the + * query runs exactly what the filter returned. Nothing is added afterwards. + */ + $this->assertStringContainsString( + "{$GLOBALS['wpdb']->posts}.post_date ASC, {$GLOBALS['wpdb']->posts}.ID ASC, {$GLOBALS['wpdb']->posts}.post_title ASC", + $query->request, + 'The query should run exactly the ORDER BY the filter returned' + ); } /** diff --git a/tests/phpunit/tests/rest-api/rest-posts-controller.php b/tests/phpunit/tests/rest-api/rest-posts-controller.php index 6d21498848ff7..38a43fef488c7 100644 --- a/tests/phpunit/tests/rest-api/rest-posts-controller.php +++ b/tests/phpunit/tests/rest-api/rest-posts-controller.php @@ -488,7 +488,7 @@ public function test_get_items_include_query( $method ) { $this->assertSame( 2, $headers['X-WP-Total'], 'Failed asserting that the number of posts is correct.' ); } - $this->assertPostsOrderedBy( '{posts}.post_date DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_date DESC, {posts}.ID DESC' ); // 'orderby' => 'include'. $request->set_param( 'orderby', 'include' ); @@ -544,7 +544,7 @@ public function test_get_items_orderby_author_query() { $this->assertSame( self::$editor_id, $data[1]['author'] ); $this->assertSame( self::$editor_id, $data[2]['author'] ); - $this->assertPostsOrderedBy( '{posts}.post_author DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_author DESC, {posts}.ID DESC' ); } public function test_get_items_orderby_modified_query() { @@ -568,7 +568,7 @@ public function test_get_items_orderby_modified_query() { $this->assertSame( $id3, $data[1]['id'] ); $this->assertSame( $id2, $data[2]['id'] ); - $this->assertPostsOrderedBy( '{posts}.post_modified DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_modified DESC, {posts}.ID DESC' ); } public function test_get_items_orderby_parent_query() { @@ -606,7 +606,7 @@ public function test_get_items_orderby_parent_query() { $this->assertSame( 0, $data[1]['parent'] ); $this->assertSame( 0, $data[2]['parent'] ); - $this->assertPostsOrderedBy( '{posts}.post_parent DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_parent DESC, {posts}.ID DESC' ); } public function test_get_items_exclude_query() { @@ -976,14 +976,14 @@ public function test_get_items_order_and_orderby() { $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); $this->assertSame( 'Apple Sauce', $data[0]['title']['rendered'] ); - $this->assertPostsOrderedBy( '{posts}.post_title DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_title DESC, {posts}.ID DESC' ); // 'order' => 'asc'. $request->set_param( 'order', 'asc' ); $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); $this->assertSame( 'Apple Cobbler', $data[0]['title']['rendered'] ); - $this->assertPostsOrderedBy( '{posts}.post_title ASC' ); + $this->assertPostsOrderedBy( '{posts}.post_title ASC, {posts}.ID ASC' ); // 'order' => 'asc,id' should error. $request->set_param( 'order', 'asc,id' ); @@ -1068,7 +1068,7 @@ public function test_get_items_with_orderby_slug() { // Default ORDER is DESC. $this->assertSame( 'xyz', $data[0]['slug'] ); $this->assertSame( 'abc', $data[1]['slug'] ); - $this->assertPostsOrderedBy( '{posts}.post_name DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_name DESC, {posts}.ID DESC' ); } public function test_get_items_with_orderby_slugs() { @@ -1120,7 +1120,7 @@ public function test_get_items_with_orderby_relevance() { $this->assertCount( 2, $data ); $this->assertSame( $id1, $data[0]['id'] ); $this->assertSame( $id2, $data[1]['id'] ); - $this->assertPostsOrderedBy( '{posts}.post_title LIKE \'%relevant%\' DESC, {posts}.post_date DESC' ); + $this->assertPostsOrderedBy( '{posts}.post_title LIKE \'%relevant%\' DESC, {posts}.post_date DESC, {posts}.ID DESC' ); } public function test_get_items_with_orderby_relevance_two_terms() { @@ -1148,7 +1148,7 @@ public function test_get_items_with_orderby_relevance_two_terms() { $this->assertCount( 2, $data ); $this->assertSame( $id1, $data[0]['id'] ); $this->assertSame( $id2, $data[1]['id'] ); - $this->assertPostsOrderedBy( '(CASE WHEN {posts}.post_title LIKE \'%relevant content%\' THEN 1 WHEN {posts}.post_title LIKE \'%relevant%\' AND {posts}.post_title LIKE \'%content%\' THEN 2 WHEN {posts}.post_title LIKE \'%relevant%\' OR {posts}.post_title LIKE \'%content%\' THEN 3 WHEN {posts}.post_excerpt LIKE \'%relevant content%\' THEN 4 WHEN {posts}.post_content LIKE \'%relevant content%\' THEN 5 ELSE 6 END), {posts}.post_date DESC' ); + $this->assertPostsOrderedBy( '(CASE WHEN {posts}.post_title LIKE \'%relevant content%\' THEN 1 WHEN {posts}.post_title LIKE \'%relevant%\' AND {posts}.post_title LIKE \'%content%\' THEN 2 WHEN {posts}.post_title LIKE \'%relevant%\' OR {posts}.post_title LIKE \'%content%\' THEN 3 WHEN {posts}.post_excerpt LIKE \'%relevant content%\' THEN 4 WHEN {posts}.post_content LIKE \'%relevant content%\' THEN 5 ELSE 6 END), {posts}.post_date DESC, {posts}.ID DESC' ); } public function test_get_items_with_orderby_relevance_missing_search() { @@ -1158,80 +1158,6 @@ public function test_get_items_with_orderby_relevance_missing_search() { $this->assertErrorResponse( 'rest_no_search_term_defined', $response, 400 ); } - /** - * Test that ID tie-breaker is added to final SQL query for deterministic ordering. - * - * This test verifies that the ID tie-breaker is present in the final SQL query, - * even though filters receive the orderby without ID (for backward compatibility). - * - * @ticket xxxxx - */ - public function test_id_tie_breaker_in_final_sql_query() { - global $wpdb; - - $identical_date = '2023-01-01 10:00:00'; - $post_ids = array(); - for ( $i = 1; $i <= 5; $i++ ) { - $post_ids[] = self::factory()->post->create( - array( - 'post_status' => 'publish', - 'post_date' => $identical_date, - ) - ); - } - - /* - * Capture the WP_Query instance via posts_clauses filter. - * We use the same hook as other tests in this class (save_posts_clauses), - * but we need to capture the query instance to access $query->request after execution. - * The existing save_posts_clauses method stores clauses but not the query instance. - */ - $captured_query = null; - $filter_callback = function ( $clauses, $query ) use ( &$captured_query ) { - /* - * Short-circuit: only capture the query on the first call. - * The posts_clauses filter may be called multiple times (e.g., for main query - * and sub-queries), but we only need the main query instance once. - */ - if ( null === $captured_query ) { - $captured_query = $query; - } - return $clauses; - }; - add_filter( 'posts_clauses', $filter_callback, 10, 2 ); - - $request = new WP_REST_Request( 'GET', '/wp/v2/posts' ); - $request->set_param( 'order', 'desc' ); - $request->set_param( 'per_page', 100 ); - - $response = rest_get_server()->dispatch( $request ); - - remove_filter( 'posts_clauses', $filter_callback ); - - $this->assertSame( 200, $response->get_status() ); - $this->assertNotNull( $captured_query, 'WP_Query should be captured' ); - $this->assertInstanceOf( 'WP_Query', $captured_query, 'Captured query should be a WP_Query instance' ); - - /** @var WP_Query $captured_query */ - $sql = $captured_query->request; - $posts_table = preg_quote( $wpdb->posts, '/' ); - - $orderby_pattern = '/ORDER\s+BY\s+.*' . $posts_table . '\.ID\s+(?:ASC|DESC)/i'; - $this->assertMatchesRegularExpression( - $orderby_pattern, - $sql, - 'Final SQL query should include ID tie-breaker in ORDER BY clause' - ); - - $this->assertCount( 1, $this->posts_clauses ); - $filter_orderby = $this->posts_clauses[0]['orderby']; - $this->assertStringNotContainsString( - 'ID', - $filter_orderby, - 'Filters should receive orderby without ID tie-breaker for backward compatibility' - ); - } - public function test_get_items_offset_query() { $request = new WP_REST_Request( 'GET', '/wp/v2/posts' ); $request->set_param( 'per_page', self::$per_page ); From 1a468a0ae524643fcf763306d3058f89d1b06874 Mon Sep 17 00:00:00 2001 From: Ramon Date: Tue, 1 Sep 2026 17:06:04 +1000 Subject: [PATCH 24/36] Query: restore the cache key's default orderby value. generate_cache_key() rewrote the 'orderby' argument from 'date' to 'date, ID' before hashing. The hash also covers the SQL, which already carries the tie-breaker, so the rewrite changed the key for every default query without distinguishing anything the SQL had not already distinguished. Upgrading would have missed every cached post query for no gain. Restores the comment explaining why the default is set at all. See #44349. --- src/wp-includes/class-wp-query.php | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index c6d80d381ac81..45d0b56211ef9 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -5182,14 +5182,9 @@ protected function generate_cache_key( array $args, $sql ) { sort( $args['post_status'] ); } - /* - * Ensure deterministic ordering to prevent duplicate records across pages. - * When multiple posts have the same value for a field, add ID as secondary sort to guarantee consistent ordering. - */ + // Add a default orderby value of date to ensure same cache key generation. if ( ! isset( $args['orderby'] ) ) { - $args['orderby'] = 'date, ID'; - } elseif ( 'date' === $args['orderby'] ) { - $args['orderby'] = 'date, ID'; + $args['orderby'] = 'date'; } $placeholder = $wpdb->placeholder_escape(); From 25f35b58ff590ab2451e763bc739cced345e7bb6 Mon Sep 17 00:00:00 2001 From: Ramon Date: Tue, 1 Sep 2026 17:06:34 +1000 Subject: [PATCH 25/36] Query: restore the cache key's SELECT field replacement. The cache key is built from the query with its SELECT fields normalised to `wp_posts.*`. This split that replacement around the ORDER BY, to stop it also rewriting a field name appearing there. The guard only held for a query containing exactly one ORDER BY; a filter adding a subquery gave three parts and fell through to the same replacement it was meant to avoid. The value is only ever hashed, never run, and $fields is restricted to three known strings just above, so nothing was corrected here. Restores the single replacement, unchanged from before. See #44349. --- src/wp-includes/class-wp-query.php | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 45d0b56211ef9..b19d3ec96c9b4 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -3357,21 +3357,10 @@ public function get_posts() { } if ( $query_vars['cache_results'] && $id_query_is_cacheable ) { - $new_request = $this->request; - // Split SQL into parts. - $parts = explode( 'ORDER BY', $new_request ); - if ( count( $parts ) === 2 ) { - // Replace only in the SELECT part, preserve ORDER BY. - $select_part = str_replace( $fields, "{$wpdb->posts}.*", $parts[0] ); - $new_request = $select_part . 'ORDER BY' . $parts[1]; - } else { - // No ORDER BY clause, safe to replace. - $new_request = str_replace( $fields, "{$wpdb->posts}.*", $new_request ); - } - + $new_request = str_replace( $fields, "{$wpdb->posts}.*", $this->request ); $cache_key = $this->generate_cache_key( $query_vars, $new_request ); - $cache_found = false; + $cache_found = false; if ( null === $this->posts ) { $cached_results = wp_cache_get_salted( $cache_key, 'post-queries', $last_changed ); From 2cd22a7c0cc21b247fd751901d7d90b833de8ecf Mon Sep 17 00:00:00 2001 From: Ramon Date: Tue, 1 Sep 2026 17:23:09 +1000 Subject: [PATCH 26/36] Tests: cover paginated queries returning a post more than once. Rewrites the ordering tests around cases that actually fail without the fix. Two of them page through posts and check that each one is returned exactly once: - Ordering by date, querying two post statuses at once. A single status lets the database read rows straight from the type_status_date index, which ends in ID and so hides the problem; two statuses make it sort the rows itself. This is the case behind most of the reports. - Ordering by menu_order, which no index covers. The rest pin down what the tie-breaker does and does not touch: the direction it takes, ordering that is already unique, seeded and unseeded random, post__in, 'none' from both WP_Query and get_pages(), and search relevance. The filter tests now assert that each clause filter is handed the ORDER BY that will run, that a filtered clause is used exactly as returned, and that a filter changing nothing keeps the tie-breaker, including one that adds only a trailing space. 17 of the 33 fail without the accompanying fix. Props ramonopoly, peterwilsoncc, azaozz. See #44349, #46294. --- .../tests/query/deterministicOrdering.php | 1066 +++++++---------- 1 file changed, 404 insertions(+), 662 deletions(-) diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index 6f7ef2275addc..e20535b51d5f4 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -1,888 +1,630 @@ true, - ) - ); - - register_post_type( - 'wptests_title_ident', - array( - 'public' => true, - ) - ); - - // Create posts with identical dates for date ordering tests. - $identical_date = '2023-01-01 10:00:00'; + public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { + /* + * Every post here shares one date. Splitting them across two statuses stops + * the database using the type_status_date index, which happens to end in ID + * and would otherwise hide the missing tie-breaker. + */ for ( $i = 1; $i <= 20; $i++ ) { - self::$date_identical_post_ids[] = self::factory()->post->create( - array( - 'post_type' => 'wptests_time_ident', - 'post_title' => "Post $i", - 'post_date' => $identical_date, - ) - ); - } - - // Create posts with identical titles for title ordering tests. - $identical_title = 'Same Title'; - for ( $i = 1; $i <= 15; $i++ ) { - self::$title_identical_post_ids[] = self::factory()->post->create( + self::$mixed_status_ids[] = $factory->post->create( array( - 'post_type' => 'wptests_title_ident', - 'post_title' => $identical_title, - 'post_date' => '2023-01-' . str_pad( (string) $i, 2, '0', STR_PAD_LEFT ) . ' 10:00:00', + 'post_title' => "Mixed status $i", + 'post_date' => '2023-01-01 10:00:00', + 'post_status' => ( 0 === $i % 2 ) ? 'private' : 'publish', ) ); } - // Create posts for search tests. - $identical_date = '2023-01-01 10:00:00'; - for ( $i = 1; $i <= 12; $i++ ) { - self::$search_post_ids[] = self::factory()->post->create( - array( - 'post_type' => 'wptests_time_ident', - 'post_title' => "Test Post $i", - 'post_content' => 'This is a test post', - 'post_date' => $identical_date, - ) - ); - } - - // Create pages with identical menu_order for menu_order tests. + // menu_order has no index at all, so ties in it are never ordered. for ( $i = 1; $i <= 20; $i++ ) { - self::$menu_order_post_ids[] = self::factory()->post->create( + self::$menu_order_ids[] = $factory->post->create( array( 'post_type' => 'page', 'post_title' => "Page $i", - 'menu_order' => 0, // All pages have same menu_order + 'menu_order' => 0, ) ); } - // Create posts for search relevance tests. - // All posts will have the same content to ensure same relevance scores. - $identical_content = 'This is a search test post with identical content'; for ( $i = 1; $i <= 20; $i++ ) { - self::$search_relevance_post_ids[] = self::factory()->post->create( + self::$same_title_ids[] = $factory->post->create( array( - 'post_type' => 'wptests_time_ident', - 'post_title' => "Search Post $i", - 'post_content' => $identical_content, - 'post_excerpt' => $identical_content, + 'post_title' => 'Same title', + 'post_date' => '2023-02-' . str_pad( (string) $i, 2, '0', STR_PAD_LEFT ) . ' 10:00:00', ) ); } } /** - * Clean up after all tests. + * Returns the post IDs on one page of a query. + * + * @param array $args Query arguments. 'posts_per_page' and 'paged' are set by the caller. + * @return int[] Post IDs, in the order the query returned them. */ - public static function tear_down_after_class() { - _unregister_post_type( 'wptests_time_ident' ); - _unregister_post_type( 'wptests_title_ident' ); + private function get_page_of_ids( $args ) { + $query = new WP_Query( $args ); + return wp_list_pluck( $query->posts, 'ID' ); + } - self::$date_identical_post_ids = array(); - self::$title_identical_post_ids = array(); - self::$search_post_ids = array(); - self::$menu_order_post_ids = array(); - self::$search_relevance_post_ids = array(); + /** + * Asserts that paging through a query returns every post exactly once. + * + * @param array $args Query arguments, without 'posts_per_page' or 'paged'. + * @param int $per_page Posts per page. + * @param int $pages Number of pages to walk. + * @param int $expected Total number of posts expected across those pages. + * @param string $message Message describing the ordering under test. + */ + private function assertPagesDoNotRepeatPosts( $args, $per_page, $pages, $expected, $message ) { + $seen = array(); + + for ( $page = 1; $page <= $pages; $page++ ) { + $seen = array_merge( + $seen, + $this->get_page_of_ids( + array_merge( + $args, + array( + 'posts_per_page' => $per_page, + 'paged' => $page, + ) + ) + ) + ); + } - parent::tear_down_after_class(); + $this->assertSameSets( array_unique( $seen ), $seen, $message . ': a post appeared on more than one page' ); + $this->assertCount( $expected, $seen, $message . ': the pages did not add up to every post' ); } /** - * Test that deterministic ordering prevents duplicate records across pages. + * Ordering by date is stable when posts share a date. * - * This is the core test for the bug fix. When multiple posts have the same - * value for a field (like post_date), pagination can show duplicate records - * without deterministic ordering. + * Two statuses are queried together so the database sorts the rows itself + * rather than reading them from an index that already ends in ID. * - * @ticket xxxxx + * @ticket 44349 */ - public function test_deterministic_ordering_prevents_duplicates_across_pages() { - // Use shared fixtures with identical post_date - - // Get first page - $query1 = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$date_identical_post_ids, - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 10, - 'paged' => 1, - ) - ); - - // Get second page - $query2 = new WP_Query( + public function test_paging_by_date_returns_each_post_once() { + $this->assertPagesDoNotRepeatPosts( array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$date_identical_post_ids, - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 10, - 'paged' => 2, - ) + 'post_type' => 'post', + 'post_status' => array( 'publish', 'private' ), + 'post__in' => self::$mixed_status_ids, + 'orderby' => 'date', + 'order' => 'DESC', + ), + 10, + 2, + 20, + 'Ordering by date' ); - - $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); - $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); - - // Verify no overlap between pages (no duplicates) - $overlap = array_intersect( $page1_ids, $page2_ids ); - $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts' ); - - // Verify total count is correct - $this->assertEquals( 20, $query1->found_posts, 'Total posts should be 20' ); - $this->assertEquals( 10, count( $page1_ids ), 'First page should have 10 posts' ); - $this->assertEquals( 10, count( $page2_ids ), 'Second page should have 10 posts' ); - - // Verify deterministic ordering: same query should return same results - $query1_repeat = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$date_identical_post_ids, - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 10, - 'paged' => 1, - ) - ); - $page1_repeat_ids = wp_list_pluck( $query1_repeat->posts, 'ID' ); - - $this->assertEquals( $page1_ids, $page1_repeat_ids, 'Same query should return same results' ); } /** - * Test that deterministic ordering works with post_title field. + * Ordering by menu_order is stable when posts share a menu_order. * - * @ticket xxxxx + * @ticket 44349 + * @ticket 46294 */ - public function test_deterministic_ordering_with_post_title() { - // Use shared fixtures with identical post_title - // Get first page - $query1 = new WP_Query( + public function test_paging_by_menu_order_returns_each_post_once() { + $this->assertPagesDoNotRepeatPosts( array( - 'post_type' => 'wptests_title_ident', - 'post__in' => self::$title_identical_post_ids, - 'orderby' => 'post_title', - 'order' => 'ASC', - 'posts_per_page' => 8, - 'paged' => 1, - ) + 'post_type' => 'page', + 'post__in' => self::$menu_order_ids, + 'orderby' => 'menu_order', + 'order' => 'ASC', + ), + 10, + 2, + 20, + 'Ordering by menu_order' ); - - // Get second page - $query2 = new WP_Query( - array( - 'post_type' => 'wptests_title_ident', - 'post__in' => self::$title_identical_post_ids, - 'orderby' => 'post_title', - 'order' => 'ASC', - 'posts_per_page' => 8, - 'paged' => 2, - ) - ); - - $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); - $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); - - // Verify no duplicates across pages - $overlap = array_intersect( $page1_ids, $page2_ids ); - $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts when ordering by title' ); } /** - * Test that deterministic ordering works with DESC order. + * Ordering by title is stable when posts share a title. * - * @ticket xxxxx + * @ticket 44349 */ - public function test_deterministic_ordering_with_desc_order() { - // Use shared fixtures with identical post_date - // Get first page with DESC order - $query1 = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$date_identical_post_ids, - 'orderby' => 'post_date', - 'order' => 'DESC', - 'posts_per_page' => 6, - 'paged' => 1, - ) - ); - - // Get second page with DESC order - $query2 = new WP_Query( + public function test_paging_by_title_returns_each_post_once() { + $this->assertPagesDoNotRepeatPosts( array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$date_identical_post_ids, - 'orderby' => 'post_date', - 'order' => 'DESC', - 'posts_per_page' => 6, - 'paged' => 2, - ) + 'post_type' => 'post', + 'post__in' => self::$same_title_ids, + 'orderby' => 'title', + 'order' => 'ASC', + ), + 10, + 2, + 20, + 'Ordering by title' ); - - $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); - $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); - - // Verify no duplicates across pages - $overlap = array_intersect( $page1_ids, $page2_ids ); - $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts with DESC order' ); } /** - * Test that deterministic ordering works with array orderby. + * The same query run twice returns the same page in the same order. * - * @ticket xxxxx + * @ticket 44349 */ - public function test_deterministic_ordering_with_array_orderby() { - // Use shared fixtures with identical post_date - // Test with array orderby - $query1 = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$date_identical_post_ids, - 'orderby' => array( - 'post_date' => 'ASC', - 'post_title' => 'ASC', - ), - 'posts_per_page' => 8, - 'paged' => 1, - ) + public function test_repeating_a_query_returns_the_same_page() { + $args = array( + 'post_type' => 'page', + 'post__in' => self::$menu_order_ids, + 'orderby' => 'menu_order', + 'order' => 'ASC', + 'posts_per_page' => 10, + 'paged' => 1, ); - $query2 = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$date_identical_post_ids, - 'orderby' => array( - 'post_date' => 'ASC', - 'post_title' => 'ASC', - ), - 'posts_per_page' => 8, - 'paged' => 2, - ) + $this->assertSame( + $this->get_page_of_ids( $args ), + $this->get_page_of_ids( $args ), + 'Running the same query twice returned a different page' ); - - $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); - $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); - - // Verify no duplicates across pages - $overlap = array_intersect( $page1_ids, $page2_ids ); - $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts with array orderby' ); } /** - * Test that deterministic ordering doesn't add ID when ID is already present. + * The ID tie-breaker follows the direction of the clause it is added to. * - * @ticket xxxxx + * @ticket 44349 + * + * @dataProvider data_tiebreaker_directions + * + * @param array $args Query arguments. + * @param string $expected Expected ORDER BY clause, with {posts} standing in for the table name. */ - public function test_deterministic_ordering_does_not_duplicate_id() { - // Use shared fixtures with identical post_date - $query = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$date_identical_post_ids, - 'orderby' => 'ID', - 'order' => 'ASC', - 'posts_per_page' => 10, - ) - ); + public function test_tiebreaker_follows_the_sort_direction( $args, $expected ) { + global $wpdb; - // Should not add duplicate ID ordering - $this->assertStringContainsString( 'ID ASC', $query->request ); - $this->assertStringNotContainsString( 'ID ASC, ID ASC', $query->request ); + $query = new WP_Query( array_merge( $args, array( 'posts_per_page' => 5 ) ) ); + + $this->assertStringContainsString( + 'ORDER BY ' . str_replace( '{posts}', $wpdb->posts, $expected ), + $query->request + ); } /** - * Test that deterministic ordering works with search queries. + * Data provider. * - * @ticket xxxxx + * @return array[] */ - public function test_deterministic_ordering_with_search() { - // Use shared fixtures for search tests - // Test with search - $query1 = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$search_post_ids, - 's' => 'test', - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 6, - 'paged' => 1, - ) - ); - - $query2 = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$search_post_ids, - 's' => 'test', - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 6, - 'paged' => 2, - ) + public function data_tiebreaker_directions() { + return array( + 'descending date' => array( + array( + 'orderby' => 'date', + 'order' => 'DESC', + ), + '{posts}.post_date DESC, {posts}.ID DESC', + ), + 'ascending date' => array( + array( + 'orderby' => 'date', + 'order' => 'ASC', + ), + '{posts}.post_date ASC, {posts}.ID ASC', + ), + 'no orderby given' => array( + array(), + '{posts}.post_date DESC, {posts}.ID DESC', + ), + 'ascending menu_order' => array( + array( + 'orderby' => 'menu_order', + 'order' => 'ASC', + ), + '{posts}.menu_order ASC, {posts}.ID ASC', + ), + 'two columns' => array( + array( + 'orderby' => array( + 'title' => 'DESC', + 'date' => 'ASC', + ), + ), + '{posts}.post_title DESC, {posts}.post_date ASC, {posts}.ID ASC', + ), + 'unparseable column' => array( + array( 'orderby' => 'a_column_that_does_not_exist' ), + '{posts}.post_date DESC, {posts}.ID DESC', + ), ); - - $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); - $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); - - // Verify no duplicates across pages even with search - $overlap = array_intersect( $page1_ids, $page2_ids ); - $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts even with search' ); } /** - * Test that deterministic ordering works with menu_order field. + * Ordering that is already unique gets no tie-breaker added to it. + * + * @ticket 44349 * - * @ticket xxxxx + * @dataProvider data_orderby_that_is_already_unique + * + * @param array $args Query arguments. */ - public function test_deterministic_ordering_with_menu_order() { - // Use shared fixtures with identical menu_order - // Get first page - $query1 = new WP_Query( - array( - 'post_type' => 'page', - 'post__in' => self::$menu_order_post_ids, - 'orderby' => 'menu_order', - 'order' => 'ASC', - 'posts_per_page' => 10, - 'paged' => 1, - ) - ); - - // Get second page - $query2 = new WP_Query( - array( - 'post_type' => 'page', - 'post__in' => self::$menu_order_post_ids, - 'orderby' => 'menu_order', - 'order' => 'ASC', - 'posts_per_page' => 10, - 'paged' => 2, - ) - ); - - $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); - $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); + public function test_unique_orderby_gets_no_tiebreaker( $args ) { + global $wpdb; - // Verify no overlap between pages (no duplicates) - $overlap = array_intersect( $page1_ids, $page2_ids ); - $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts when ordering by menu_order' ); + $query = new WP_Query( array_merge( $args, array( 'posts_per_page' => 5 ) ) ); - // Verify total count is correct - $this->assertEquals( 20, $query1->found_posts, 'Total pages should be 20' ); - $this->assertEquals( 10, count( $page1_ids ), 'First page should have 10 pages' ); - $this->assertEquals( 10, count( $page2_ids ), 'Second page should have 10 pages' ); + preg_match( '/ORDER BY(.*?)LIMIT/s', $query->request, $matches ); + $orderby = isset( $matches[1] ) ? $matches[1] : ''; - // Verify deterministic ordering: same query should return same results - $query1_repeat = new WP_Query( - array( - 'post_type' => 'page', - 'post__in' => self::$menu_order_post_ids, - 'orderby' => 'menu_order', - 'order' => 'ASC', - 'posts_per_page' => 10, - 'paged' => 1, - ) + $this->assertSame( + 1, + substr_count( $orderby, "{$wpdb->posts}.ID" ), + 'The ID appeared more than once in the ORDER BY' ); - $page1_repeat_ids = wp_list_pluck( $query1_repeat->posts, 'ID' ); - - $this->assertEquals( $page1_ids, $page1_repeat_ids, 'Same query should return same results when ordering by menu_order' ); } /** - * Test that deterministic ordering works with metadata ordering. + * Data provider. * - * @ticket xxxxx + * @return array[] */ - public function test_deterministic_ordering_with_metadata() { - $post_ids = array(); - - // Create posts with identical meta values to trigger the bug - $identical_meta_value = 'same_price'; - for ( $i = 1; $i <= 20; $i++ ) { - $post_id = self::factory()->post->create( + public function data_orderby_that_is_already_unique() { + return array( + 'ID' => array( array( - 'post_type' => 'wptests_time_ident', - 'post_title' => "Post $i", - ) - ); - add_post_meta( $post_id, 'price', $identical_meta_value ); - $post_ids[] = $post_id; - } - - // Get first page ordering by metadata - $query1 = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => $post_ids, - 'meta_query' => array( - 'price_key' => array( - 'key' => 'price', - 'compare' => 'EXISTS', - ), + 'orderby' => 'ID', + 'order' => 'ASC', ), - 'orderby' => 'price_key', - 'order' => 'ASC', - 'posts_per_page' => 10, - 'paged' => 1, - ) - ); - - // Get second page ordering by metadata - $query2 = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => $post_ids, - 'meta_query' => array( - 'price_key' => array( - 'key' => 'price', - 'compare' => 'EXISTS', + ), + 'ID, descending' => array( + array( + 'orderby' => 'ID', + 'order' => 'DESC', + ), + ), + 'ID given as an array' => array( array( 'orderby' => array( 'ID' => 'DESC' ) ) ), + 'ID named after another' => array( + array( + 'orderby' => 'title ID', + 'order' => 'ASC', + ), + ), + 'ID named before another' => array( + array( + 'orderby' => array( + 'ID' => 'DESC', + 'title' => 'ASC', ), ), - 'orderby' => 'price_key', - 'order' => 'ASC', - 'posts_per_page' => 10, - 'paged' => 2, - ) + ), ); - - $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); - $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); - - // Verify no overlap between pages (no duplicates) - $overlap = array_intersect( $page1_ids, $page2_ids ); - $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts when ordering by metadata' ); - - // Verify total count is correct - $this->assertEquals( 20, $query1->found_posts, 'Total posts should be 20' ); - $this->assertEquals( 10, count( $page1_ids ), 'First page should have 10 posts' ); - $this->assertEquals( 10, count( $page2_ids ), 'Second page should have 10 posts' ); } /** - * Test that deterministic ordering works with search relevance ordering. + * Random ordering is left alone. + * + * A seed is passed to get the same shuffle back on every page, which sorting + * by ID afterwards would undo. * - * When ordering by search relevance, multiple posts can have the same relevance score, - * causing duplicate records across pages without deterministic ordering. + * @ticket 44349 * - * @ticket xxxxx + * @dataProvider data_random_orderby + * + * @param string $orderby The 'orderby' value. */ - public function test_deterministic_ordering_with_search_relevance() { - // Use shared fixtures with identical content (same relevance scores) - // Get first page ordering by relevance - $query1 = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$search_relevance_post_ids, - 's' => 'search test', - 'orderby' => 'relevance', - 'order' => 'DESC', - 'posts_per_page' => 10, - 'paged' => 1, - ) - ); + public function test_random_ordering_gets_no_tiebreaker( $orderby ) { + global $wpdb; - // Get second page ordering by relevance - $query2 = new WP_Query( + $query = new WP_Query( array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$search_relevance_post_ids, - 's' => 'search test', - 'orderby' => 'relevance', - 'order' => 'DESC', - 'posts_per_page' => 10, - 'paged' => 2, + 'orderby' => $orderby, + 'posts_per_page' => 5, ) ); - $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); - $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); - - // Verify no overlap between pages (no duplicates) - $overlap = array_intersect( $page1_ids, $page2_ids ); - $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts when ordering by search relevance' ); + preg_match( '/ORDER BY(.*?)LIMIT/s', $query->request, $matches ); - // Verify total count is correct - $this->assertEquals( 20, $query1->found_posts, 'Total posts should be 20' ); - $this->assertEquals( 10, count( $page1_ids ), 'First page should have 10 posts' ); - $this->assertEquals( 10, count( $page2_ids ), 'Second page should have 10 posts' ); + $this->assertStringNotContainsString( "{$wpdb->posts}.ID", $matches[1] ); + } - // Verify deterministic ordering: same query should return same results - $query1_repeat = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$search_relevance_post_ids, - 's' => 'search test', - 'orderby' => 'relevance', - 'order' => 'DESC', - 'posts_per_page' => 10, - 'paged' => 1, - ) + /** + * Data provider. + * + * @return array[] + */ + public function data_random_orderby() { + return array( + 'unseeded' => array( 'rand' ), + 'seeded' => array( 'RAND(5)' ), ); - $page1_repeat_ids = wp_list_pluck( $query1_repeat->posts, 'ID' ); - - $this->assertEquals( $page1_ids, $page1_repeat_ids, 'Same query should return same results when ordering by search relevance' ); } /** - * Test that deterministic ordering works with search when orderby is empty (defaults to relevance). - * - * When orderby is empty and search is present, WordPress orders by relevance. - * Multiple posts can have the same relevance score, causing duplicate records across pages. + * An explicit list of IDs keeps the order it was given in. * - * @ticket xxxxx + * @ticket 44349 */ - public function test_deterministic_ordering_with_search_empty_orderby() { - // Use shared fixtures with identical content (same relevance scores) - // Get first page with empty orderby (defaults to relevance) - $query1 = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$search_relevance_post_ids, - 's' => 'search test', - 'orderby' => '', // Empty orderby with search defaults to relevance - 'order' => 'DESC', - 'posts_per_page' => 10, - 'paged' => 1, - ) - ); + public function test_post__in_keeps_its_own_order() { + $ids = array_slice( self::$menu_order_ids, 0, 5 ); + shuffle( $ids ); - // Get second page with empty orderby - $query2 = new WP_Query( + $query = new WP_Query( array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$search_relevance_post_ids, - 's' => 'search test', - 'orderby' => '', // Empty orderby with search defaults to relevance - 'order' => 'DESC', - 'posts_per_page' => 10, - 'paged' => 2, + 'post_type' => 'page', + 'post__in' => $ids, + 'orderby' => 'post__in', + 'posts_per_page' => 5, ) ); - $page1_ids = wp_list_pluck( $query1->posts, 'ID' ); - $page2_ids = wp_list_pluck( $query2->posts, 'ID' ); - - // Verify no overlap between pages (no duplicates) - $overlap = array_intersect( $page1_ids, $page2_ids ); - $this->assertEmpty( $overlap, 'Pages should not contain duplicate posts when ordering by search relevance (empty orderby)' ); - - // Verify total count is correct - $this->assertEquals( 20, $query1->found_posts, 'Total posts should be 20' ); - $this->assertEquals( 10, count( $page1_ids ), 'First page should have 10 posts' ); - $this->assertEquals( 10, count( $page2_ids ), 'Second page should have 10 posts' ); + $this->assertSame( $ids, wp_list_pluck( $query->posts, 'ID' ) ); } /** - * Test that posts_orderby filter receives original orderby value. + * An 'orderby' of 'none' still produces no ORDER BY. + * + * @ticket 44349 * - * This ensures backward compatibility - filters should receive the same orderby - * value they received before the deterministic ordering changes. + * @dataProvider data_orderby_that_blanks_the_clause * - * @ticket xxxxx + * @param mixed $orderby The 'orderby' value. */ - public function test_posts_orderby_filter_receives_original_orderby() { - global $wpdb; - - $received_orderby = ''; - - $filter_callback = function ( $orderby ) use ( &$received_orderby ) { - $received_orderby = $orderby; - return $orderby; - }; - - add_filter( 'posts_orderby', $filter_callback ); - + public function test_orderby_can_still_be_blanked( $orderby ) { $query = new WP_Query( array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$date_identical_post_ids, - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 10, + 'orderby' => $orderby, + 'posts_per_page' => 5, ) ); - remove_filter( 'posts_orderby', $filter_callback ); + $this->assertStringNotContainsString( 'ORDER BY', $query->request ); + } - // The filter receives the complete ORDER BY, tie-breaker included. - $expected_orderby = "{$wpdb->posts}.post_date ASC, {$wpdb->posts}.ID ASC"; - $this->assertSame( $expected_orderby, $received_orderby, 'posts_orderby filter should receive the ORDER BY that will run, including the ID tie-breaker' ); + /** + * Data provider. + * + * @return array[] + */ + public function data_orderby_that_blanks_the_clause() { + return array( + 'the string none' => array( 'none' ), + 'none as an array key' => array( array( 'none' => 'DESC' ) ), + 'an empty array' => array( array() ), + 'false' => array( false ), + ); } /** - * Test that ID tie-breaker is added when posts_orderby filter does not modify orderby. + * get_pages() can still ask for no ordering. + * + * It passes 'sort_column' through as an array key rather than a bare string. * - * @ticket xxxxx + * @ticket 44349 */ - public function test_id_tie_breaker_added_when_posts_orderby_filter_does_not_modify() { + public function test_get_pages_can_still_ask_for_no_ordering() { global $wpdb; - $filter_callback = function ( $orderby ) { - return $orderby; // Return unchanged. - }; - - add_filter( 'posts_orderby', $filter_callback ); - - $query = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$date_identical_post_ids, - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 10, - ) - ); - - remove_filter( 'posts_orderby', $filter_callback ); + get_pages( array( 'sort_column' => 'none' ) ); - // Since filter did NOT modify orderby, ID tie-breaker SHOULD be added. - $this->assertStringContainsString( ', ' . $wpdb->posts . '.ID ASC', $query->request, 'ID tie-breaker should be added when posts_orderby filter does not modify orderby' ); + $this->assertStringNotContainsString( 'ORDER BY', $wpdb->last_query ); } /** - * Test that posts_clauses filter receives original orderby value. + * Clause filters are handed the ORDER BY that will actually run. + * + * @ticket 44349 * - * This ensures backward compatibility - filters should receive the same orderby - * value they received before the deterministic ordering changes. + * @dataProvider data_orderby_filters * - * @ticket xxxxx + * @param string $filter Name of the filter under test. */ - public function test_posts_clauses_filter_receives_original_orderby() { + public function test_filters_receive_the_final_orderby( $filter ) { global $wpdb; - $received_orderby = ''; + $received = null; + $is_array = str_contains( $filter, 'clauses' ); - $filter_callback = function ( $clauses ) use ( &$received_orderby ) { - $received_orderby = $clauses['orderby'] ?? ''; - return $clauses; + $callback = static function ( $value ) use ( &$received, $is_array ) { + if ( null === $received ) { + $received = $is_array ? $value['orderby'] : $value; + } + return $value; }; - add_filter( 'posts_clauses', $filter_callback ); - - $query = new WP_Query( + add_filter( $filter, $callback ); + new WP_Query( array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$date_identical_post_ids, - 'orderby' => 'post_date', + 'orderby' => 'date', 'order' => 'ASC', - 'posts_per_page' => 10, + 'posts_per_page' => 5, ) ); + remove_filter( $filter, $callback ); - remove_filter( 'posts_clauses', $filter_callback ); + $this->assertSame( "{$wpdb->posts}.post_date ASC, {$wpdb->posts}.ID ASC", $received ); + } - // The filter receives the complete ORDER BY, tie-breaker included. - $expected_orderby = "{$wpdb->posts}.post_date ASC, {$wpdb->posts}.ID ASC"; - $this->assertSame( $expected_orderby, $received_orderby, 'posts_clauses filter should receive the ORDER BY that will run, including the ID tie-breaker' ); + /** + * Data provider. + * + * @return array[] + */ + public function data_orderby_filters() { + return array( + 'posts_orderby' => array( 'posts_orderby' ), + 'posts_clauses' => array( 'posts_clauses' ), + 'posts_orderby_request' => array( 'posts_orderby_request' ), + 'posts_clauses_request' => array( 'posts_clauses_request' ), + ); } /** - * Test that ID tie-breaker is added when posts_clauses filter does not modify orderby. + * A filtered ORDER BY is used exactly as the filter returned it. * - * @ticket xxxxx + * Nothing is appended afterwards, so a filter cannot be handed back SQL it + * did not write. + * + * @ticket 44349 */ - public function test_id_tie_breaker_added_when_posts_clauses_filter_does_not_modify() { + public function test_a_filtered_orderby_is_used_verbatim() { global $wpdb; - $filter_callback = function ( $clauses ) { - return $clauses; // Return unchanged. + $callback = static function () use ( $wpdb ) { + return "{$wpdb->posts}.post_title ASC"; }; - add_filter( 'posts_clauses', $filter_callback ); + add_filter( 'posts_orderby', $callback ); + $query = new WP_Query( array( 'posts_per_page' => 5 ) ); + remove_filter( 'posts_orderby', $callback ); - $query = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$date_identical_post_ids, - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 10, - ) - ); - - remove_filter( 'posts_clauses', $filter_callback ); - - // Since filter did NOT modify orderby, ID tie-breaker SHOULD be added. - $this->assertStringContainsString( ', ' . $wpdb->posts . '.ID ASC', $query->request, 'ID tie-breaker should be added when posts_clauses filter does not modify orderby' ); + $this->assertStringContainsString( "ORDER BY {$wpdb->posts}.post_title ASC", $query->request ); + $this->assertStringNotContainsString( "post_title ASC, {$wpdb->posts}.ID", $query->request ); } /** - * Test that filter modifications to orderby are preserved. + * A filter returning an unchanged clause keeps the tie-breaker. * - * When a filter modifies the orderby, the modification should be preserved - * and we should not add the ID tie-breaker (we assume the filter knows what it's doing). + * Trailing whitespace used to be enough to lose it. * - * @ticket xxxxx + * @ticket 44349 + * + * @dataProvider data_filters_that_change_nothing + * + * @param callable $callback Filter callback. */ - public function test_filter_modifications_to_orderby_are_preserved() { - // Filter that modifies the orderby by adding post_title. - $filter_callback = function ( $orderby ) { - global $wpdb; - return $orderby . ', ' . "{$wpdb->posts}.post_title ASC"; - }; - - add_filter( 'posts_orderby', $filter_callback ); + public function test_a_filter_that_changes_nothing_keeps_the_tiebreaker( $callback ) { + global $wpdb; + add_filter( 'posts_orderby', $callback ); $query = new WP_Query( array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$date_identical_post_ids, - 'orderby' => 'post_date', + 'orderby' => 'date', 'order' => 'ASC', - 'posts_per_page' => 10, + 'posts_per_page' => 5, ) ); + remove_filter( 'posts_orderby', $callback ); - remove_filter( 'posts_orderby', $filter_callback ); - - // Verify filter modification is preserved in the final query. - $this->assertStringContainsString( 'post_title ASC', $query->request, 'Filter modification to orderby should be preserved' ); + $this->assertStringContainsString( "{$wpdb->posts}.ID ASC", $query->request ); + } - /* - * The filter appended to a clause that already carried the tie-breaker, so the - * query runs exactly what the filter returned. Nothing is added afterwards. - */ - $this->assertStringContainsString( - "{$GLOBALS['wpdb']->posts}.post_date ASC, {$GLOBALS['wpdb']->posts}.ID ASC, {$GLOBALS['wpdb']->posts}.post_title ASC", - $query->request, - 'The query should run exactly the ORDER BY the filter returned' + /** + * Data provider. + * + * @return array[] + */ + public function data_filters_that_change_nothing() { + return array( + 'returns the value it was given' => array( + static function ( $orderby ) { + return $orderby; + }, + ), + 'adds a trailing space' => array( + static function ( $orderby ) { + return $orderby . ' '; + }, + ), ); } /** - * Test that posts_clauses filter modifications to orderby are preserved. + * The ORDER BY survives a posts_clauses_request filter that leaves it out. * - * When a posts_clauses filter modifies the orderby, the modification should be preserved - * and we should not add the ID tie-breaker (we assume the filter knows what it's doing). - * - * @ticket xxxxx + * @ticket 44349 */ - public function test_posts_clauses_filter_modifications_to_orderby_are_preserved() { + public function test_orderby_survives_a_clauses_filter_that_omits_it() { global $wpdb; - // Filter that modifies the orderby via posts_clauses. - $filter_callback = function ( $clauses ) use ( $wpdb ) { - // Modify orderby to add post_title. - $clauses['orderby'] = "{$wpdb->posts}.post_date ASC, {$wpdb->posts}.post_title ASC"; + $callback = static function ( $clauses ) { + unset( $clauses['orderby'] ); return $clauses; }; - add_filter( 'posts_clauses', $filter_callback ); + add_filter( 'posts_clauses_request', $callback ); + $query = new WP_Query( array( 'posts_per_page' => 5 ) ); + remove_filter( 'posts_clauses_request', $callback ); - $query = new WP_Query( - array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$date_identical_post_ids, - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 10, - ) + $this->assertStringContainsString( + "ORDER BY {$wpdb->posts}.post_date DESC, {$wpdb->posts}.ID DESC", + $query->request ); - - remove_filter( 'posts_clauses', $filter_callback ); - - // Verify filter modification is preserved in the final query. - $this->assertStringContainsString( 'post_title ASC', $query->request, 'posts_clauses filter modification to orderby should be preserved' ); - - // Verify ID tie-breaker is NOT added when filter modifies orderby. - $this->assertStringNotContainsString( ', ' . $wpdb->posts . '.ID ASC', $query->request, 'ID tie-breaker should not be added when posts_clauses filter modifies orderby' ); } /** - * Test that ID tie-breaker is not duplicated when filter already includes ID. - * - * If a filter adds ID to the orderby, we should not add it again. + * posts_orderby_request survives a later filter that returns no ordering. * - * @ticket xxxxx + * @ticket 44349 */ - public function test_id_tie_breaker_not_duplicated_when_filter_includes_id() { + public function test_orderby_request_survives_a_later_clauses_filter() { global $wpdb; - // Filter that already includes ID in orderby. - $filter_callback = function ( $orderby ) use ( $wpdb ) { - return "{$wpdb->posts}.post_date ASC, {$wpdb->posts}.ID ASC"; + $set_orderby = static function () use ( $wpdb ) { + return "{$wpdb->posts}.post_title ASC"; + }; + $drop_orderby = static function ( $clauses ) { + unset( $clauses['orderby'] ); + return $clauses; }; - add_filter( 'posts_orderby', $filter_callback ); + add_filter( 'posts_orderby_request', $set_orderby ); + add_filter( 'posts_clauses_request', $drop_orderby ); + $query = new WP_Query( array( 'posts_per_page' => 5 ) ); + remove_filter( 'posts_orderby_request', $set_orderby ); + remove_filter( 'posts_clauses_request', $drop_orderby ); + + $this->assertStringContainsString( "ORDER BY {$wpdb->posts}.post_title ASC", $query->request ); + } + + /** + * Searching still orders by relevance first. + * + * @ticket 44349 + */ + public function test_search_relevance_still_comes_first() { + global $wpdb; $query = new WP_Query( array( - 'post_type' => 'wptests_time_ident', - 'post__in' => self::$date_identical_post_ids, - 'orderby' => 'post_date', - 'order' => 'ASC', - 'posts_per_page' => 10, + 's' => 'Same title', + 'posts_per_page' => 5, ) ); - remove_filter( 'posts_orderby', $filter_callback ); - - // Should not have duplicate ID ordering. - $this->assertStringContainsString( 'ID ASC', $query->request, 'ID should be present' ); - // Count occurrences of "ID ASC" - should be exactly 1. - $id_count = substr_count( $query->request, 'ID ASC' ); - $this->assertEquals( 1, $id_count, 'ID should not be duplicated when filter already includes it' ); + $this->assertStringContainsString( 'ORDER BY (CASE WHEN', $query->request ); + $this->assertStringContainsString( "{$wpdb->posts}.ID DESC", $query->request ); } } From 7f4bb49d3bae69900706aee4d61e9345491f2e05 Mon Sep 17 00:00:00 2001 From: Ramon Date: Tue, 1 Sep 2026 17:23:18 +1000 Subject: [PATCH 27/36] Query: only normalise the selected columns in the cache key. The cache key is built from the query with its SELECT columns rewritten to `wp_posts.*`, so that two queries differing only in 'fields' share one entry. That rewrite replaced every occurrence, and now that ORDER BY ends in `wp_posts.ID`, a query with 'fields' => 'ids' had its ORDER BY rewritten too. Its key stopped matching the same query asking for full post objects, so the second one missed the cache and ran again. Replaces the first occurrence only, which is the SELECT list. Reverses the removal in [25f35b58ff]: the greedy replacement was harmless before the ID was added to the default ORDER BY, and is not now. See #44349. --- src/wp-includes/class-wp-query.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index b19d3ec96c9b4..c1fcb02284b1b 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -3357,8 +3357,18 @@ public function get_posts() { } if ( $query_vars['cache_results'] && $id_query_is_cacheable ) { - $new_request = str_replace( $fields, "{$wpdb->posts}.*", $this->request ); - $cache_key = $this->generate_cache_key( $query_vars, $new_request ); + /* + * Normalise the selected columns so that queries differing only in 'fields' + * share a cache key. Only the first occurrence is replaced: the same column + * names also appear in ORDER BY, and rewriting them there would give the + * same query two different keys. + */ + $pos = strpos( $this->request, $fields ); + $new_request = false === $pos + ? $this->request + : substr_replace( $this->request, "{$wpdb->posts}.*", $pos, strlen( $fields ) ); + + $cache_key = $this->generate_cache_key( $query_vars, $new_request ); $cache_found = false; if ( null === $this->posts ) { From dfeea0f095f879b4d3463f0ab3037dbd0aa1197f Mon Sep 17 00:00:00 2001 From: Ramon Date: Tue, 1 Sep 2026 17:23:50 +1000 Subject: [PATCH 28/36] Docs: record the version the ordering change landed in. Replaces the x.x.x placeholder on WP_Query::get_posts() and sets the same version on the two methods added alongside it. See #44349. --- src/wp-includes/class-wp-query.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index c1fcb02284b1b..7144d2286771f 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -1841,7 +1841,7 @@ protected function parse_order( $order ) { * Ordering by ID, by an explicit list of IDs, or at random already gives every * post a distinct position, so no tie-breaker is needed on top of it. * - * @since 7.0.0 + * @since 7.2.0 * * @param string $orderby Single 'orderby' value, before it is parsed into SQL. * @return bool Whether the value orders posts uniquely. @@ -1869,7 +1869,7 @@ protected function is_unique_orderby( $orderby ) { * The tie-breaker follows the direction of the last clause it is appended to, so * that reversing a query's 'order' also reverses the sequence of tied posts. * - * @since 7.0.0 + * @since 7.2.0 * * @param string[] $orderby_array ORDER BY clauses built so far, each ending in a direction. * @param string $order The query's 'order' value, used when no clause carries one. @@ -1942,7 +1942,8 @@ public function set( $query_var, $value ) { * database query. * * @since 1.5.0 - * @since x.x.x Adds deterministic ordering to prevent duplicate records across pages. + * @since 7.2.0 Adds the post ID to ORDER BY so that paginated queries do not + * return the same post on more than one page. * * @global wpdb $wpdb WordPress database abstraction object. * From df7897c3fb8d76e6aa0912c256867bb2a7435cf3 Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 2 Sep 2026 11:29:54 +1000 Subject: [PATCH 29/36] Query: match parse_orderby() when spotting a seeded RAND. Both places that recognise a random seed now use the same pattern, so a change to one cannot leave the other behind. The previous pattern was anchored and allowed spaces, accepting forms parse_orderby() rejects and rejecting none that it takes, but the two were free to drift apart. Covers the seed forms parse_orderby() honours, and checks that a seeded query returns the same page twice, which appending the ID would break. Follow-up to [3bf4883afa]. Props peterwilsoncc. See #44349. --- src/wp-includes/class-wp-query.php | 8 ++++-- .../tests/query/deterministicOrdering.php | 28 +++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 7144d2286771f..c6fd16a418d12 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -1859,8 +1859,12 @@ protected function is_unique_orderby( $orderby ) { return true; } - // Seeded random, for example 'RAND(5)', is handled by parse_orderby(). - return 1 === preg_match( '/^RAND\(\s*[0-9]*\s*\)$/i', $orderby ); + /* + * Random ordering with a seed, for example 'RAND(5)'. The seed is there to get + * the same shuffle back on every page, which a sort on ID afterwards would undo. + * Matches the pattern parse_orderby() accepts the seed with. + */ + return 1 === preg_match( '/RAND\(([0-9]+)\)/i', $orderby ); } /** diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index e20535b51d5f4..9ecd7f6e81860 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -367,8 +367,32 @@ public function test_random_ordering_gets_no_tiebreaker( $orderby ) { */ public function data_random_orderby() { return array( - 'unseeded' => array( 'rand' ), - 'seeded' => array( 'RAND(5)' ), + 'unseeded' => array( 'rand' ), + 'seeded' => array( 'RAND(5)' ), + 'seeded with zero' => array( 'RAND(0)' ), + 'seeded, lower case' => array( 'rand(5)' ), + 'seeded, large number' => array( 'RAND(99999999999)' ), + ); + } + + /** + * A seed returns the same shuffle on every page. + * + * @ticket 44349 + */ + public function test_a_random_seed_gives_the_same_order_each_time() { + $args = array( + 'post_type' => 'page', + 'post__in' => self::$menu_order_ids, + 'orderby' => 'RAND(5)', + 'posts_per_page' => 5, + 'paged' => 1, + ); + + $this->assertSame( + $this->get_page_of_ids( $args ), + $this->get_page_of_ids( $args ), + 'A seeded random order changed between two runs of the same query' ); } From e1d23b9cae991bfa0089a49197d10bffc2369a76 Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 2 Sep 2026 14:49:39 +1000 Subject: [PATCH 30/36] Tests: cover media pagination ordered by a shared column. Unattached uploads all have a post_parent of 0, so ordering /wp/v2/media by it leaves every attachment tied. Paging through that returned some attachments twice and never returned others. This is the path the media library's DataViews table takes when sorting on "Uploaded to", and the one the block editor reaches through core-data. Fails without the accompanying fix. See #44349, #46294. --- .../rest-api/rest-attachments-controller.php | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/phpunit/tests/rest-api/rest-attachments-controller.php b/tests/phpunit/tests/rest-api/rest-attachments-controller.php index 4dd0b60172cb4..1c9b66d33372a 100644 --- a/tests/phpunit/tests/rest-api/rest-attachments-controller.php +++ b/tests/phpunit/tests/rest-api/rest-attachments-controller.php @@ -423,6 +423,49 @@ public function test_get_items() { $this->check_get_posts_response( $response ); } + /** + * Paging through media ordered by a shared column returns each item once. + * + * Unattached uploads all have a post_parent of 0, so ordering by it leaves + * every attachment tied. The media library and its DataViews table sort on + * that column, which is where this surfaced. + * + * @ticket 44349 + * @ticket 46294 + */ + public function test_get_items_paged_by_parent_returns_each_attachment_once() { + wp_set_current_user( self::$editor_id ); + + $expected = 10; + for ( $i = 0; $i < $expected; $i++ ) { + self::factory()->attachment->create_object( + array( + 'file' => "image-$i.jpg", + 'post_parent' => 0, + 'post_mime_type' => 'image/jpeg', + 'post_status' => 'inherit', + ) + ); + } + + $seen = array(); + for ( $page = 1; $page <= 2; $page++ ) { + $request = new WP_REST_Request( 'GET', '/wp/v2/media' ); + $request->set_param( 'orderby', 'parent' ); + $request->set_param( 'order', 'asc' ); + $request->set_param( 'per_page', 5 ); + $request->set_param( 'page', $page ); + + $response = rest_get_server()->dispatch( $request ); + $this->assertSame( 200, $response->get_status() ); + + $seen = array_merge( $seen, wp_list_pluck( $response->get_data(), 'id' ) ); + } + + $this->assertSameSets( array_unique( $seen ), $seen, 'An attachment was returned on more than one page' ); + $this->assertCount( $expected, $seen, 'The pages did not add up to every attachment' ); + } + public function test_get_items_logged_in_editor() { wp_set_current_user( self::$editor_id ); $id1 = self::factory()->attachment->create_object( From fb944c6a4d1798d7350526c9dc9efec753df144e Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 2 Sep 2026 15:05:51 +1000 Subject: [PATCH 31/36] Query: track the tie-breaker's direction instead of parsing it back out. The direction is known when each ORDER BY clause is built, so record it there rather than reading it back off the generated SQL with a regular expression. Removes parse_tiebreaker_order(). WP_Query is widely subclassed, so a protected method is close to public API and worth not adding without need. No change in behaviour, including for an array 'orderby' whose clauses sort in different directions, and for post__in and friends, where 'order' is forced empty and the tie-breaker falls back to the default. Follow-up to [3bf4883afa]. See #44349. --- src/wp-includes/class-wp-query.php | 35 ++++++++++-------------------- 1 file changed, 11 insertions(+), 24 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index c6fd16a418d12..c35fa3a85be11 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -1867,28 +1867,6 @@ protected function is_unique_orderby( $orderby ) { return 1 === preg_match( '/RAND\(([0-9]+)\)/i', $orderby ); } - /** - * Works out which direction the ID tie-breaker should sort in. - * - * The tie-breaker follows the direction of the last clause it is appended to, so - * that reversing a query's 'order' also reverses the sequence of tied posts. - * - * @since 7.2.0 - * - * @param string[] $orderby_array ORDER BY clauses built so far, each ending in a direction. - * @param string $order The query's 'order' value, used when no clause carries one. - * @return string Either 'ASC' or 'DESC'. - */ - protected function parse_tiebreaker_order( $orderby_array, $order ) { - $last_clause = end( $orderby_array ); - - if ( is_string( $last_clause ) && preg_match( '/\s(ASC|DESC)\s*$/i', $last_clause, $matches ) ) { - return strtoupper( $matches[1] ); - } - - return $this->parse_order( $order ); - } - /** * Sets the 404 property and saves whether query is feed. * @@ -2594,6 +2572,14 @@ public function get_posts() { */ $found_unique_orderby = false; + /* + * Direction of the last clause added. The tie-breaker follows it, so that + * reversing the column it breaks ties for also reverses the tied posts. + * Each clause of an array 'orderby' carries its own direction, so the + * query's 'order' is not necessarily the last one used. + */ + $last_order = $query_vars['order']; + if ( is_array( $query_vars['orderby'] ) ) { foreach ( $query_vars['orderby'] as $_orderby => $order ) { $orderby = wp_slash( urldecode( $_orderby ) ); @@ -2603,7 +2589,8 @@ public function get_posts() { continue; } - $orderby_array[] = $parsed . ' ' . $this->parse_order( $order ); + $last_order = $this->parse_order( $order ); + $orderby_array[] = $parsed . ' ' . $last_order; if ( $this->is_unique_orderby( $orderby ) ) { $found_unique_orderby = true; @@ -2639,7 +2626,7 @@ public function get_posts() { * Skipped when the ordering already ends in a unique value. */ if ( ! $found_unique_orderby ) { - $orderby_array[] = "{$wpdb->posts}.ID " . $this->parse_tiebreaker_order( $orderby_array, $query_vars['order'] ); + $orderby_array[] = "{$wpdb->posts}.ID " . $this->parse_order( $last_order ); } $orderby = trim( implode( ', ', $orderby_array ) ); From fc8c389415ed7f53b6d565d87f72607905b84405 Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 2 Sep 2026 15:19:37 +1000 Subject: [PATCH 32/36] Query: describe the ID clause the way WP_Comment_Query does. The comments called the appended clause a "tie-breaker", a term core does not otherwise use. WP_Comment_Query solves the same problem and says "to ensure determinate sorting, always include a comment_ID clause", which needs no glossary. Follow that wording, and rename is_unique_orderby() to is_orderby_id() and $found_unique_orderby to $found_orderby_id to match its $found_orderby_comment_id. Two comments also described the code inaccurately. One said the ordering had to "end in" a unique value, when a unique column anywhere in the list is enough: 'orderby' => array( 'ID' => 'DESC', 'title' => 'ASC' ) correctly gets no extra clause. The other said random ordering "cannot produce ties", which is not true of RAND() and is not the reason it is left alone; the reason is that an ID clause would either change nothing or undo a seeded shuffle. Comments and test names only. Follow-up to [3bf4883afa]. See #44349. --- src/wp-includes/class-wp-query.php | 60 +++++++++---------- .../tests/query/deterministicOrdering.php | 24 ++++---- 2 files changed, 40 insertions(+), 44 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index c35fa3a85be11..1798180685b44 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -1836,18 +1836,19 @@ protected function parse_order( $order ) { } /** - * Determines whether an 'orderby' value can never produce two equal rows. + * Determines whether an 'orderby' value already puts posts in a fixed sequence. * - * Ordering by ID, by an explicit list of IDs, or at random already gives every - * post a distinct position, so no tie-breaker is needed on top of it. + * True for the ID and for an explicit list of IDs, which give every post a + * distinct position, and for random ordering, where sorting by ID afterwards + * would either change nothing or undo a seeded shuffle. * * @since 7.2.0 * * @param string $orderby Single 'orderby' value, before it is parsed into SQL. - * @return bool Whether the value orders posts uniquely. + * @return bool Whether an ID clause would make any difference. */ - protected function is_unique_orderby( $orderby ) { - $unique_orderby = array( + protected function is_orderby_id( $orderby ) { + $orderby_id = array( 'ID', 'rand', 'post__in', @@ -1855,15 +1856,11 @@ protected function is_unique_orderby( $orderby ) { 'post_parent__in', ); - if ( in_array( $orderby, $unique_orderby, true ) ) { + if ( in_array( $orderby, $orderby_id, true ) ) { return true; } - /* - * Random ordering with a seed, for example 'RAND(5)'. The seed is there to get - * the same shuffle back on every page, which a sort on ID afterwards would undo. - * Matches the pattern parse_orderby() accepts the seed with. - */ + // Random ordering with a seed, for example 'RAND(5)', as parse_orderby() accepts it. return 1 === preg_match( '/RAND\(([0-9]+)\)/i', $orderby ); } @@ -2548,10 +2545,10 @@ public function get_posts() { $orderby = ''; } else { /* - * Sorting by post_date alone is not stable: posts sharing a date can be + * Sorting by post_date alone is not determinate: posts sharing a date are * returned in a different sequence each time the query runs, so a post - * can appear on two pages at once or be skipped entirely. Appending the - * ID breaks those ties and gives every page a fixed sequence. + * can appear on two pages at once or be missed entirely. The ID clause + * gives every page a fixed sequence. */ $orderby = "{$wpdb->posts}.post_date {$query_vars['order']}, {$wpdb->posts}.ID {$query_vars['order']}"; } @@ -2566,17 +2563,16 @@ public function get_posts() { $orderby_array = array(); /* - * Tracks whether the requested ordering already ends in a unique value. - * Ordering by ID, by an explicit list of IDs, or at random cannot produce - * ties, so those need no tie-breaker appended. + * Whether the ordering already puts the posts in a fixed sequence, in which + * case an ID clause would make no difference. */ - $found_unique_orderby = false; + $found_orderby_id = false; /* - * Direction of the last clause added. The tie-breaker follows it, so that - * reversing the column it breaks ties for also reverses the tied posts. - * Each clause of an array 'orderby' carries its own direction, so the - * query's 'order' is not necessarily the last one used. + * An array 'orderby' gives each column its own direction, so the query's + * 'order' is not necessarily the one the last column used. Track it, so the + * ID sorts the same way as the column above it and reversing the query + * reverses the whole page. */ $last_order = $query_vars['order']; @@ -2592,8 +2588,8 @@ public function get_posts() { $last_order = $this->parse_order( $order ); $orderby_array[] = $parsed . ' ' . $last_order; - if ( $this->is_unique_orderby( $orderby ) ) { - $found_unique_orderby = true; + if ( $this->is_orderby_id( $orderby ) ) { + $found_orderby_id = true; } } } else { @@ -2609,8 +2605,8 @@ public function get_posts() { $orderby_array[] = $parsed . ' ' . $query_vars['order']; - if ( $this->is_unique_orderby( $orderby ) ) { - $found_unique_orderby = true; + if ( $this->is_orderby_id( $orderby ) ) { + $found_orderby_id = true; } } } @@ -2620,12 +2616,12 @@ public function get_posts() { } /* - * Posts sharing the same value for the requested column can be returned in - * a different sequence each time the query runs, so a post can appear on two - * pages at once or be skipped entirely. Appending the ID breaks those ties. - * Skipped when the ordering already ends in a unique value. + * To ensure determinate sorting, always include an ID clause. Posts sharing + * the same value for the requested column are otherwise returned in a + * different sequence each time the query runs, so a post can appear on two + * pages at once or be missed entirely. */ - if ( ! $found_unique_orderby ) { + if ( ! $found_orderby_id ) { $orderby_array[] = "{$wpdb->posts}.ID " . $this->parse_order( $last_order ); } diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index 9ecd7f6e81860..64429236e00e8 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -41,7 +41,7 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { /* * Every post here shares one date. Splitting them across two statuses stops * the database using the type_status_date index, which happens to end in ID - * and would otherwise hide the missing tie-breaker. + * and would otherwise hide the missing ID clause. */ for ( $i = 1; $i <= 20; $i++ ) { self::$mixed_status_ids[] = $factory->post->create( @@ -204,16 +204,16 @@ public function test_repeating_a_query_returns_the_same_page() { } /** - * The ID tie-breaker follows the direction of the clause it is added to. + * The ID clause sorts the same way as the column above it. * * @ticket 44349 * - * @dataProvider data_tiebreaker_directions + * @dataProvider data_orderby_directions * * @param array $args Query arguments. * @param string $expected Expected ORDER BY clause, with {posts} standing in for the table name. */ - public function test_tiebreaker_follows_the_sort_direction( $args, $expected ) { + public function test_id_clause_follows_the_sort_direction( $args, $expected ) { global $wpdb; $query = new WP_Query( array_merge( $args, array( 'posts_per_page' => 5 ) ) ); @@ -229,7 +229,7 @@ public function test_tiebreaker_follows_the_sort_direction( $args, $expected ) { * * @return array[] */ - public function data_tiebreaker_directions() { + public function data_orderby_directions() { return array( 'descending date' => array( array( @@ -273,15 +273,15 @@ public function data_tiebreaker_directions() { } /** - * Ordering that is already unique gets no tie-breaker added to it. + * Ordering that already fixes the sequence gets no extra ID clause. * * @ticket 44349 * - * @dataProvider data_orderby_that_is_already_unique + * @dataProvider data_orderby_that_fixes_the_sequence * * @param array $args Query arguments. */ - public function test_unique_orderby_gets_no_tiebreaker( $args ) { + public function test_orderby_id_gets_no_extra_id_clause( $args ) { global $wpdb; $query = new WP_Query( array_merge( $args, array( 'posts_per_page' => 5 ) ) ); @@ -301,7 +301,7 @@ public function test_unique_orderby_gets_no_tiebreaker( $args ) { * * @return array[] */ - public function data_orderby_that_is_already_unique() { + public function data_orderby_that_fixes_the_sequence() { return array( 'ID' => array( array( @@ -345,7 +345,7 @@ public function data_orderby_that_is_already_unique() { * * @param string $orderby The 'orderby' value. */ - public function test_random_ordering_gets_no_tiebreaker( $orderby ) { + public function test_random_ordering_gets_no_id_clause( $orderby ) { global $wpdb; $query = new WP_Query( @@ -539,7 +539,7 @@ public function test_a_filtered_orderby_is_used_verbatim() { } /** - * A filter returning an unchanged clause keeps the tie-breaker. + * A filter returning an unchanged clause keeps the ID clause. * * Trailing whitespace used to be enough to lose it. * @@ -549,7 +549,7 @@ public function test_a_filtered_orderby_is_used_verbatim() { * * @param callable $callback Filter callback. */ - public function test_a_filter_that_changes_nothing_keeps_the_tiebreaker( $callback ) { + public function test_a_filter_that_changes_nothing_keeps_the_id_clause( $callback ) { global $wpdb; add_filter( 'posts_orderby', $callback ); From e6945a50edc9a6a78d951167877cb6b5eccb1b65 Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 2 Sep 2026 17:01:46 +1000 Subject: [PATCH 33/36] Query: append the ID clause to parent and slug list orderings too. is_orderby_id() treated 'post_parent__in' and 'post_name__in' as already determinate, but neither is. FIELD( wp_posts.post_parent, ... ) gives every child of the same parent the same sort value, and slugs are only unique within one post type and parent, so both orderings can tie - and paging a set of pages sharing one parent returned a page twice and missed another entirely. Only the ID itself, post__in (a FIELD over the unique ID), and random ordering stay exempt. The appended ID clause now inherits a blank direction instead of turning it into DESC. 'order' is forced empty for the FIELD()-based orderings, whose clauses sort implicitly ascending; the ID should follow them, and today's observed within-group order (ascending ID) is what existing behaviour and the assertions in tests/phpunit/tests/query/results.php already expect. Also corrects the inverted @return description on is_orderby_id(). Follow-up to [3bf4883afa]. See #44349. --- src/wp-includes/class-wp-query.php | 19 +++-- .../tests/query/deterministicOrdering.php | 84 +++++++++++++++++++ 2 files changed, 96 insertions(+), 7 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 1798180685b44..2b97668e89566 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -1838,22 +1838,22 @@ protected function parse_order( $order ) { /** * Determines whether an 'orderby' value already puts posts in a fixed sequence. * - * True for the ID and for an explicit list of IDs, which give every post a - * distinct position, and for random ordering, where sorting by ID afterwards - * would either change nothing or undo a seeded shuffle. + * True for the ID, for an explicit list of IDs, and for random ordering, where + * an ID clause would either change nothing or undo a seeded shuffle. + * + * Not true for 'post_parent__in' or 'post_name__in': several posts can share + * one parent, or one slug across post types, so those orderings can tie. * * @since 7.2.0 * * @param string $orderby Single 'orderby' value, before it is parsed into SQL. - * @return bool Whether an ID clause would make any difference. + * @return bool Whether the ordering is already determinate, making an ID clause unnecessary. */ protected function is_orderby_id( $orderby ) { $orderby_id = array( 'ID', 'rand', 'post__in', - 'post_name__in', - 'post_parent__in', ); if ( in_array( $orderby, $orderby_id, true ) ) { @@ -2622,7 +2622,12 @@ public function get_posts() { * pages at once or be missed entirely. */ if ( ! $found_orderby_id ) { - $orderby_array[] = "{$wpdb->posts}.ID " . $this->parse_order( $last_order ); + /* + * $last_order is already 'ASC', 'DESC', or '' here. Blank stays blank: + * 'order' is forced empty for the FIELD()-based orderings, whose clauses + * sort implicitly ascending, and the ID should sort the same way. + */ + $orderby_array[] = trim( "{$wpdb->posts}.ID " . $last_order ); } $orderby = trim( implode( ', ', $orderby_array ) ); diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index 64429236e00e8..890b69341c2a8 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -417,6 +417,90 @@ public function test_post__in_keeps_its_own_order() { $this->assertSame( $ids, wp_list_pluck( $query->posts, 'ID' ) ); } + /** + * Ordering by a list of parents or slugs still gets the ID clause. + * + * Unlike post__in, these lists do not order posts uniquely: several posts can + * share one parent, or one slug across post types. + * + * @ticket 44349 + */ + public function test_field_orderings_get_the_id_clause() { + $parent = self::factory()->post->create( + array( + 'post_type' => 'page', + 'post_title' => 'FIELD parent', + ) + ); + self::factory()->post->create( + array( + 'post_type' => 'page', + 'post_parent' => $parent, + ) + ); + self::factory()->post->create( + array( + 'post_type' => 'page', + 'post_parent' => $parent, + ) + ); + + $query = new WP_Query( + array( + 'post_type' => 'page', + 'post_parent__in' => array( $parent ), + 'orderby' => 'post_parent__in', + 'posts_per_page' => 5, + ) + ); + + global $wpdb; + preg_match( '/ORDER BY(.*?)LIMIT/s', $query->request, $matches ); + + $this->assertStringContainsString( "FIELD( {$wpdb->posts}.post_parent,", $matches[1] ); + $this->assertStringContainsString( "{$wpdb->posts}.ID", $matches[1] ); + } + + /** + * Paging posts that all share one parent returns each post exactly once. + * + * FIELD( wp_posts.post_parent, ... ) gives every child of the same parent the + * same sort value, so without the ID clause the whole result set is one tie. + * + * @ticket 44349 + */ + public function test_paging_by_post_parent__in_returns_each_post_once() { + $parent = self::factory()->post->create( + array( + 'post_type' => 'page', + 'post_title' => 'Shared parent', + ) + ); + + $children = array(); + for ( $i = 1; $i <= 12; $i++ ) { + $children[] = self::factory()->post->create( + array( + 'post_type' => 'page', + 'post_title' => "Child $i", + 'post_parent' => $parent, + ) + ); + } + + $this->assertPagesDoNotRepeatPosts( + array( + 'post_type' => 'page', + 'post_parent__in' => array( $parent ), + 'orderby' => 'post_parent__in', + ), + 5, + 3, + 12, + 'Ordering by post_parent__in' + ); + } + /** * An 'orderby' of 'none' still produces no ORDER BY. * From e6b9b3d003c535a8ee41aaa365ca4df05035beec Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 2 Sep 2026 17:02:53 +1000 Subject: [PATCH 34/36] Query: only blank the ORDER BY when 'none' is the whole ordering. The array check accepted 'none' as ANY key of an array 'orderby', so get_pages( array( 'sort_column' => 'post_title,none' ) ) - which maps to array( 'post_title' => ..., 'none' => ... ) - dropped the whole ORDER BY, where trunk kept the title ordering. An unordered paginated query is the exact failure this branch fixes. 'none' now blanks the clause only as a bare string or as the array's only key, the form get_pages() sends for 'sort_column' => 'none'. Next to real columns it falls through and is skipped like any other unparseable key, so the remaining columns order as requested, ID clause included. Follow-up to [3bf4883afa]. See #44349. --- src/wp-includes/class-wp-query.php | 8 ++++-- .../tests/query/deterministicOrdering.php | 28 +++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 2b97668e89566..94d8a1292b4c1 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -2552,11 +2552,13 @@ public function get_posts() { */ $orderby = "{$wpdb->posts}.post_date {$query_vars['order']}, {$wpdb->posts}.ID {$query_vars['order']}"; } - } elseif ( 'none' === $query_vars['orderby'] || isset( $query_vars['orderby']['none'] ) ) { + } elseif ( 'none' === $query_vars['orderby'] || array( 'none' ) === array_keys( (array) $query_vars['orderby'] ) ) { /* * 'none' blanks out ORDER BY. It arrives as a bare string from WP_Query, and - * as an array key from get_pages(), which turns its 'sort_column' into - * array( 'none' => $sort_order ). + * as the array's only key from get_pages(), which turns its 'sort_column' + * into array( 'none' => $sort_order ). When 'none' appears in an array next + * to real columns it is not the whole ordering, so it falls through and is + * skipped below like any other unparseable key. */ $orderby = ''; } else { diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index 890b69341c2a8..ac1a08fa78700 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -535,6 +535,34 @@ public function data_orderby_that_blanks_the_clause() { ); } + /** + * 'none' next to real columns does not blank the ordering. + * + * get_pages( array( 'sort_column' => 'post_title,none' ) ) produces + * array( 'post_title' => ..., 'none' => ... ); only the 'none' part is + * dropped, the rest orders as requested. + * + * @ticket 44349 + */ + public function test_none_beside_real_columns_keeps_the_ordering() { + global $wpdb; + + $query = new WP_Query( + array( + 'orderby' => array( + 'title' => 'ASC', + 'none' => 'DESC', + ), + 'posts_per_page' => 5, + ) + ); + + $this->assertStringContainsString( + "ORDER BY {$wpdb->posts}.post_title ASC, {$wpdb->posts}.ID ASC", + $query->request + ); + } + /** * get_pages() can still ask for no ordering. * From 933c75971d4e819a4f10ecdff3c99aeb9d26daff Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 2 Sep 2026 17:04:16 +1000 Subject: [PATCH 35/36] Query: keep an all-invalid array 'orderby' unordered, as on trunk. The rewrite gave the array branch the string branch's post_date fallback, so 'orderby' => array( 'invalid_field' => 'ASC' ) switched from running with no ORDER BY to ordering by post_date and ID. Nothing reported that behaviour and no test pinned the change, so restore the old result: the fallback now applies only to a string 'orderby', where it always has, and an array whose keys all fail to parse produces no ORDER BY. Follow-up to [3bf4883afa]. See #44349. --- src/wp-includes/class-wp-query.php | 12 ++++++++---- tests/phpunit/tests/query/deterministicOrdering.php | 9 +++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 94d8a1292b4c1..f26be995ef89b 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -2611,10 +2611,11 @@ public function get_posts() { $found_orderby_id = true; } } - } - if ( empty( $orderby_array ) ) { - $orderby_array[] = "{$wpdb->posts}.post_date " . $query_vars['order']; + // If no valid clauses were found, order by post_date. + if ( empty( $orderby_array ) ) { + $orderby_array[] = "{$wpdb->posts}.post_date " . $query_vars['order']; + } } /* @@ -2622,8 +2623,11 @@ public function get_posts() { * the same value for the requested column are otherwise returned in a * different sequence each time the query runs, so a post can appear on two * pages at once or be missed entirely. + * + * An array 'orderby' whose keys all failed to parse stays empty here and + * produces no ORDER BY at all, as it always has. */ - if ( ! $found_orderby_id ) { + if ( ! $found_orderby_id && ! empty( $orderby_array ) ) { /* * $last_order is already 'ASC', 'DESC', or '' here. Blank stays blank: * 'order' is forced empty for the FIELD()-based orderings, whose clauses diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index ac1a08fa78700..ad4cd04029ede 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -528,10 +528,11 @@ public function test_orderby_can_still_be_blanked( $orderby ) { */ public function data_orderby_that_blanks_the_clause() { return array( - 'the string none' => array( 'none' ), - 'none as an array key' => array( array( 'none' => 'DESC' ) ), - 'an empty array' => array( array() ), - 'false' => array( false ), + 'the string none' => array( 'none' ), + 'none as an array key' => array( array( 'none' => 'DESC' ) ), + 'an empty array' => array( array() ), + 'false' => array( false ), + 'an array of only invalid fields' => array( array( 'a_column_that_does_not_exist' => 'ASC' ) ), ); } From 6366ed17fba080307f9dc4fe6ba14c07392b0ca6 Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 2 Sep 2026 17:12:48 +1000 Subject: [PATCH 36/36] Tests: cover meta ordering and page pagination; tighten the assertions. Two orderings the fix covers had no test at all: a meta value shared by every post, and /wp/v2/pages paged by menu_order - the scenario from ticket 46294, where pages usually all tie on the default menu_order of 0. Both new tests fail without the fix. The repeat-query test compared the query cache with itself: the second identical query never reached the database. It now runs uncached. The page-walk assertions counted rows, which duplicated posts pad back up to the expected total; counting distinct posts makes a missed post fail the count as well as the set comparison. Also replaces a test docblock that described this branch's own development history as though it were released behaviour, fixes docblock alignment, and uses the US spelling in the cache-key comment. See #44349, #46294. --- src/wp-includes/class-wp-query.php | 2 +- .../tests/query/deterministicOrdering.php | 46 ++++++++++++++++--- .../rest-api/rest-attachments-controller.php | 2 +- .../tests/rest-api/rest-pages-controller.php | 38 +++++++++++++++ 4 files changed, 79 insertions(+), 9 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index f26be995ef89b..d60e3e9b3049e 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -3357,7 +3357,7 @@ public function get_posts() { if ( $query_vars['cache_results'] && $id_query_is_cacheable ) { /* - * Normalise the selected columns so that queries differing only in 'fields' + * Normalize the selected columns so that queries differing only in 'fields' * share a cache key. Only the first occurrence is replaced: the same column * names also appear in ORDER BY, and rewriting them there would give the * same query two different keys. diff --git a/tests/phpunit/tests/query/deterministicOrdering.php b/tests/phpunit/tests/query/deterministicOrdering.php index ad4cd04029ede..42aa664e870c0 100644 --- a/tests/phpunit/tests/query/deterministicOrdering.php +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -88,11 +88,11 @@ private function get_page_of_ids( $args ) { /** * Asserts that paging through a query returns every post exactly once. * - * @param array $args Query arguments, without 'posts_per_page' or 'paged'. - * @param int $per_page Posts per page. - * @param int $pages Number of pages to walk. - * @param int $expected Total number of posts expected across those pages. - * @param string $message Message describing the ordering under test. + * @param array $args Query arguments, without 'posts_per_page' or 'paged'. + * @param int $per_page Posts per page. + * @param int $pages Number of pages to walk. + * @param int $expected Total number of posts expected across those pages. + * @param string $message Message describing the ordering under test. */ private function assertPagesDoNotRepeatPosts( $args, $per_page, $pages, $expected, $message ) { $seen = array(); @@ -113,7 +113,7 @@ private function assertPagesDoNotRepeatPosts( $args, $per_page, $pages, $expecte } $this->assertSameSets( array_unique( $seen ), $seen, $message . ': a post appeared on more than one page' ); - $this->assertCount( $expected, $seen, $message . ': the pages did not add up to every post' ); + $this->assertCount( $expected, array_unique( $seen ), $message . ': the pages did not add up to every post' ); } /** @@ -181,6 +181,36 @@ public function test_paging_by_title_returns_each_post_once() { ); } + /** + * Ordering by a meta value is stable when posts share the value. + * + * @ticket 44349 + */ + public function test_paging_by_meta_value_returns_each_post_once() { + global $wpdb; + + $post_ids = array(); + for ( $i = 1; $i <= 12; $i++ ) { + $post_id = self::factory()->post->create( array( 'post_title' => "Meta post $i" ) ); + add_post_meta( $post_id, 'shared_value', 'identical' ); + $post_ids[] = $post_id; + } + + $args = array( + 'post_type' => 'post', + 'post__in' => $post_ids, + 'meta_key' => 'shared_value', + 'orderby' => 'meta_value', + 'order' => 'ASC', + ); + + $this->assertPagesDoNotRepeatPosts( $args, 5, 3, 12, 'Ordering by meta_value' ); + + // The ID clause follows the meta clause in the SQL. + $query = new WP_Query( array_merge( $args, array( 'posts_per_page' => 5 ) ) ); + $this->assertStringContainsString( ".meta_value ASC, {$wpdb->posts}.ID ASC", $query->request ); + } + /** * The same query run twice returns the same page in the same order. * @@ -194,6 +224,7 @@ public function test_repeating_a_query_returns_the_same_page() { 'order' => 'ASC', 'posts_per_page' => 10, 'paged' => 1, + 'cache_results' => false, // Make the second run hit the database, not the query cache. ); $this->assertSame( @@ -654,7 +685,8 @@ public function test_a_filtered_orderby_is_used_verbatim() { /** * A filter returning an unchanged clause keeps the ID clause. * - * Trailing whitespace used to be enough to lose it. + * Incidental changes such as added trailing whitespace must not matter: the + * ID clause is part of the value the filter receives, not compared against it. * * @ticket 44349 * diff --git a/tests/phpunit/tests/rest-api/rest-attachments-controller.php b/tests/phpunit/tests/rest-api/rest-attachments-controller.php index 1c9b66d33372a..9a72c8f096ebb 100644 --- a/tests/phpunit/tests/rest-api/rest-attachments-controller.php +++ b/tests/phpunit/tests/rest-api/rest-attachments-controller.php @@ -463,7 +463,7 @@ public function test_get_items_paged_by_parent_returns_each_attachment_once() { } $this->assertSameSets( array_unique( $seen ), $seen, 'An attachment was returned on more than one page' ); - $this->assertCount( $expected, $seen, 'The pages did not add up to every attachment' ); + $this->assertCount( $expected, array_unique( $seen ), 'The pages did not add up to every attachment' ); } public function test_get_items_logged_in_editor() { diff --git a/tests/phpunit/tests/rest-api/rest-pages-controller.php b/tests/phpunit/tests/rest-api/rest-pages-controller.php index 9717a7fcda1c6..61ededb874862 100644 --- a/tests/phpunit/tests/rest-api/rest-pages-controller.php +++ b/tests/phpunit/tests/rest-api/rest-pages-controller.php @@ -282,6 +282,44 @@ public function test_get_items_menu_order_query() { $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); } + /** + * Paging pages that share a menu_order returns each page exactly once. + * + * Pages default to menu_order 0, so a site's pages usually all tie on it. + * + * @ticket 44349 + * @ticket 46294 + */ + public function test_get_items_paged_by_menu_order_returns_each_page_once() { + $expected = 10; + for ( $i = 0; $i < $expected; $i++ ) { + self::factory()->post->create( + array( + 'post_status' => 'publish', + 'post_type' => 'page', + 'menu_order' => 0, + ) + ); + } + + $seen = array(); + for ( $page = 1; $page <= 2; $page++ ) { + $request = new WP_REST_Request( 'GET', '/wp/v2/pages' ); + $request->set_param( 'orderby', 'menu_order' ); + $request->set_param( 'order', 'asc' ); + $request->set_param( 'per_page', 5 ); + $request->set_param( 'page', $page ); + + $response = rest_get_server()->dispatch( $request ); + $this->assertSame( 200, $response->get_status() ); + + $seen = array_merge( $seen, wp_list_pluck( $response->get_data(), 'id' ) ); + } + + $this->assertSameSets( array_unique( $seen ), $seen, 'A page was returned on more than one result page' ); + $this->assertCount( $expected, array_unique( $seen ), 'The result pages did not add up to every page' ); + } + public function test_get_items_min_max_pages_query() { $request = new WP_REST_Request( 'GET', '/wp/v2/pages' ); $request->set_param( 'per_page', 0 );