Move Product Match functions from db_select to EntityFieldQuery

Hey Ben, sorry your patch fell off my radar. It works perfectly as described; I ran tests comparing the return values of the function before and after your patch, and they're identical as expected. Committed!

I hesitate to reopen this issue since I know the site in question is an edge case, but...
We have built a Drupal Commerce site for a large university which uses it to manage a continuing education program.
For complicated reasons the commerce_product table has > 12,000 rows (it contains migrated data going back to the early '90s).
When this patch is applied we are unable to restart the site after clearing caches.
If I reverse the patch, start the site, and apply the patch everything works fine, but the overhead of loading that many records through entity query is giving us out of memory errors no matter how much memory we have.
I first noticed this when 7.x-1.10 was released, but we did not have time at that point to troubleshoot, so we rolled back to 7.x-1.9.

I am not sure if there is a reasonable solution, and I am able to reverse this patch and move on, but that is less then optimal and I would love some thoughts on possible options!

Hmm, this patch should've just replicated the same query just in a different API. Does it appear we left out a limit statement or something?

Thanks for the response Ryan.
It does replicate the functionality of the query, and the results are identical and work fine in every case except at bootstrap after a cache clear.

I will spend more time in xdebug today , but my current guess is that the problem is the unlimited use of commerce_product_load_multiple().
It is just a guess though.
Will add more info as I find it.

Oooh, I see, we used commerce_product_load_multiple() whereas before all the information we needed was in the query results. So I wonder if somehow your site has an unrestricted use of this function, since $limit normally defaults to 10.

I wonder if there's some way for us to get the SKU into the stub that an EntityFieldQuery returns. If not, we should just directly query the database for SKUs, as nothing in a full load is going to alter that.

Well that was instructive. :)
Here is what I have determined.
I had a rule (actually several rules) that use product reference fields as the value of a "Fetch entity by property".
These product reference fields had no "Options list limit" values set in their field settings.
When rules_init does its cache magic it loads every single product (in this case 12,467).
A similar thing happened whenever I hit admin>configuration>workflow>rules

Without the patch (i.e. using an old style db_select) everything is just fine even through it loads all those rows.
Switching to an entity query brings the server to a grinding halt.

The bottom line here is that I have fixed my problem by applying options list limits to all the product reference fields.
Is it naively optimistic to think I may be the only person who will ever experience this?
Thanks in advance for your wisdom.

Hah, nah, if that hit you, I'm sure it would affect someone else. ; )

I actually wouldn't have guessed the options list limit would even apply here. What field widget are those product reference fields configured to use?

I dug into it a bit more, and think that what is happing is that on cache clear, or on the config page load, rules is building options lists for its forms.
When it does that it loads every entity that can be referenced.
The options limit list applies since rules always presents them in that format, but it does not seem to matter which widget was wanted.
Some are checkboxes, some are options lists, and some are views.
Internally we are creating and assigning these products in code as part of a very complex workflow, so the actual reference is never really exposed to users.

I'm experiencing the exact same problem as described in #5. The database consists of almost 7000 products. Everything works if I revert the patch from #1.

I haven't been investigating this as much as @jkopel, but I believe that any findings would be redundant since he's been able to provide quite accurate details.

I have same problem described in #5 ad #12. The database consist of 14000 products.

I think you need to replace commerce_load_multiply() with db_select(). See attached patch.

+++ b/modules/product/commerce_product.module
+++ b/modules/product/commerce_product.module
@@ -897,7 +897,11 @@ function _commerce_product_match_products_standard($instance, $string = '', $mat

@@ -897,7 +897,11 @@ function _commerce_product_match_products_standard($instance, $string = '', $mat
+    unset($entities);

Is this unset() needed?

Otherwise this looks look like a great idea, no need to load the entities when trying to get specific keys.

Tagging for http://contribkanban.com/#/board/commerce.

Thanks for the patch. This has solved my issues described at #2537872: Updating to 7.x-1.10 causes memory issues due to having large number of products

The foreach is taking between 60 and 90 seconds to run through 29,050 products. I'm wondering if there's any way to get past this (having to loop through all products to build the array) without setting max_execution_time to 90 seconds?

Running a re-test. The test for this function was partially broken and want to re-run this

[comment removed to avoid confusion]

[comment removed to avoid confusion]

[comment removed to avoid confusion]

Before pushing back to "Needs Work" let's review this

I'm loading product entities from a remote system over a REST API. Using db_select breaks product match queries when your entities are using a different storage engine.

By moving to EntityFieldQuery, the product match query is abstracted away from SQL and can work with different entity storage controller engines.

EntityFieldQuery utilizes field storage API effectively, whereas db_select and others do not. So if someone is not using field_sql_storage this will break. The patch in #13 breaks the original issue, and at that point we may as well revert patch in #1.

So it seems the biggest issue is that commerce_product_reference_options_list() defaults to "NULL" if the field instance isn't limiting

  // Look for an options list limit in the field settings.
  if (!empty($field['settings']['options_list_limit'])) {
    $limit = (int) $field['settings']['options_list_limit'];
  }
  else {
    $limit = NULL;
  }

Ditto with commerce_product_reference_field_validate, but it shouldn't matter here because it's being passed a set of product IDs to validate.

So, I'm attaching a patch which defaults options_list to 10 if the setting is left empty, meaning people can bump it up if they'd like to. This option should keep abstraction benefits of EFQ and cut down on performance issues from Rules cache rebuild.

I/R/T: #9 and #11

Thanks mglaman.

I just reverted back to stock Commerce 7.x-1.11 (which includes the patch in #1 afaik). Without your limit patch I get an allowed memory error when I clear my cache. With your patch the error goes away and my cache is cleared appropriately. In my case it wasn't rules, but Views/Search API that were loading all products from a product reference field.

Works perfectly for me.

Thanks for review! Committed.