Sign Up ›

Browse by Topic
ShowAll Questionssorted byDate Posted
+ Start a Discussion
Ross Gilbert 31Ross Gilbert 31 

too many soql queries on opportunity trigger/class

I call the method below from my opportunity's before update trigger.  I'm hitting a too many soql queries error in some cases and when i run the debug log the culprit is this block of code.  I think it is because this thing has a for loop within a for loop that is causing the issue but I'm not sure.  

Anyone see anything in this piece of code that would cause too many soql queries error:
  
public static void updateProductMarket(List<Opportunity> opps) {
        Schema.DescribeFieldResult Pickvalue = Opportunity.Product_Market_s__c.getDescribe();
        List<Schema.PicklistEntry> PickListValue = Pickvalue.getPicklistValues();
        //get a list of all Ids we are updating and add them to oppIds
        set<Id> oppIds = new set<Id>();
        for(Opportunity opp : opps) {
            oppIds.add(opp.Id);
        }
        //get all Opportunity Products for all Opportunities being updated
        List<OpportunityLineItem> olis = [SELECT Product_Market__c, Opportunity.Id FROM OpportunityLineItem WHERE Opportunity.Id in:oppIds];
        for(Opportunity opp : opps) {
            //loop through each line item
            for(OpportunityLineItem oli : olis) {
                if(oli.Opportunity.Id == opp.Id) {
                    //we have a match
                    if(oli.Product_Market__c != null){
                        if(opp.Product_Market_s__c == null) {
                            opp.Product_Market_s__c = oli.Product_Market__c;
                        } else {
                            if(isNewProductMarket(opp.Product_Market_s__c.split(';'), oli.Product_Market__c)){
                                opp.Product_Market_s__c = opp.Product_Market_s__c + ';' + oli.Product_Market__c;
                            }
                        }
                    }
                }
            }
        }
    }

 
Joseph BauerJoseph Bauer
Hi Ross,

There's nothing wrong with this code as far as queries go. It just makes one query (line 10). This method could be causing the error because you had 100 queries in the execution context just before this method was called and then line 10 become query 101 and thus the error occurred here. To make sure your code is efficient check to make sure this method isn't being called from within a loop (if it loops over 100 times it would hit the soql error) and also make sure no other queries in your code are within a loop.
Peter FribergPeter Friberg
One other thing.
You are doing a soql of all opportunity line items belonging to the opportunity Ids found.
Then you are looping over all opportunities and for each opportunity you are looping through all opportunity line items for all opportunities. Thiswill  generate lots of executions in the inner loop context. Imagine 10 opptys with 10 lines each. Your implementation would execute the inner loop context 10 x (10 x 10) = 1000 times.
You could change your query to use parent child relation: 
Map<Id,Opportunity> oppMap = new Mapt<Id,Opportunity> ([
   SELECT Id, Product_Market_s__c,
          (SELECT Id, Product_Market__c
           FROM OpportunityLineItems)
   WHERE Id in:oppIds]);
Then outer loop as before: 
for (Opportunity opp : opps) {
But now inner loop using parent child query relation (list) 
for (OpportunityLineItem oli : oppMap.get(opp.Id).OpportunityLineItems) {
And do your stuff
This would only make the inner loop context execute 10 x (10) = 100 times.
Could be handy if you are doing complex stuff.

As a tip: try always to use relational queries to simplify processing :)