Skip to content

Commit 3a6cca9

Browse files
spacedmonkeyspacedmonkey
committed
Query: Improve cache key generation.
Query caching in `WP_Query` was added in [53941]. When this functionality was added to the `WP_Query` class, a number of edge cases were missed that would result in redundant duplicate queries. It was possible to pass parameters to `WP_Query` that would be different but would result in the same database query being generated. As the cache key is generated from a mixture of query arguments and the SQL query, this resulted in different cache keys for the same database query, resulting in unnecessary duplicate queries. In this change, the logic in the `generate_cache_key` method has been improved to ensure reuse of existing caches. The following edge cases have been considered: - Passing `post_type` as an empty string. - Passing `post_type` as the string `any`. - Passing `post_type` as array vs. string. - Passing `post_type` as an array in a different order. - Not passing `orderby`. - Passing `post_status` as an array vs. string. - Passing `post_status` as an array in a different order. This change also fixes an issue where the old SQL query would not match, as the queries had different whitespaces. Props spacedmonkey, joemcgill, pbearne, peterwilsoncc, rajinsharwar, mukesh27, thekt12, huzaifaalmesbah, rodionov201. Fixes #59442. git-svn-id: https://develop.svn.wordpress.org/trunk@58122 602fd350-edb4-49c9-b593-d223f7449a82
1 parent 5b9cdef commit 3a6cca9

2 files changed

Lines changed: 187 additions & 7 deletions

File tree

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

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2264,6 +2264,9 @@ public function get_posts() {
22642264
$post_type = 'any';
22652265
} elseif ( count( $post_type ) === 1 ) {
22662266
$post_type = $post_type[0];
2267+
} else {
2268+
// Sort post types to ensure same cache key generation.
2269+
sort( $post_type );
22672270
}
22682271

