[wp-trac] [WordPress Trac] #59492: WP_Query::generate_cache_key can create different hash for same queries leading to cache miss.

[WordPress Trac]

#59492: WP_Query::generate_cache_key can create different hash for same queries
leading to cache miss.
--------------------------+-----------------------------
 Reporter:  thekt12       |      Owner:  thekt12
     Type:  defect (bug)  |     Status:  assigned
 Priority:  normal        |  Milestone:  Awaiting Review
Component:  Database      |    Version:
 Severity:  normal        |   Keywords:  has-patch
  Focuses:  performance   |
--------------------------+-----------------------------
 This issue first came to light while investigating #59442. In #59442, two
 queries where found to have have a different cache key leading to a cache
 miss.

 === Another, example is shown below:

 {{{
 // add this to any test plugin
 add_action( 'init', 'test_wp_query_changes' );
 function test_wp_query_changes() {
         // post_type set to post.
         $query1 = new WP_Query(
                 array(
                         'cache_results' => true,
                         'fields'        => 'ids',
                         'post_type'     => [ 'post', 'page', 'attachment'
 ],
                 )
         );
         print_r("SQL ->". $query1->request );
         echo "<br>";

         // post_type set to post.
         $query2 = new WP_Query(
                 array(
                                 'cache_results' => true,
                                 'fields'        => 'ids',
                                 'post_type'     => 'any',
                         )
         );
         print_r("SQL ->". $query2->request );
         die();
 }
 }}}

 // I also added the following line below (https://github.com/WordPress
 /wordpress-develop/blob/e3a612a97c4c45acd7613779a46d86860e76cc65/src/wp-
 includes/class-wp-query.php#L3175):

 {{{
 print_r("Cache key formed - {$cache_key} <br>");
 }}}

 **Output was:**

 {{{
 Cache key formed -wp_query:87aeb9c56ee3545b729fe38b1becabd9:0.38106200
 1695924120

 SQL -> SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts WHERE 1=1 AND
 ((wp_posts.post_type = 'post' AND (wp_posts.post_status = 'publish' OR
 wp_posts.post_status = 'private')) OR (wp_posts.post_type = 'page' AND
 (wp_posts.post_status = 'publish' OR wp_posts.post_status = 'private')) OR
 (wp_posts.post_type = 'attachment' AND (wp_posts.post_status = 'publish'
 OR wp_posts.post_status = 'private'))) ORDER BY wp_posts.post_date DESC
 LIMIT 0, 10

 Cache key formed -wp_query:1e07ead1e7cc64aee3597dc16472f3bb:0.38106200
 1695924120

 SQL -> SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts WHERE 1=1 AND
 ((wp_posts.post_type = 'post' AND (wp_posts.post_status = 'publish' OR
 wp_posts.post_status = 'private')) OR (wp_posts.post_type = 'page' AND
 (wp_posts.post_status = 'publish' OR wp_posts.post_status = 'private')) OR
 (wp_posts.post_type = 'attachment' AND (wp_posts.post_status = 'publish'
 OR wp_posts.post_status = 'private'))) ORDER BY wp_posts.post_date DESC
 LIMIT 0, 10
 }}}

 SQL Queries obtained for both the arguments **were similar**, however the
 **cache_key** obtained were different leading to cache miss.


 == The problem:
 [https://github.com/WordPress/wordpress-
 develop/blob/e3a612a97c4c45acd7613779a46d86860e76cc65/src/wp-includes
 /class-wp-query.php#L4837 generate_cache_key( array $args, $sql )] needs
 args to replace wpdb placeholders with the string used in the database.

 However, the problem arise when the same args is serialized and used to
 create key ->

 [https://github.com/WordPress/wordpress-
 develop/blob/e3a612a97c4c45acd7613779a46d86860e76cc65/src/wp-includes
 /class-wp-query.php#L4870 $key = md5( serialize( $args ) . $sql );]


 === Proposal
 `$args` parameter in `generate_cache_key( array $args, $sql )` be only
 used to replace placeholder and not be seralized for creating hash.
 Hashing key is generated towards the end where the SQL cannot be further
 modified, so it is indeed reliable.

 Creating hash only with `$sql` will help improve performace by increasing
 the changes of hit. Another benefit is it  improves performance by not
 serializing large `$args`. `generate_cache_key` get called for almost
 WP_Query as caching is the [https://github.com/WordPress/wordpress-
 develop/blob/e3a612a97c4c45acd7613779a46d86860e76cc65/src/wp-includes
 /class-wp-query.php#L660C94-L660C94 default] nature.



 Same with `WP_Term_Query::generate_cache_key( array $args, $sql )`

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/59492>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform