Skip to content

Commit 5d06442

Browse files
ramonjdramonjd
committed
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.
1 parent 58a2e32 commit 5d06442

File tree

3 files changed

+301
-4
lines changed

3 files changed

+301
-4
lines changed

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2509,7 +2509,10 @@ public function get_posts() {
25092509
*/
25102510
$orderby = "{$wpdb->posts}.post_date " . $query_vars['order'] . ', ' . "{$wpdb->posts}.ID " . $query_vars['order'];
25112511
}
2512-
} elseif ( 'none' === $query_vars['orderby'] ) {
2512+
// See get_pages(): when sort_column is 'none', the get_pages() function should not generate any ORDER BY clause.
2513+
// Should it rather be handled in the get_pages() function?
2514+
// src/wp-includes/post.php L6496
2515+
} elseif ( 'none' === $query_vars['orderby'] || ( is_array( $query_vars['orderby'] ) && array_key_exists( 'none', $query_vars['orderby'] ) ) ) {
25132516
$orderby = '';
25142517
} else {
25152518
/*
@@ -2539,6 +2542,7 @@ public function get_posts() {
25392542
$orderby_array = array();
25402543
$needs_deterministic_orderby = false;
25412544
$has_id_orderby = false;
2545+
$id_tie_breaker_order = $query_vars['order']; // Default to global order
25422546

25432547
if ( is_array( $query_vars['orderby'] ) ) {
25442548
foreach ( $query_vars['orderby'] as $_orderby => $order ) {
@@ -2554,6 +2558,8 @@ public function get_posts() {
25542558
// Check if this field needs deterministic ordering
25552559
if ( in_array( $_orderby, $fields_requiring_deterministic_orderby, true ) ) {
25562560
$needs_deterministic_orderby = true;
2561+
// Use the order from the array for ID tie-breaker
2562+
$id_tie_breaker_order = $this->parse_order( $order );
25572563
} elseif ( 'ID' === $_orderby ) {
25582564
$has_id_orderby = true;
25592565
}
@@ -2582,7 +2588,7 @@ public function get_posts() {
25822588

25832589
// Add ID as tie-breaker if needed and not already present
25842590
if ( $needs_deterministic_orderby && ! $has_id_orderby ) {
2585-
$orderby_array[] = "{$wpdb->posts}.ID " . $query_vars['order'];
2591+
$orderby_array[] = "{$wpdb->posts}.ID " . $id_tie_breaker_order;
25862592
}
25872593

25882594
// Build the final orderby string

‎tests/phpunit/tests/admin/wpPrivacyRequestsTable.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,14 +193,14 @@ public function data_columns_should_be_sortable() {
193193
'order' => 'ASC',
194194
'orderby' => 'requester',
195195
's' => 'foo',
196-
'expected' => 'post_title ASC',
196+
'expected' => 'post_title ASC, ID ASC',
197197
),
198198
// Search and order by requested (post_date) ASC.
199199
array(
200200
'order' => 'ASC',
201201
'orderby' => 'requested',
202202
's' => 'foo',
203-
'expected' => 'post_date ASC',
203+
'expected' => 'post_date ASC, ID ASC',
204204
),
205205
);
206206
}

‎tests/phpunit/tests/query/deterministicOrdering.php‎

Lines changed: 291 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,291 @@
1+
<?php
2+
/**
3+
* Test deterministic ordering functionality in WP_Query.
4+
*
5+
* @package WordPress\UnitTests
6+
*
7+
* @group query
8+
* @group ordering
9+
* @ticket 44349
10+
*/
11+
class Tests_Query_DeterministicOrdering extends WP_UnitTestCase {
12+
13+
/**
14+
* Test that deterministic ordering adds ID as tie-breaker for fields that can have duplicates.
15+
*
16+
* @ticket 44349
17+
*/
18+
public function test_deterministic_ordering_adds_id_tie_breaker() {
19+
global $wpdb;
20+
21+
// Create posts with same post_date to test deterministic ordering
22+
$post1 = self::factory()->post->create( array(
23+
'post_title' => 'Post A',
24+
'post_date' => '2023-01-01 10:00:00',
25+
) );
26+
$post2 = self::factory()->post->create( array(
27+
'post_title' => 'Post B',
28+
'post_date' => '2023-01-01 10:00:00', // Same date as post1
29+
) );
30+
$post3 = self::factory()->post->create( array(
31+
'post_title' => 'Post C',
32+
'post_date' => '2023-01-01 10:00:00', // Same date as post1 and post2
33+
) );
34+
35+
// Test ordering by post_date (should add ID tie-breaker)
36+
$query = new WP_Query( array(
37+
'orderby' => 'post_date',
38+
'order' => 'ASC',
39+
'posts_per_page' => 10,
40+
) );
41+
42+
// Verify SQL contains ID as secondary sort
43+
$this->assertStringContainsString( 'ORDER BY', $query->request );
44+
$this->assertStringContainsString( 'post_date ASC', $query->request );
45+
$this->assertStringContainsString( 'ID ASC', $query->request );
46+
$this->assertStringNotContainsString( 'ASC ASC', $query->request ); // No double ASC
47+
}
48+
49+
/**
50+
* Test that deterministic ordering works with post_title.
51+
*
52+
* @ticket 44349
53+
*/
54+
public function test_deterministic_ordering_with_post_title() {
55+
// Create posts with same title to test deterministic ordering
56+
$post1 = self::factory()->post->create( array(
57+
'post_title' => 'Same Title',
58+
'post_date' => '2023-01-01 10:00:00',
59+
) );
60+
$post2 = self::factory()->post->create( array(
61+
'post_title' => 'Same Title', // Same title as post1
62+
'post_date' => '2023-01-01 11:00:00',
63+
) );
64+
65+
$query = new WP_Query( array(
66+
'orderby' => 'post_title',
67+
'order' => 'ASC',
68+
'posts_per_page' => 10,
69+
) );
70+
71+
// Verify SQL contains ID as secondary sort
72+
$this->assertStringContainsString( 'post_title ASC', $query->request );
73+
$this->assertStringContainsString( 'ID ASC', $query->request );
74+
$this->assertStringNotContainsString( 'ASC ASC', $query->request );
75+
}
76+
77+
/**
78+
* Test that deterministic ordering works with DESC order.
79+
*
80+
* @ticket 44349
81+
*/
82+
public function test_deterministic_ordering_with_desc_order() {
83+
$query = new WP_Query( array(
84+
'orderby' => 'post_date',
85+
'order' => 'DESC',
86+
'posts_per_page' => 10,
87+
) );
88+
89+
// Verify SQL contains ID as secondary sort with DESC
90+
$this->assertStringContainsString( 'post_date DESC', $query->request );
91+
$this->assertStringContainsString( 'ID DESC', $query->request );
92+
$this->assertStringNotContainsString( 'DESC DESC', $query->request );
93+
}
94+
95+
/**
96+
* Test that deterministic ordering works with array orderby.
97+
*
98+
* @ticket 44349
99+
*/
100+
public function test_deterministic_ordering_with_array_orderby() {
101+
$query = new WP_Query( array(
102+
'orderby' => array(
103+
'post_date' => 'ASC',
104+
'post_title' => 'ASC',
105+
),
106+
'posts_per_page' => 10,
107+
) );
108+
109+
// Verify SQL contains both fields with directions
110+
$this->assertStringContainsString( 'post_date ASC', $query->request );
111+
$this->assertStringContainsString( 'post_title ASC', $query->request );
112+
$this->assertStringContainsString( 'ID ASC', $query->request );
113+
$this->assertStringNotContainsString( 'ASC ASC', $query->request );
114+
}
115+
116+
/**
117+
* Test that deterministic ordering doesn't add ID when ID is already present.
118+
*
119+
* @ticket 44349
120+
*/
121+
public function test_deterministic_ordering_does_not_duplicate_id() {
122+
$query = new WP_Query( array(
123+
'orderby' => 'ID',
124+
'order' => 'ASC',
125+
'posts_per_page' => 10,
126+
) );
127+
128+
// Should not add duplicate ID
129+
$this->assertStringContainsString( 'ID ASC', $query->request );
130+
$this->assertStringNotContainsString( 'ID ASC, ID ASC', $query->request );
131+
}
132+
133+
/**
134+
* Test that deterministic ordering works with fields that don't need it.
135+
*
136+
* @ticket 44349
137+
*/
138+
public function test_deterministic_ordering_with_non_deterministic_fields() {
139+
$query = new WP_Query( array(
140+
'orderby' => 'rand',
141+
'posts_per_page' => 10,
142+
) );
143+
144+
// Should not add ID tie-breaker for rand
145+
$this->assertStringContainsString( 'RAND()', $query->request );
146+
$this->assertStringNotContainsString( 'ID ASC', $query->request );
147+
}
148+
149+
/**
150+
* Test that deterministic ordering works with default ordering.
151+
*
152+
* @ticket 44349
153+
*/
154+
public function test_deterministic_ordering_with_default_ordering() {
155+
$query = new WP_Query( array(
156+
'posts_per_page' => 10,
157+
) );
158+
159+
// Default ordering should include ID tie-breaker
160+
$this->assertStringContainsString( 'post_date DESC', $query->request );
161+
$this->assertStringContainsString( 'ID DESC', $query->request );
162+
$this->assertStringNotContainsString( 'DESC DESC', $query->request );
163+
}
164+
165+
/**
166+
* Test that deterministic ordering prevents duplicate records across pages.
167+
*
168+
* @ticket 44349
169+
*/
170+
public function test_deterministic_ordering_prevents_duplicates_across_pages() {
171+
// Create multiple posts with same post_date
172+
$posts = array();
173+
for ( $i = 1; $i <= 10; $i++ ) {
174+
$posts[] = self::factory()->post->create( array(
175+
'post_title' => "Post $i",
176+
'post_date' => '2023-01-01 10:00:00', // All same date
177+
) );
178+
}
179+
180+
// Get first page
181+
$query1 = new WP_Query( array(
182+
'orderby' => 'post_date',
183+
'order' => 'ASC',
184+
'posts_per_page' => 5,
185+
'paged' => 1,
186+
) );
187+
188+
// Get second page
189+
$query2 = new WP_Query( array(
190+
'orderby' => 'post_date',
191+
'order' => 'ASC',
192+
'posts_per_page' => 5,
193+
'paged' => 2,
194+
) );
195+
196+
$page1_ids = wp_list_pluck( $query1->posts, 'ID' );
197+
$page2_ids = wp_list_pluck( $query2->posts, 'ID' );
198+
199+
// No overlap between pages
200+
$this->assertEmpty( array_intersect( $page1_ids, $page2_ids ) );
201+
202+
// Total posts should equal sum of both pages
203+
$this->assertEquals( 10, $query1->found_posts );
204+
$this->assertEquals( 5, count( $page1_ids ) );
205+
$this->assertEquals( 5, count( $page2_ids ) );
206+
}
207+
208+
/**
209+
* Test that deterministic ordering works with search queries.
210+
*
211+
* @ticket 44349
212+
*/
213+
public function test_deterministic_ordering_with_search() {
214+
// Create posts with searchable content
215+
$post1 = self::factory()->post->create( array(
216+
'post_title' => 'Test Post 1',
217+
'post_content' => 'This is a test post',
218+
'post_date' => '2023-01-01 10:00:00',
219+
) );
220+
$post2 = self::factory()->post->create( array(
221+
'post_title' => 'Test Post 2',
222+
'post_content' => 'This is another test post',
223+
'post_date' => '2023-01-01 10:00:00', // Same date
224+
) );
225+
226+
$query = new WP_Query( array(
227+
's' => 'test',
228+
'orderby' => 'post_date',
229+
'order' => 'ASC',
230+
'posts_per_page' => 10,
231+
) );
232+
233+
// Should still have deterministic ordering even with search
234+
$this->assertStringContainsString( 'post_date ASC', $query->request );
235+
$this->assertStringContainsString( 'ID ASC', $query->request );
236+
$this->assertStringNotContainsString( 'ASC ASC', $query->request );
237+
}
238+
239+
/**
240+
* Test that deterministic ordering works with meta queries.
241+
*
242+
* @ticket 44349
243+
*/
244+
public function test_deterministic_ordering_with_meta_query() {
245+
// Create posts with meta values
246+
$post1 = self::factory()->post->create();
247+
add_post_meta( $post1, 'test_meta', 'value1' );
248+
249+
$post2 = self::factory()->post->create();
250+
add_post_meta( $post2, 'test_meta', 'value2' );
251+
252+
$query = new WP_Query( array(
253+
'meta_key' => 'test_meta',
254+
'orderby' => 'post_date',
255+
'order' => 'ASC',
256+
'posts_per_page' => 10,
257+
) );
258+
259+
// Should still have deterministic ordering with meta queries
260+
$this->assertStringContainsString( 'post_date ASC', $query->request );
261+
$this->assertStringContainsString( 'ID ASC', $query->request );
262+
$this->assertStringNotContainsString( 'ASC ASC', $query->request );
263+
}
264+
265+
/**
266+
* Test that deterministic ordering works with taxonomy queries.
267+
*
268+
* @ticket 44349
269+
*/
270+
public function test_deterministic_ordering_with_taxonomy_query() {
271+
// Create posts with categories
272+
$post1 = self::factory()->post->create();
273+
$post2 = self::factory()->post->create();
274+
275+
$cat_id = self::factory()->category->create( array( 'name' => 'Test Category' ) );
276+
wp_set_post_categories( $post1, array( $cat_id ) );
277+
wp_set_post_categories( $post2, array( $cat_id ) );
278+
279+
$query = new WP_Query( array(
280+
'category_name' => 'test-category',
281+
'orderby' => 'post_date',
282+
'order' => 'ASC',
283+
'posts_per_page' => 10,
284+
) );
285+
286+
// Should still have deterministic ordering with taxonomy queries
287+
$this->assertStringContainsString( 'post_date ASC', $query->request );
288+
$this->assertStringContainsString( 'ID ASC', $query->request );
289+
$this->assertStringNotContainsString( 'ASC ASC', $query->request );
290+
}
291+
}

0 commit comments

Comments
 (0)