[wp-trac] [WordPress Trac] #56992: The Loop displays incorrect data for queries started with `fields => 'id=>parent'`.

[WordPress Trac]

#56992: The Loop displays incorrect data for queries started with `fields =>
'id=>parent'`.
--------------------------------------+----------------------------
 Reporter:  peterwilsoncc             |       Owner:  peterwilsoncc
     Type:  defect (bug)              |      Status:  reopened
 Priority:  normal                    |   Milestone:  6.8
Component:  Query                     |     Version:  3.1
 Severity:  normal                    |  Resolution:
 Keywords:  has-patch has-unit-tests  |     Focuses:  performance
--------------------------------------+----------------------------
Changes (by SirLouen):

 * keywords:  dev-feedback has-patch has-unit-tests => has-patch has-unit-
     tests


Comment:

 Replying to [comment:44 peterwilsoncc]:
 > I've put together a [https://github.com/WordPress/wordpress-
 develop/pull/8495/files#diff-
 1d050668621cbc3028e027d15e68438f4d879dd6e80ba98f9c9f69b9fb5469d0 PR to
 change how it's determined whether the method needs to walk the array] of
 posts for cache warming of posts and author caches.

 Have you found that

 {{{
 // Ensure a full post object is available
     if ( $post instanceof stdClass ) {
         // stdClass indicates that a partial post object was queried
         $post = get_post( $post->ID );
     } elseif ( is_numeric( $post ) ) {
         // Numeric indicates that only post IDs were queried
         $post = get_post( $post );
     }
 }}}

 Is somehow different from the other version published in
 [https://github.com/WordPress/wordpress-develop/pull/8418 8418] patch?
 {{{
 if ( ! $post instanceof WP_Post ) {
         if ( 'ids' === $this->query_vars['fields'] ) {
                 // Post IDs queried.
                 $post = get_post( $post );
         } elseif ( isset( $post->ID ) ) {
                 // Partial object (stdClass) queried.
                 $post = get_post( $post->ID );
         }
 }
 }}}

 From what I found over my previous tests, there were only 3 options:
 1. WP_Post on `all`
 2. A numeric ID, for `ids`
 3. The stdClass for `id=>parent`.

 Anyway, I think that the same in a way or another way, the improvement
 here is just to check for WP_Post to avoid priming cache in just that
 scenario.

 Again, using the same implementation as the patch just before this one,
 the results are barely identical and excluding the `array_reduce` from the
 `ids` doesn't seem to make a difference with such test battery.

 ||= Fields     =||= 6.7 Time    =||= 6.7 Queries =||= 8495 Patch Time
 =||= 8495 Patch Queries =||=  This Comment Patch time  =||= This Comment
 Patch Queries =||
 || all        || 3.2470240592956543 ||         4    || 3.2879478931427 ||
 4    || 3.2534329891204834 ||       4        ||
 || ids        || 3.8917858600616455 ||       5      || 3.8529629707336426
 ||        5       || 3.8428380489349365  ||        5       ||
 || id, parent || 1.0311799049377441  ||      1       || 3.907115936279297
 ||         5      || 3.9453020095825195 ||        5       ||


 This is the function with the `WP_Post` addition

 {{{#!php
 <?php
 public function the_post() {
     global $post;

     if ( ! $this->in_the_loop ) {
         if ( $this->posts && $this->posts[0] instanceof WP_Post ) {
             $post_objects = $this->posts;
         } else {
             // Get post IDs to prime incomplete post objects
             $post_ids = array_reduce(
                 $this->posts,
                 function ( $carry, $post ) {
                     if ( is_numeric( $post ) && $post > 0 ) {
                         // Query for post ID
                         $carry[] = $post;
                     }

                     if ( is_object( $post ) && isset( $post->ID ) ) {
                         // Query for object, either partial WP_Post or
 stdClass
                         $carry[] = $post->ID;
                     }

                     return $carry;
                 },
                 array()
             );
             if ( $post_ids ) {
                 _prime_post_caches( $post_ids,
 $this->query_vars['update_post_term_cache'],
 $this->query_vars['update_post_meta_cache'] );
             }
             $post_objects = array_map( 'get_post', $post_ids );
         }
         update_post_author_caches( $post_objects );
     }

     $this->in_the_loop = true;
     $this->before_loop = false;

     if ( -1 === $this->current_post ) { // Loop has just started
         /**
          * Fires once the loop is started.
          *
          * @since 2.0.0
          *
          * @param WP_Query $query The WP_Query instance (passed by
 reference).
          */
         do_action_ref_array( 'loop_start', array( &$this ) );
     }

     $post = $this->next_post();

     // Ensure a full post object is available
     if ( $post instanceof stdClass ) {
         // stdClass indicates that a partial post object was queried
         $post = get_post( $post->ID );
     } elseif ( is_numeric( $post ) ) {
         // Numeric indicates that only post IDs were queried
         $post = get_post( $post );
     }

     // Set up the global post object for the loop
     $this->setup_postdata( $post );
 }
 }}}

 Both versions pass the query unit-tests anyway, so given that both the
 original version with the conditional for `WP_Post` and the last in
 [https://github.com/WordPress/wordpress-develop/pull/8495 8495] seem to be
 identical in performance, functionality and correctness, just choose
 whatever you prefer.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/56992#comment:45>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform