Changeset 58122

Timestamp:

05/08/2024 10:49:46 PM (7 months ago)

Author:

spacedmonkey

Message:

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.

Location:

trunk

Files:

• src/wp-includes/class-wp-query.php (modified) (6 diffs)
• tests/phpunit/tests/query/cacheResults.php (modified) (3 diffs)

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

@@ -2265 +2265 @@
                 } elseif ( count( $post_type ) === 1 ) {
                     $post_type = $post_type[0];
+                } else {
+                    // Sort post types to ensure same cache key generation.
+                    sort( $post_type );
                 }
 
@@ -2543 +2546 @@
             }
         } elseif ( ! empty( $post_type ) && is_array( $post_type ) ) {
+            // Sort post types to ensure same cache key generation.
+            sort( $post_type );
             $post_type_where = " AND {$wpdb->posts}.post_type IN ('" . implode( "', '", esc_sql( $post_type ) ) . "')";
         } elseif ( ! empty( $post_type ) ) {
@@ -2648 +2653 @@
 
             if ( ! empty( $queried_post_types ) ) {
-
+                sort( $queried_post_types );
                 $status_type_clauses = array();
 
@@ -3105 +3110 @@
         }
 
-        // Beginning of the string is on a new line to prevent leading whitespace. See https://core.trac.wordpress.org/ticket/56841.
+        /*
+         * 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
+         */
         $old_request =
             "SELECT $found_rows $distinct $fields
-             FROM {$wpdb->posts} $join
-             WHERE 1=1 $where
-             $groupby
-             $orderby
-             $limits";
+                     FROM {$wpdb->posts} $join
+                     WHERE 1=1 $where
+                     $groupby
+                     $orderby
+                     $limits";
 
         $this->request = $old_request;
@@ -4857 +4872 @@
         );
 
+        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';
+        }
+
         $placeholder = $wpdb->placeholder_escape();
         array_walk_recursive(
@@ -4875 +4916 @@
         );
 
+        ksort( $args );
+
         // Replace wpdb placeholder in the SQL statement used by the cache key.
         $sql = $wpdb->remove_placeholder_escape( $sql );

trunk/tests/phpunit/tests/query/cacheResults.php

@@ -170 +170 @@
 
         $this->assertSame( $cache_key_1, $cache_key_2, 'Cache key differs when using wpdb placeholder.' );
+    }
+
+    /**
+     * @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.' );
     }
 
@@ -218 +275 @@
 
     /**
+     * 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,
+                ),
+            ),
+        );
+    }
+
+    /**
      * Data provider.
      *
@@ -262 +387 @@
                     'cache_results' => true,
                     'post_type'     => 'page',
+                ),
+            ),
+            '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,
                 ),
             ),