Skip to content

Commit 864399f

Browse files
committed
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.
1 parent 83efab1 commit 864399f

File tree

2 files changed

+84
-24
lines changed

2 files changed

+84
-24
lines changed

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

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2519,24 +2519,14 @@ public function get_posts() {
25192519
* Ensure deterministic ordering to prevent duplicate records across pages.
25202520
* When multiple posts have the same value for a field, add ID as secondary sort to guarantee consistent ordering.
25212521
* Note: this is to circumvent a bug that is currently being tracked in https://core.trac.wordpress.org/ticket/44349.
2522+
*
2523+
* Use a blacklist approach: add ID as tie-breaker for all orderby fields except those that are
2524+
* already deterministic (ID itself, random ordering, or search relevance).
25222525
*/
2523-
$fields_requiring_deterministic_orderby = array(
2524-
'post_name',
2525-
'post_author',
2526-
'post_date',
2527-
'post_title',
2528-
'post_modified',
2529-
'post_parent',
2530-
'post_type',
2531-
'name',
2532-
'author',
2533-
'date',
2534-
'title',
2535-
'modified',
2536-
'parent',
2537-
'type',
2538-
'menu_order',
2539-
'comment_count',
2526+
$fields_excluding_deterministic_orderby = array(
2527+
'ID',
2528+
'rand',
2529+
'relevance',
25402530
);
25412531

25422532
$orderby_array = array();
@@ -2555,10 +2545,10 @@ public function get_posts() {
25552545

25562546
$orderby_array[] = $parsed . ' ' . $this->parse_order( $order );
25572547

2558-
// Check if this field needs deterministic ordering
2559-
if ( in_array( $_orderby, $fields_requiring_deterministic_orderby, true ) ) {
2548+
// Check if this field should have deterministic ordering (not in blacklist).
2549+
if ( ! in_array( $_orderby, $fields_excluding_deterministic_orderby, true ) ) {
25602550
$needs_deterministic_orderby = true;
2561-
// Use the order from the array for ID tie-breaker
2551+
// Use the order from the array for ID tie-breaker.
25622552
$id_tie_breaker_order = $this->parse_order( $order );
25632553
} elseif ( 'ID' === $_orderby ) {
25642554
$has_id_orderby = true;
@@ -2577,21 +2567,21 @@ public function get_posts() {
25772567

25782568
$orderby_array[] = $parsed . ' ' . $query_vars['order'];
25792569

2580-
// Check if this field needs deterministic ordering
2581-
if ( in_array( $orderby, $fields_requiring_deterministic_orderby, true ) ) {
2570+
// Check if this field should have deterministic ordering (not in blacklist).
2571+
if ( ! in_array( $orderby, $fields_excluding_deterministic_orderby, true ) ) {
25822572
$needs_deterministic_orderby = true;
25832573
} elseif ( 'ID' === $orderby ) {
25842574
$has_id_orderby = true;
25852575
}
25862576
}
25872577
}
25882578

2589-
// Add ID as tie-breaker if needed and not already present
2579+
// Add ID as tie-breaker if needed and not already present.
25902580
if ( $needs_deterministic_orderby && ! $has_id_orderby ) {
25912581
$orderby_array[] = "{$wpdb->posts}.ID " . $id_tie_breaker_order;
25922582
}
25932583

2594-
// Build the final orderby string
2584+
// Build the final orderby string.
25952585
if ( empty( $orderby_array ) ) {
25962586
$orderby = "{$wpdb->posts}.post_date " . $query_vars['order'] . ', ' . "{$wpdb->posts}.ID " . $query_vars['order'];
25972587
} else {

tests/phpunit/tests/query/deterministicOrdering.php

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,4 +427,74 @@ public function test_deterministic_ordering_with_menu_order() {
427427

428428
$this->assertEquals( $page1_ids, $page1_repeat_ids, 'Same query should return same results when ordering by menu_order' );
429429
}
430+
431+
/**
432+
* Test that deterministic ordering works with metadata ordering.
433+
*
434+
* @ticket xxxxx
435+
*/
436+
public function test_deterministic_ordering_with_metadata() {
437+
$post_ids = array();
438+
439+
// Create posts with identical meta values to trigger the bug
440+
$identical_meta_value = 'same_price';
441+
for ( $i = 1; $i <= 20; $i++ ) {
442+
$post_id = self::factory()->post->create(
443+
array(
444+
'post_type' => 'wptests_time_ident',
445+
'post_title' => "Post $i",
446+
)
447+
);
448+
add_post_meta( $post_id, 'price', $identical_meta_value );
449+
$post_ids[] = $post_id;
450+
}
451+
452+
// Get first page ordering by metadata
453+
$query1 = new WP_Query(
454+
array(
455+
'post_type' => 'wptests_time_ident',
456+
'post__in' => $post_ids,
457+
'meta_query' => array(
458+
'price_key' => array(
459+
'key' => 'price',
460+
'compare' => 'EXISTS',
461+
),
462+
),
463+
'orderby' => 'price_key',
464+
'order' => 'ASC',
465+
'posts_per_page' => 10,
466+
'paged' => 1,
467+
)
468+
);
469+
470+
// Get second page ordering by metadata
471+
$query2 = new WP_Query(
472+
array(
473+
'post_type' => 'wptests_time_ident',
474+
'post__in' => $post_ids,
475+
'meta_query' => array(
476+
'price_key' => array(
477+
'key' => 'price',
478+
'compare' => 'EXISTS',
479+
),
480+
),
481+
'orderby' => 'price_key',
482+
'order' => 'ASC',
483+
'posts_per_page' => 10,
484+
'paged' => 2,
485+
)
486+
);
487+
488+
$page1_ids = wp_list_pluck( $query1->posts, 'ID' );
489+
$page2_ids = wp_list_pluck( $query2->posts, 'ID' );
490+
491+
// Verify no overlap between pages (no duplicates)
492+
$overlap = array_intersect( $page1_ids, $page2_ids );
493+
$this->assertEmpty( $overlap, 'Pages should not contain duplicate posts when ordering by metadata' );
494+
495+
// Verify total count is correct
496+
$this->assertEquals( 20, $query1->found_posts, 'Total posts should be 20' );
497+
$this->assertEquals( 10, count( $page1_ids ), 'First page should have 10 posts' );
498+
$this->assertEquals( 10, count( $page2_ids ), 'Second page should have 10 posts' );
499+
}
430500
}

0 commit comments

Comments
 (0)