diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 3268351e99347..bd5d9cd22de23 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -1886,6 +1886,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. * @@ -2493,6 +2494,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, @@ -2501,12 +2509,44 @@ 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. + * + * Build base orderby without ID tie-breaker for filters, then add it after filters. + */ + $orderby = "{$wpdb->posts}.post_date " . $query_vars['order']; + $deterministic_orderby_meta['needed'] = true; } - } 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'] || isset( $query_vars['orderby']['none'] ) ) { $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. + * + * 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', + 'rand', + 'relevance', + 'post__in', + 'post_name__in', + 'post_parent__in', + 'include', + ); + $orderby_array = array(); + if ( is_array( $query_vars['orderby'] ) ) { foreach ( $query_vars['orderby'] as $_orderby => $order ) { $orderby = addslashes_gpc( urldecode( $_orderby ) ); @@ -2517,9 +2557,16 @@ public function get_posts() { } $orderby_array[] = $parsed . ' ' . $this->parse_order( $order ); - } - $orderby = implode( ', ', $orderby_array ); + // 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; + } + } } else { $query_vars['orderby'] = urldecode( $query_vars['orderby'] ); $query_vars['orderby'] = addslashes_gpc( $query_vars['orderby'] ); @@ -2531,16 +2578,24 @@ public function get_posts() { continue; } - $orderby_array[] = $parsed; - } - $orderby = implode( ' ' . $query_vars['order'] . ', ', $orderby_array ); + $orderby_array[] = $parsed . ' ' . $query_vars['order']; - if ( empty( $orderby ) ) { - $orderby = "{$wpdb->posts}.post_date " . $query_vars['order']; - } elseif ( ! empty( $query_vars['order'] ) ) { - $orderby .= " {$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; + } } } + + // 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 ) ); + } } // Order search results by relevance only when another "orderby" is not specified in the query. @@ -3153,15 +3208,34 @@ 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'] ?? ''; - $orderby = $clauses['orderby'] ?? ''; + $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'] ?? ''; $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; } @@ -3255,10 +3329,21 @@ 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 ); + $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; + if ( null === $this->posts ) { $cached_results = wp_cache_get_salted( $cache_key, 'post-queries', $last_changed ); @@ -5042,9 +5127,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. + /* + * 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'; + $args['orderby'] = 'date, ID'; + } elseif ( 'date' === $args['orderby'] ) { + $args['orderby'] = 'date, ID'; } $placeholder = $wpdb->placeholder_escape(); diff --git a/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php b/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php index 66e3e02501cfb..1fc58f88e2059 100644 --- a/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php +++ b/tests/phpunit/tests/admin/wpPrivacyRequestsTable.php @@ -99,7 +99,15 @@ 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 + ); + + $this->assertStringContainsString( 'ORDER BY ' . implode( ', ', $expected_query ), $this->sql ); } /** @@ -136,42 +144,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( @@ -185,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..a2d8d943f17ba --- /dev/null +++ b/tests/phpunit/tests/query/deterministicOrdering.php @@ -0,0 +1,855 @@ + 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++ ) { + 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 + ) + ); + } + + // 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, + ) + ); + } + } + + /** + * 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(); + self::$search_relevance_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, + 'paged' => 1, + ) + ); + + // 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, + '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( + '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. + * + * @ticket xxxxx + */ + public function test_deterministic_ordering_with_post_title() { + // 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, + 'paged' => 1, + ) + ); + + // 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. + * + * @ticket xxxxx + */ + 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( + array( + 'post_type' => 'wptests_time_ident', + 'post__in' => self::$date_identical_post_ids, + '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' ); + } + + /** + * Test that deterministic ordering works with array orderby. + * + * @ticket xxxxx + */ + 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, + ) + ); + + $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, + ) + ); + + $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. + * + * @ticket xxxxx + */ + 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, + ) + ); + + // Should not add duplicate ID ordering + $this->assertStringContainsString( 'ID ASC', $query->request ); + $this->assertStringNotContainsString( 'ID ASC, ID ASC', $query->request ); + } + + /** + * Test that deterministic ordering works with search queries. + * + * @ticket xxxxx + */ + 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, + ) + ); + + $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. + * + * @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' ); + } + + /** + * 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' ); + } + + /** + * 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' ); + } + + /** + * 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' ); + } +} diff --git a/tests/phpunit/tests/rest-api/rest-posts-controller.php b/tests/phpunit/tests/rest-api/rest-posts-controller.php index d701d12f9dd68..36d05afad67ce 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() {