22692272
$post_status_join = true;
@@ -2542,6 +2545,8 @@ public function get_posts() {
25422545
$post_type_where = " AND {$wpdb->posts}.post_type IN ('" . implode( "', '", array_map( 'esc_sql', $in_search_post_types ) ) . "')";
25432546
}
25442547
} elseif ( ! empty( $post_type ) && is_array( $post_type ) ) {
2548+
// Sort post types to ensure same cache key generation.
2549+
sort( $post_type );
25452550
$post_type_where = " AND {$wpdb->posts}.post_type IN ('" . implode( "', '", esc_sql( $post_type ) ) . "')";
25462551
} elseif ( ! empty( $post_type ) ) {
25472552
$post_type_where = $wpdb->prepare( " AND {$wpdb->posts}.post_type = %s", $post_type );
@@ -2647,7 +2652,7 @@ public function get_posts() {
26472652
}
26482653

26492654
if ( ! empty( $queried_post_types ) ) {
2650-
2655+
sort( $queried_post_types );
26512656
$status_type_clauses = array();
26522657

26532658
foreach ( $queried_post_types as $queried_post_type ) {
@@ -3104,14 +3109,24 @@ public function get_posts() {
31043109
$found_rows = 'SQL_CALC_FOUND_ROWS';
31053110
}
31063111

3107-
// Beginning of the string is on a new line to prevent leading whitespace. See https://core.trac.wordpress.org/ticket/56841.
3112+
/*
3113+
* Beginning of the string is on a new line to prevent leading whitespace.
3114+
*
3115+
* The additional indentation of subsequent lines is to ensure the SQL
3116+
* queries are identical to those generated when splitting queries. This
3117+
* improves caching of the query by ensuring the same cache key is
3118+
* generated for the same database queries functionally.
3119+
*
3120+
* See https://core.trac.wordpress.org/ticket/56841.
3121+
* See https://github.com/WordPress/wordpress-develop/pull/6393#issuecomment-2088217429
3122+
*/
31083123
$old_request =
31093124
"SELECT $found_rows $distinct $fields
3110-
FROM {$wpdb->posts} $join
3111-
WHERE 1=1 $where
3112-
$groupby
3113-
$orderby
3114-
$limits";
3125+
FROM {$wpdb->posts} $join
3126+
WHERE 1=1 $where
3127+
$groupby
3128+
$orderby
3129+
$limits";
31153130

31163131
$this->request = $old_request;
31173132

@@ -4856,6 +4871,32 @@ protected function generate_cache_key( array $args, $sql ) {
48564871
$args['suppress_filters']
48574872
);
48584873

4874+
if ( empty( $args['post_type'] ) ) {
4875+
if ( $this->is_attachment ) {
4876+
$args['post_type'] = 'attachment';
4877+
} elseif ( $this->is_page ) {
4878+
$args['post_type'] = 'page';
4879+
} else {
4880+
$args['post_type'] = 'post';
4881+
}
4882+
} elseif ( 'any' === $args['post_type'] ) {
4883+
$args['post_type'] = array_values( get_post_types( array( 'exclude_from_search' => false ) ) );
4884+
}
4885+
$args['post_type'] = (array) $args['post_type'];
4886+
// Sort post types to ensure same cache key generation.
4887+
sort( $args['post_type'] );
4888+
4889+
if ( isset( $args['post_status'] ) ) {
4890+
$args['post_status'] = (array) $args['post_status'];
4891+
// Sort post status to ensure same cache key generation.
4892+
sort( $args['post_status'] );
4893+
}
4894+
4895+
// Add a default orderby value of date to ensure same cache key generation.
4896+
if ( ! isset( $q['orderby'] ) ) {
4897+
$args['orderby'] = 'date';
4898+
}
4899+
48594900
$placeholder = $wpdb->placeholder_escape();
48604901
array_walk_recursive(
48614902
$args,
@@ -4874,6 +4915,8 @@ static function ( &$value ) use ( $wpdb, $placeholder ) {
48744915
}
48754916
);
48764917

4918+
ksort( $args );
4919+
48774920
// Replace wpdb placeholder in the SQL statement used by the cache key.
48784921
$sql = $wpdb->remove_placeholder_escape( $sql );
48794922
$key = md5( serialize( $args ) . $sql );

‎tests/phpunit/tests/query/cacheResults.php‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,63 @@ public function test_generate_cache_key_placeholder() {
171171
$this->assertSame( $cache_key_1, $cache_key_2, 'Cache key differs when using wpdb placeholder.' );
172172
}
173173

174+
/**
175+
* @covers WP_Query::generate_cache_key
176+
* @ticket 59442
177+
*/
178+
public function test_generate_cache_key_unregister_post_type() {
179+
global $wpdb;
180+
register_post_type(
181+
'wptests_pt',
182+
array(
183+
'exclude_from_search' => false,
184+
)
185+
);
186+
$query_vars = array(
187+
'post_type' => 'any',
188+
);
189+
$fields = "{$wpdb->posts}.ID";
190+
$query1 = new WP_Query( $query_vars );
191+
$request1 = str_replace( $fields, "{$wpdb->posts}.*", $query1->request );
192+
193+
$reflection = new ReflectionMethod( $query1, 'generate_cache_key' );
194+
$reflection->setAccessible( true );
195+
196+
$cache_key_1 = $reflection->invoke( $query1, $query_vars, $request1 );
197+
unregister_post_type( 'wptests_pt' );
198+
$cache_key_2 = $reflection->invoke( $query1, $query_vars, $request1 );
199+
200+
$this->assertNotSame( $cache_key_1, $cache_key_2, 'Cache key should differ after unregistering post type.' );
201+
}
202+
203+
/**
204+
* @ticket 59442
205+
*
206+
* @covers WP_Query::generate_cache_key
207+
*
208+
* @dataProvider data_query_cache_duplicate
209+
*/
210+
public function test_generate_cache_key_normalize( $query_vars1, $query_vars2 ) {
211+
global $wpdb;
212+
213+
$fields = "{$wpdb->posts}.ID";
214+
$query1 = new WP_Query( $query_vars1 );
215+
$request1 = str_replace( $fields, "{$wpdb->posts}.*", $query1->request );
216+
217+
$query2 = new WP_Query( $query_vars2 );
218+
$request2 = str_replace( $fields, "{$wpdb->posts}.*", $query2->request );
219+
220+
$reflection = new ReflectionMethod( $query1, 'generate_cache_key' );
221+
$reflection->setAccessible( true );
222+
223+
$this->assertSame( $request1, $request2, 'Queries should match' );
224+
225+
$cache_key_1 = $reflection->invoke( $query1, $query_vars1, $request1 );
226+
$cache_key_2 = $reflection->invoke( $query1, $query_vars2, $request2 );
227+
228+
$this->assertSame( $cache_key_1, $cache_key_2, 'Cache key differs the same paramters.' );
229+
}
230+
174231
/**
175232
* @dataProvider data_query_cache
176233
* @ticket 22176
@@ -216,6 +273,74 @@ public function test_query_cache( $args ) {
216273
}
217274
}
218275

276+
/**
277+
* Data provider for test_generate_cache_key_normalize().
278+
*
279+
* @return array[]
280+
*/
281+
public function data_query_cache_duplicate() {
282+
return array(
283+
'post type empty' => array(
284+
'query_vars1' => array( 'post_type' => '' ),
285+
'query_vars2' => array( 'post_type' => 'post' ),
286+
),
287+
'post type array' => array(
288+
'query_vars1' => array( 'post_type' => array( 'page' ) ),
289+
'query_vars2' => array( 'post_type' => 'page' ),
290+
),
291+
'orderby empty' => array(
292+
'query_vars1' => array( 'orderby' => null ),
293+
'query_vars2' => array( 'orderby' => 'date' ),
294+
),
295+
'different order parameter' => array(
296+
'query_vars1' => array(
297+
'post_type' => 'post',
298+
'posts_per_page' => 15,
299+
),
300+
'query_vars2' => array(
301+
'posts_per_page' => 15,
302+
'post_type' => 'post',
303+
),
304+
),
305+
'same args' => array(
306+
'query_vars1' => array( 'post_type' => 'post' ),
307+
'query_vars2' => array( 'post_type' => 'post' ),
308+
),
309+
'same args any' => array(
310+
'query_vars1' => array( 'post_type' => 'any' ),
311+
'query_vars2' => array( 'post_type' => 'any' ),
312+
),
313+
'any and post types' => array(
314+
'query_vars1' => array( 'post_type' => 'any' ),
315+
'query_vars2' => array( 'post_type' => array( 'post', 'page', 'attachment' ) ),
316+
),
317+
'different order post type' => array(
318+
'query_vars1' => array( 'post_type' => array( 'post', 'page' ) ),
319+
'query_vars2' => array( 'post_type' => array( 'page', 'post' ) ),
320+
),
321+
'post status array' => array(
322+
'query_vars1' => array( 'post_status' => 'publish' ),
323+
'query_vars2' => array( 'post_status' => array( 'publish' ) ),
324+
),
325+
'post status order' => array(
326+
'query_vars1' => array( 'post_status' => array( 'draft', 'publish' ) ),
327+
'query_vars2' => array( 'post_status' => array( 'publish', 'draft' ) ),
328+
),
329+
'cache parameters' => array(
330+
'query_vars1' => array(
331+
'update_post_meta_cache' => true,
332+
'update_post_term_cache' => true,
333+
'update_menu_item_cache' => true,
334+
),
335+
'query_vars2' => array(
336+
'update_post_meta_cache' => false,
337+
'update_post_term_cache' => false,
338+
'update_menu_item_cache' => false,
339+
),
340+
),
341+
);
342+
}
343+
219344
/**
220345
* Data provider.
221346
*
@@ -263,6 +388,18 @@ public function data_query_cache() {
263388
'post_type' => 'page',
264389
),
265390
),
391+
'cache true and empty post type' => array(
392+
'args' => array(
393+
'cache_results' => true,
394+
'post_type' => '',
395+
),
396+
),
397+
'cache true and orderby null' => array(
398+
'args' => array(
399+
'cache_results' => true,
400+
'orderby' => null,
401+
),
402+
),
266403
'cache true and ids' => array(
267404
'args' => array(
268405
'cache_results' => true,

0 commit comments

Comments
 (0)