Opened 5 weeks ago
Closed 4 weeks ago
#59224 closed defect (bug) (fixed)
get_pages: duplicate `WP_Query::get_posts` call
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.3.2 | Priority: | normal |
Severity: | normal | Version: | 6.3 |
Component: | Posts, Post Types | Keywords: | has-patch commit fixed-major |
Focuses: | performance | Cc: |
Description (last modified by )
The internal usage of WP_Query
in get_pages
(introduced in r55569 ) performs the WP_Query::get_posts()
call twice ( see source:tags/6.3/src/wp-includes/post.php#L6094 ).
When query args are passed to the WP_Query
constructor, it returns the result of the WP_Query::get_posts
method call (proxied through WP_Query::query
). All the fetched posts are then already present in the WP_Query
object and the correct way to access those is via the WP_Query->posts
property.
In case the WP_Query::get_posts()
is used, a duplicate SQL query is performed, eventually leading to performance downgrade (yet the caching inside the WP_Query
mitigates this to some degree, unless cache_results
query param is set to true
).
Anyway, I would propose to work with the WP_Query
inside the get_pages
in the similar way as the code does in the get_posts
, where the query args are passed to the object through the WP_Query::query
method it's return value (the found posts) is being used (see source:tags/6.3/src/wp-includes/post.php#L2440 ).
Attachments (1)
Change History (15)
#3
@
5 weeks ago
Thank you, @davidbinda, for providing the ticket and patch.
As you mentioned, the lack of extra query requests due to the wp_query cache explains why this went unnoticed. Well done on catching that detail.
The proposed changes are looks good to me.
#4
@
5 weeks ago
- Keywords commit added
Good catch @davidbinda.
Patch looks good to me. If other are happy, I will commit.
This ticket was mentioned in Slack in #core-performance by clarkeemily. View the logs.
5 weeks ago
This ticket was mentioned in PR #5114 on WordPress/wordpress-develop by joemcgill.
5 weeks ago
#8
This is a direct copy of 59224.diff from the original ticket. Opening the PR to allow CI to run before committing.
Trac ticket: https://core.trac.wordpress.org/ticket/59224
#11
@
5 weeks ago
- Keywords fixed-major added
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening to be backported to the 6.3 branch.
joemcgill commented on PR #5114:
5 weeks ago
#12
Committed in 56491.
Uh, yea, I always suspected that [55569] will backfire eventually.
WP pages are not "just another post type", they have some specifics that all current code is expecting. Throwing away 14 years of incremental fixes and improvements for pages is not a good decision imho.
Changing the milestone to 6.3.2 as it would be best to fix this soon.