Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
commerce_cart_commerce_product_calculate_sell_price_line_item_alter() calls commerce_cart_order_id() which explicitely calls some additional SQL queries, while commerce_cart_order_load() will trigger the same function but will statically cache the result: it should be used instead.
SVN patch (sorry, no GIT here) 2 liner patch attached, which saves 1 to 20 SQL queries on my site.
Comments
Comment #1
Damien Tournoud CreditAttribution: Damien Tournoud commentedAlternatively, couldn't we move the static cache from
commerce_cart_order_load()
tocommerce_cart_order_id()
?Comment #2
pounardThat probably could do the trick too. Since the cart will be loaded anyway it doesn't really change a thing, but yes, it may be "cleaner" in some way.
EDIT: For the record, my patch is correct because it triggers PHP warnings when the user has no cart.
Comment #3
marcin.wosinek CreditAttribution: marcin.wosinek commentedI've analysed patch and both functions used and this patch makes sense for me. Here it is slightly changed.
Comment #4
marcin.wosinek CreditAttribution: marcin.wosinek commented#1
It would be more clean, but we have multiple
return
s incommerce_cart_order_id
. Because of this, I think we have two ways to move the static cache:return
_commerce_cart_order_id
doing all logic, and leavecommerce_cart_order_id
do do only cachingWhat do you think is a better way for it?
Comment #5
pounardI dont see the point of introducing a new function for this, but I'm definitely ok for using a variable instead of the two return statements. Nevertheless, the function is small enough to leave no ambiguity in what it does so I'm fine with keeping the #3 patch too.
Comment #6
marcin.wosinek CreditAttribution: marcin.wosinek commentedHere is patch implementing idea from comment #2. For me, both solutions are equal; I would chose one which introduce pattern more common in commerce.
Comment #8
rszrama CreditAttribution: rszrama commentedHonestly, I observed no difference in query counts with or without the patch. However, it does make more sense to cache cart order IDs in the actual function that's loading them. The function commerce_cart_order_ids_reset() also needed to be updated to reference the changed static cache function ID. Let's see what test bot thinks; in practice, it seemed to work fine for me.
Comment #9
rszrama CreditAttribution: rszrama commentedAlrighty, committed.
Comment #10
pounardYou can observe differences only if the cart is being manipulated more than once on the page, which can happen very fast as soon as you play with Views, blocks, etc... In my use case, it removed 9 SQL queries.
Comment #11
rszrama CreditAttribution: rszrama commentedAhh, ok - I was only viewing a few listing / product pages. Glad to know it's helping. : )
Comment #12
rszrama CreditAttribution: rszrama commentedAhh, ok - I was only viewing a few listing / product pages. Glad to know it's helping. : )