Make WordPress Core

Opened 3 weeks ago

Last modified 5 days ago

#53280 new defect (bug)

Incorrect use of WP_Query::get_posts in new get_block_template and get_block_templates functions

Reported by: david.binda Owned by:
Milestone: 5.8 Priority: normal
Severity: normal Version: trunk
Component: Widgets Keywords: has-patch
Focuses: performance Cc:


The usage of WP_Query in both get_block_template and get_block_templates does not feel right to me.

Both functions create a new WP_Query object while passing in the args for the query, and are then calling the WP_Query::get_posts, which leads to a duplicate SQL query execution.

When passing args to WP_Query::__construct method, this one internally executes the WP_Query::get_posts method and stores the result in the WP_Query::posts property. When calling the WP_Query::get_posts again, the same SQL query gets executed, and the result is again stored in the WP_Query::posts property.

Thus, the correct usage, IMHO, is to read the posts already stored in the WP_Query::posts property.

See https://build.trac.wordpress.org/browser/trunk/wp-includes/block-template-utils.php?rev=50612#L91 as well as https://build.trac.wordpress.org/browser/trunk/wp-includes/block-template-utils.php?rev=50612#L133

Both cases were introduced in r50612

Attachments (2)

53280.diff (830 bytes) - added by david.binda 3 weeks ago.
53280.2.diff (741 bytes) - added by david.binda 3 weeks ago.

Download all attachments as: .zip

Change History (5)

3 weeks ago

#1 @david.binda
3 weeks ago

There seems to be the same issue in the wp_filter_wp_template_unique_post_slug function (also part of the same commit - r51003 ). See https://core.trac.wordpress.org/browser/trunk/src/wp-includes/theme-templates.php?rev=51003#L52 and https://core.trac.wordpress.org/browser/trunk/src/wp-includes/theme-templates.php?rev=51003#L62

I'm attaching another patch, addressing the other bit. Not sure if I should combine those patches, or if a new ticket should be opened for the other instance.

3 weeks ago

#2 @desrosj
3 weeks ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 5.8

#3 @desrosj
5 days ago

We're at the beta 1 deadline, but this should remain and be evaluated during beta since it directly relates to new functionality.

Note: See TracTickets for help on using tickets.