This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Fix duplicate queries in product grids #4695 #5002
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rubikuserbot
requested review from
a team and
opr
and removed request for
a team
|
Size Change: +8.31 kB (+1%) Total Size: 1.11 MB
ℹ️ View Unchanged
|
opr
approved these changes
Oct 28, 2021
| global $wpdb; | ||
|
|
||
| // phpcs:disable WordPress.DB.PreparedSQL.NotPrepared | ||
| $product_variations = $wpdb->get_results( "SELECT ID as variation_id, post_parent as product_id from {$wpdb->posts} WHERE post_parent IN ( " . implode( ',', $prime_product_ids ) . ' )', ARRAY_A ); |
There was a problem hiding this comment.
Just wondering why prime_variation_ids in the next query is run through esc_sql but this one isn't.
There was a problem hiding this comment.
This one has been ran through wp_parse_id_list. $prime_variation_ids is from the results of the first query.
Comment on lines
631
to
635
| // Whilst the query above only includes keys of $default_variation_meta_data, the cache may have been | ||
| // primed elsewhere. | ||
| if ( in_array( $data->attribute_key, $default_variation_meta_keys, true ) ) { | ||
| $values[ $data->variation_id ][ $data->attribute_key ] = $data->attribute_value; | ||
| } |
There was a problem hiding this comment.
It took me quite a while to understand what this check was for, even in spite of the comment. Could you expand on the comment a little bit to give more context? (Feel free to disagree if you think it's just me, though!)
There was a problem hiding this comment.
In the context of this function, we have $default_variation_meta_keys so we know all the values are relevant. The cache prime function I wrote however may have other keys outside of $default_variation_meta_keys e.g. attributes from over variable products. Does that sort of make sense?
github-actions
bot
added
status: ready to merge
and removed
status: needs review
labels
nielslange
changed the title
nielslange
added
the
type: enhancement
jonny-bull
pushed a commit
to jonny-bull/woocommerce-gutenberg-products-block
that referenced
this pull request
Dec 14, 2021
…5002) * Cache variation_meta_data to prevent duplicate queries with multiple grids * Prime the cache * Improve existing cache detection * Expand comment
jonny-bull
pushed a commit
to jonny-bull/woocommerce-gutenberg-products-block
that referenced
this pull request
Dec 16, 2021
…5002) * Cache variation_meta_data to prevent duplicate queries with multiple grids * Prime the cache * Improve existing cache detection * Expand comment
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds caching (and cache priming) to the product grid to prevent duplicate queries.
Fixes #4695
The issue here was the
get_variationsmethod in theProductSchemaclass queries all product variations per product, and that custom query is uncached. If the same product is queried multiple times (for instance, if showing multiple product grids) the query will repeat. This results in duplicate queries.To test this, I added 2 product grids to a page, both containing variable products. This is an example from Query Monitor:
Notice (1), there are 2 calls to each set of variations (duplicate queries). This needs fixing with cache.
Notice (2), there is a separate query for each variable product. This is expected, but could be fixed by priming the cache.
This PR implements both. A cache for each products
$variation_meta_dataquery (1), and a cache priming function once the results of the product grid query are returned (2). This eliminates duplicate queries, and reduces the overall total number of queries on the page:Testing
User facing:
Products by Category. Ensure there are some variable products visible, or add some new onesDev:
https://store.local/wp-json/wc/store/products/11. Confirm thevariationssection is still populated.Changelog