<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[58122] trunk: Query: Improve cache key generation.</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { white-space: pre-line; overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta" style="font-size: 105%">
<dt style="float: left; width: 6em; font-weight: bold">Revision</dt> <dd><a style="font-weight: bold" href="https://core.trac.wordpress.org/changeset/58122">58122</a><script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","description":"Review this Commit","action":{"@type":"ViewAction","url":"https://core.trac.wordpress.org/changeset/58122","name":"Review Commit"}}</script></dd>
<dt style="float: left; width: 6em; font-weight: bold">Author</dt> <dd>spacedmonkey</dd>
<dt style="float: left; width: 6em; font-weight: bold">Date</dt> <dd>2024-05-08 22:49:46 +0000 (Wed, 08 May 2024)</dd>
</dl>
<pre style='padding-left: 1em; margin: 2em 0; border-left: 2px solid #ccc; line-height: 1.25; font-size: 105%; font-family: sans-serif'>Query: Improve cache key generation.
Query caching in `WP_Query` was added in <a href="https://core.trac.wordpress.org/changeset/53941">[53941]</a>. 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 <a href="https://core.trac.wordpress.org/ticket/59442">#59442</a>.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunksrcwpincludesclasswpqueryphp">trunk/src/wp-includes/class-wp-query.php</a></li>
<li><a href="#trunktestsphpunittestsquerycacheResultsphp">trunk/tests/phpunit/tests/query/cacheResults.php</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunksrcwpincludesclasswpqueryphp"></a>
<div class="modfile"><h4 style="background-color: #eee; color: inherit; margin: 1em 0; padding: 1.3em; font-size: 115%">Modified: trunk/src/wp-includes/class-wp-query.php</h4>
<pre class="diff"><span>
<span class="info" style="display: block; padding: 0 10px; color: #888">--- trunk/src/wp-includes/class-wp-query.php 2024-05-08 22:04:11 UTC (rev 58121)
+++ trunk/src/wp-includes/class-wp-query.php 2024-05-08 22:49:46 UTC (rev 58122)
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -2264,6 +2264,9 @@
</span><span class="cx" style="display: block; padding: 0 10px"> $post_type = 'any';
</span><span class="cx" style="display: block; padding: 0 10px"> } elseif ( count( $post_type ) === 1 ) {
</span><span class="cx" style="display: block; padding: 0 10px"> $post_type = $post_type[0];
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ } else {
+ // Sort post types to ensure same cache key generation.
+ sort( $post_type );
</ins><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> $post_status_join = true;
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -2542,6 +2545,8 @@
</span><span class="cx" style="display: block; padding: 0 10px"> $post_type_where = " AND {$wpdb->posts}.post_type IN ('" . implode( "', '", array_map( 'esc_sql', $in_search_post_types ) ) . "')";
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px"> } elseif ( ! empty( $post_type ) && is_array( $post_type ) ) {
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ // Sort post types to ensure same cache key generation.
+ sort( $post_type );
</ins><span class="cx" style="display: block; padding: 0 10px"> $post_type_where = " AND {$wpdb->posts}.post_type IN ('" . implode( "', '", esc_sql( $post_type ) ) . "')";
</span><span class="cx" style="display: block; padding: 0 10px"> } elseif ( ! empty( $post_type ) ) {
</span><span class="cx" style="display: block; padding: 0 10px"> $post_type_where = $wpdb->prepare( " AND {$wpdb->posts}.post_type = %s", $post_type );
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -2647,7 +2652,7 @@
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> if ( ! empty( $queried_post_types ) ) {
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">-
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ sort( $queried_post_types );
</ins><span class="cx" style="display: block; padding: 0 10px"> $status_type_clauses = array();
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> foreach ( $queried_post_types as $queried_post_type ) {
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -3104,14 +3109,24 @@
</span><span class="cx" style="display: block; padding: 0 10px"> $found_rows = 'SQL_CALC_FOUND_ROWS';
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- // Beginning of the string is on a new line to prevent leading whitespace. See https://core.trac.wordpress.org/ticket/56841.
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ /*
+ * Beginning of the string is on a new line to prevent leading whitespace.
+ *
+ * The additional indentation of subsequent lines is to ensure the SQL
+ * queries are identical to those generated when splitting queries. This
+ * improves caching of the query by ensuring the same cache key is
+ * generated for the same database queries functionally.
+ *
+ * See https://core.trac.wordpress.org/ticket/56841.
+ * See https://github.com/WordPress/wordpress-develop/pull/6393#issuecomment-2088217429
+ */
</ins><span class="cx" style="display: block; padding: 0 10px"> $old_request =
</span><span class="cx" style="display: block; padding: 0 10px"> "SELECT $found_rows $distinct $fields
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- FROM {$wpdb->posts} $join
- WHERE 1=1 $where
- $groupby
- $orderby
- $limits";
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ FROM {$wpdb->posts} $join
+ WHERE 1=1 $where
+ $groupby
+ $orderby
+ $limits";
</ins><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> $this->request = $old_request;
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -4856,6 +4871,32 @@
</span><span class="cx" style="display: block; padding: 0 10px"> $args['suppress_filters']
</span><span class="cx" style="display: block; padding: 0 10px"> );
</span><span class="cx" style="display: block; padding: 0 10px">
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ if ( empty( $args['post_type'] ) ) {
+ if ( $this->is_attachment ) {
+ $args['post_type'] = 'attachment';
+ } elseif ( $this->is_page ) {
+ $args['post_type'] = 'page';
+ } else {
+ $args['post_type'] = 'post';
+ }
+ } elseif ( 'any' === $args['post_type'] ) {
+ $args['post_type'] = array_values( get_post_types( array( 'exclude_from_search' => false ) ) );
+ }
+ $args['post_type'] = (array) $args['post_type'];
+ // Sort post types to ensure same cache key generation.
+ sort( $args['post_type'] );
+
+ if ( isset( $args['post_status'] ) ) {
+ $args['post_status'] = (array) $args['post_status'];
+ // Sort post status to ensure same cache key generation.
+ sort( $args['post_status'] );
+ }
+
+ // Add a default orderby value of date to ensure same cache key generation.
+ if ( ! isset( $q['orderby'] ) ) {
+ $args['orderby'] = 'date';
+ }
+
</ins><span class="cx" style="display: block; padding: 0 10px"> $placeholder = $wpdb->placeholder_escape();
</span><span class="cx" style="display: block; padding: 0 10px"> array_walk_recursive(
</span><span class="cx" style="display: block; padding: 0 10px"> $args,
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -4874,6 +4915,8 @@
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px"> );
</span><span class="cx" style="display: block; padding: 0 10px">
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ ksort( $args );
+
</ins><span class="cx" style="display: block; padding: 0 10px"> // Replace wpdb placeholder in the SQL statement used by the cache key.
</span><span class="cx" style="display: block; padding: 0 10px"> $sql = $wpdb->remove_placeholder_escape( $sql );
</span><span class="cx" style="display: block; padding: 0 10px"> $key = md5( serialize( $args ) . $sql );
</span></span></pre></div>
<a id="trunktestsphpunittestsquerycacheResultsphp"></a>
<div class="modfile"><h4 style="background-color: #eee; color: inherit; margin: 1em 0; padding: 1.3em; font-size: 115%">Modified: trunk/tests/phpunit/tests/query/cacheResults.php</h4>
<pre class="diff"><span>
<span class="info" style="display: block; padding: 0 10px; color: #888">--- trunk/tests/phpunit/tests/query/cacheResults.php 2024-05-08 22:04:11 UTC (rev 58121)
+++ trunk/tests/phpunit/tests/query/cacheResults.php 2024-05-08 22:49:46 UTC (rev 58122)
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -172,6 +172,63 @@
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> /**
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * @covers WP_Query::generate_cache_key
+ * @ticket 59442
+ */
+ public function test_generate_cache_key_unregister_post_type() {
+ global $wpdb;
+ register_post_type(
+ 'wptests_pt',
+ array(
+ 'exclude_from_search' => false,
+ )
+ );
+ $query_vars = array(
+ 'post_type' => 'any',
+ );
+ $fields = "{$wpdb->posts}.ID";
+ $query1 = new WP_Query( $query_vars );
+ $request1 = str_replace( $fields, "{$wpdb->posts}.*", $query1->request );
+
+ $reflection = new ReflectionMethod( $query1, 'generate_cache_key' );
+ $reflection->setAccessible( true );
+
+ $cache_key_1 = $reflection->invoke( $query1, $query_vars, $request1 );
+ unregister_post_type( 'wptests_pt' );
+ $cache_key_2 = $reflection->invoke( $query1, $query_vars, $request1 );
+
+ $this->assertNotSame( $cache_key_1, $cache_key_2, 'Cache key should differ after unregistering post type.' );
+ }
+
+ /**
+ * @ticket 59442
+ *
+ * @covers WP_Query::generate_cache_key
+ *
+ * @dataProvider data_query_cache_duplicate
+ */
+ public function test_generate_cache_key_normalize( $query_vars1, $query_vars2 ) {
+ global $wpdb;
+
+ $fields = "{$wpdb->posts}.ID";
+ $query1 = new WP_Query( $query_vars1 );
+ $request1 = str_replace( $fields, "{$wpdb->posts}.*", $query1->request );
+
+ $query2 = new WP_Query( $query_vars2 );
+ $request2 = str_replace( $fields, "{$wpdb->posts}.*", $query2->request );
+
+ $reflection = new ReflectionMethod( $query1, 'generate_cache_key' );
+ $reflection->setAccessible( true );
+
+ $this->assertSame( $request1, $request2, 'Queries should match' );
+
+ $cache_key_1 = $reflection->invoke( $query1, $query_vars1, $request1 );
+ $cache_key_2 = $reflection->invoke( $query1, $query_vars2, $request2 );
+
+ $this->assertSame( $cache_key_1, $cache_key_2, 'Cache key differs the same paramters.' );
+ }
+
+ /**
</ins><span class="cx" style="display: block; padding: 0 10px"> * @dataProvider data_query_cache
</span><span class="cx" style="display: block; padding: 0 10px"> * @ticket 22176
</span><span class="cx" style="display: block; padding: 0 10px"> */
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -217,6 +274,74 @@
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> /**
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * Data provider for test_generate_cache_key_normalize().
+ *
+ * @return array[]
+ */
+ public function data_query_cache_duplicate() {
+ return array(
+ 'post type empty' => array(
+ 'query_vars1' => array( 'post_type' => '' ),
+ 'query_vars2' => array( 'post_type' => 'post' ),
+ ),
+ 'post type array' => array(
+ 'query_vars1' => array( 'post_type' => array( 'page' ) ),
+ 'query_vars2' => array( 'post_type' => 'page' ),
+ ),
+ 'orderby empty' => array(
+ 'query_vars1' => array( 'orderby' => null ),
+ 'query_vars2' => array( 'orderby' => 'date' ),
+ ),
+ 'different order parameter' => array(
+ 'query_vars1' => array(
+ 'post_type' => 'post',
+ 'posts_per_page' => 15,
+ ),
+ 'query_vars2' => array(
+ 'posts_per_page' => 15,
+ 'post_type' => 'post',
+ ),
+ ),
+ 'same args' => array(
+ 'query_vars1' => array( 'post_type' => 'post' ),
+ 'query_vars2' => array( 'post_type' => 'post' ),
+ ),
+ 'same args any' => array(
+ 'query_vars1' => array( 'post_type' => 'any' ),
+ 'query_vars2' => array( 'post_type' => 'any' ),
+ ),
+ 'any and post types' => array(
+ 'query_vars1' => array( 'post_type' => 'any' ),
+ 'query_vars2' => array( 'post_type' => array( 'post', 'page', 'attachment' ) ),
+ ),
+ 'different order post type' => array(
+ 'query_vars1' => array( 'post_type' => array( 'post', 'page' ) ),
+ 'query_vars2' => array( 'post_type' => array( 'page', 'post' ) ),
+ ),
+ 'post status array' => array(
+ 'query_vars1' => array( 'post_status' => 'publish' ),
+ 'query_vars2' => array( 'post_status' => array( 'publish' ) ),
+ ),
+ 'post status order' => array(
+ 'query_vars1' => array( 'post_status' => array( 'draft', 'publish' ) ),
+ 'query_vars2' => array( 'post_status' => array( 'publish', 'draft' ) ),
+ ),
+ 'cache parameters' => array(
+ 'query_vars1' => array(
+ 'update_post_meta_cache' => true,
+ 'update_post_term_cache' => true,
+ 'update_menu_item_cache' => true,
+ ),
+ 'query_vars2' => array(
+ 'update_post_meta_cache' => false,
+ 'update_post_term_cache' => false,
+ 'update_menu_item_cache' => false,
+ ),
+ ),
+ );
+ }
+
+ /**
</ins><span class="cx" style="display: block; padding: 0 10px"> * Data provider.
</span><span class="cx" style="display: block; padding: 0 10px"> *
</span><span class="cx" style="display: block; padding: 0 10px"> * @return array[] Test parameters.
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -263,6 +388,18 @@
</span><span class="cx" style="display: block; padding: 0 10px"> 'post_type' => 'page',
</span><span class="cx" style="display: block; padding: 0 10px"> ),
</span><span class="cx" style="display: block; padding: 0 10px"> ),
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ 'cache true and empty post type' => array(
+ 'args' => array(
+ 'cache_results' => true,
+ 'post_type' => '',
+ ),
+ ),
+ 'cache true and orderby null' => array(
+ 'args' => array(
+ 'cache_results' => true,
+ 'orderby' => null,
+ ),
+ ),
</ins><span class="cx" style="display: block; padding: 0 10px"> 'cache true and ids' => array(
</span><span class="cx" style="display: block; padding: 0 10px"> 'args' => array(
</span><span class="cx" style="display: block; padding: 0 10px"> 'cache_results' => true,
</span></span></pre>
</div>
</div>
</body>
</html>