From 1f5147bf834871466e5f86f5c02baa3c10934adc Mon Sep 17 00:00:00 2001 From: Boone Gorges Date: Thu, 2 Jun 2016 18:27:43 +0000 Subject: [PATCH] Comments: Improve caching for hierarchical queries. Hierarchical comment queries work by first fetching the IDs of top-level comments, and then filling the descendant tree one level at a time based on the top-level results. When top-level comment IDs are found in the cache, `WP_Comment_Query` does not generate the SQL used to fetch these comments. In this case, the `fill_descendants()` query does not have enough information to fill children. As a result, descendant comments were failing to be filled in cases where the top-level comments were found in the cache. This was a minor bug previously, because comment caches were not maintained between pageloads. Since comment caches are now persistent [37613], the problem becomes evident anywhere that a persistent object cache is in use. The solution is to cache parent-child relationships, so that when top-level comments are found in the cache, descendant comments should be found there as well. Fixes #36487. git-svn-id: https://develop.svn.wordpress.org/trunk@37625 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-includes/class-wp-comment-query.php | 52 +++++++++++++++++----- tests/phpunit/tests/comment/query.php | 45 +++++++++++++++++++ 2 files changed, 86 insertions(+), 11 deletions(-) diff --git a/src/wp-includes/class-wp-comment-query.php b/src/wp-includes/class-wp-comment-query.php index bccb7a9bbe..8c35112ba3 100644 --- a/src/wp-includes/class-wp-comment-query.php +++ b/src/wp-includes/class-wp-comment-query.php @@ -742,12 +742,13 @@ class WP_Comment_Query { } } - if ( $this->query_vars['hierarchical'] && ! $this->query_vars['parent'] ) { - $this->query_vars['parent'] = 0; + $parent = $this->query_vars['parent']; + if ( $this->query_vars['hierarchical'] && ! $parent ) { + $parent = 0; } - if ( '' !== $this->query_vars['parent'] ) { - $this->sql_clauses['where']['parent'] = $wpdb->prepare( 'comment_parent = %d', $this->query_vars['parent'] ); + if ( '' !== $parent ) { + $this->sql_clauses['where']['parent'] = $wpdb->prepare( 'comment_parent = %d', $parent ); } if ( is_array( $this->query_vars['user_id'] ) ) { @@ -941,20 +942,49 @@ class WP_Comment_Query { } } + $key = md5( serialize( wp_array_slice_assoc( $this->query_vars, array_keys( $this->query_var_defaults ) ) ) ); + $last_changed = wp_cache_get( 'last_changed', 'comment' ); + if ( ! $last_changed ) { + $last_changed = microtime(); + wp_cache_set( 'last_changed', $last_changed, 'comment' ); + } + // Fetch an entire level of the descendant tree at a time. $level = 0; do { - $parent_ids = $levels[ $level ]; - if ( ! $parent_ids ) { - break; + // Parent-child relationships may be cached. Only query for those that are not. + $child_ids = $uncached_parent_ids = array(); + $_parent_ids = $levels[ $level ]; + foreach ( $_parent_ids as $parent_id ) { + $cache_key = "get_comment_child_ids:$parent_id:$key:$last_changed"; + $parent_child_ids = wp_cache_get( $cache_key, 'comment' ); + if ( false !== $parent_child_ids ) { + $child_ids = array_merge( $child_ids, $parent_child_ids ); + } else { + $uncached_parent_ids[] = $parent_id; + } } - $where = 'WHERE ' . $_where . ' AND comment_parent IN (' . implode( ',', array_map( 'intval', $parent_ids ) ) . ')'; - $comment_ids = $wpdb->get_col( "{$this->sql_clauses['select']} {$this->sql_clauses['from']} {$where} {$this->sql_clauses['groupby']} ORDER BY comment_date_gmt ASC, comment_ID ASC" ); + if ( $uncached_parent_ids ) { + $where = 'WHERE ' . $_where . ' AND comment_parent IN (' . implode( ',', array_map( 'intval', $uncached_parent_ids ) ) . ')'; + $level_comments = $wpdb->get_results( "SELECT $wpdb->comments.comment_ID, $wpdb->comments.comment_parent {$this->sql_clauses['from']} {$where} {$this->sql_clauses['groupby']} ORDER BY comment_date_gmt ASC, comment_ID ASC" ); + + // Cache parent-child relationships. + $parent_map = array_fill_keys( $uncached_parent_ids, array() ); + foreach ( $level_comments as $level_comment ) { + $parent_map[ $level_comment->comment_parent ][] = $level_comment->comment_ID; + $child_ids[] = $level_comment->comment_ID; + } + + foreach ( $parent_map as $parent_id => $children ) { + $cache_key = "get_comment_child_ids:$parent_id:$key:$last_changed"; + wp_cache_set( $cache_key, $children, 'comment' ); + } + } $level++; - $levels[ $level ] = $comment_ids; - } while ( $comment_ids ); + $levels[ $level ] = $child_ids; + } while ( $child_ids ); // Prime comment caches for non-top-level comments. $descendant_ids = array(); diff --git a/tests/phpunit/tests/comment/query.php b/tests/phpunit/tests/comment/query.php index c52e12d098..54015eb47a 100644 --- a/tests/phpunit/tests/comment/query.php +++ b/tests/phpunit/tests/comment/query.php @@ -2430,6 +2430,51 @@ class Tests_Comment_Query extends WP_UnitTestCase { $clauses['where'] .= $wpdb->prepare( ' AND comment_ID != %d AND comment_ID != %d', $this->to_exclude[0], $this->to_exclude[1] ); return $clauses; } + + /** + * @ticket 36487 + */ + public function test_cache_should_be_hit_when_querying_descendants() { + global $wpdb; + + $p = self::factory()->post->create(); + $comment_1 = self::factory()->comment->create( array( + 'comment_post_ID' => $p, + 'comment_approved' => '1', + ) ); + $comment_2 = self::factory()->comment->create( array( + 'comment_post_ID' => $p, + 'comment_approved' => '1', + 'comment_parent' => $comment_1, + ) ); + $comment_3 = self::factory()->comment->create( array( + 'comment_post_ID' => $p, + 'comment_approved' => '1', + 'comment_parent' => $comment_1, + ) ); + $comment_4 = self::factory()->comment->create( array( + 'comment_post_ID' => $p, + 'comment_approved' => '1', + 'comment_parent' => $comment_2, + ) ); + + $q1 = new WP_Comment_Query( array( + 'post_id' => $p, + 'hierarchical' => true, + ) ); + $q1_ids = wp_list_pluck( $q1->comments, 'comment_ID' ); + + $num_queries = $wpdb->num_queries; + $q2 = new WP_Comment_Query( array( + 'post_id' => $p, + 'hierarchical' => true, + ) ); + $q2_ids = wp_list_pluck( $q2->comments, 'comment_ID' ); + + $this->assertEqualSets( $q1_ids, $q2_ids ); + $this->assertSame( $num_queries, $wpdb->num_queries ); + } + /** * @ticket 27571 */