+ Start a Discussion
Lee_CampbellLee_Campbell 

Large SOQL help

Hi folks,

 

I've written a function that's called by a trigger upon creation of an opportunity that calculates the percentage chance of winning an opportunity based on historical performance given a number of "factors" pertaining to that opportunity, namely: who the opportunity owner is, which product it is we're selling, whether we've sold to this customer before, the total value of the proposed sale, and which account we're selling to (which is subtly different from repeat business). The function works out the previous percentages of successful sales based on each of these criteria and provides a kind of weighted average of the "probability" of winning the opportunity based on these, for forecasting purposes.

 

The trouble, however, is that I'm getting SOQL limit exceptions in my live environment, which, as you might expect, has a lot more data to draw upon to make these calculations. Having said that, thinking about the number of SOQL queries a single calculation should make, it should be nowehere near the 50,000 limit. The code (below), as I said, works out the percentage chance of winning an opportunity based upon the following, which I've tabulated, and given, what I think is a sum of the total number of SOQL queries induced by a single opportunity being created (to reiterate, the function code is called by a trigger when an opportunity is created, and this trigger does NOTHING else):

 

Opportunity Owner - nobody in my organisation owns more than about 500 opportunities, so that's 500 SOQL queries.

Account - no account has more than about 200 opportunities recorded against it, taking our total to 700.

Value - I've bucketed the value into three categories, none of which has more than 4,000 members, taking our total to 4,700.

Type - this pertains to whether or not this is repeat business and has three categories, none of which has more than about 4,000 members again, taking our total to 8,700 queries.

Product - again, this has a few categories, but none of these have more than about 2,000 recorded opportunities against them, taking the total to 10,700

 

So, after these very generous estimates, my code can call no more than 10,700 queries per invocation of the trigger (I understand that this would cause me problems if I want to bulk upload opportunities, but I'll cross that bridge when I come to it), in the meantime, I'm getting the SOQL limit error after only one invocation. My question then, is, given the code, can anybody point me in the direction of a solution to hitting this limit, which, I don't see how I'm hitting?

 

Apologies for being so verbose, but, hopefully, there shouldn't be anything I need to clarify :/

 

Thanks,

Lee

public class FCastProb {
	public static double getProbability(ID own, String opptype, String pipe, ID acc, decimal value){
    	
        double ownWins = 0;
        double ownSize=0;
        for(Opportunity a: [select Probability from Opportunity where OwnerID =:own and IsClosed=true]){
           		ownSize+=1;
                if(a.Probability==100){
               		ownWins+=1;
           		}
        	}
        
        
        if(ownWins == 0||ownSize==0){
            ownSize=1;
            ownWins=1;
        }
                
        double typeWins = 0;
        double typeSize = 0;
                
        	for(Opportunity b : [select Probability from Opportunity where Type =: oppType and IsClosed=true]){
           		typeSize+=1;
                if(b.Probability==100){
	               	typeWins+=1;
    	       	}
        	}
    	
        
    	if(typeWins==0||typeSize==0){
        	typeSize = 1;
        	typeWins = 1;
    	}
        
        double pipeWins = 0;
        double pipeSize = 0;
                	for(Opportunity c : [select Probability from Opportunity where Market_Pipeline__c =: pipe and IsClosed=true]){
           		pipeSize+=1;
                if(c.Probability==100){
	               	pipeWins+=1;
    	       	}
        	}
                if(pipeSize==0||pipeWins==0){
            pipeSize = 1;
            pipeWins = 1;
        }
                
        double accWins = 0;
               	double accSize=0;
        
			for(Opportunity d : [select Probability from Opportunity where AccountID =: acc and IsClosed = true]){
           		accSize+=1;
                if(d.Probability==100){
               		accWins+=1;
           		}
        	}
        
        if(accSize==0||accWins==0){
            accSize = 1;
            accWins = 1;
        }
        
        double valwins = 0;
        double valsize = 0;
        if(value<SOME_VALUE){
            for(Opportunity a: [select Probability from Opportunity where Total_Oppty_Amount__c < SOME_VALUE and IsClosed = true]){
                valsize+=1;
                if(a.Probability==0){
                    valwins+=1;
                }
            }
        }
        else if(value<SOME_VALUE&&value>=SOME_VALUE){
            for(Opportunity a:[select Probability from Opportunity where Total_Oppty_Amount__c >=SOME_VALUE and Total_Oppty_Amount__c <SOME_VALUE and IsClosed =true]){
                valsize+=1;
                if(a.Probability==100){
                    valwins+=1;
                }
            }
        }
        else for(Opportunity a:[select Probability from Opportunity where Total_Oppty_Amount__c >=SOME_VALUE and IsClosed = true]){
            valsize+=1;
            if(a.Probability==100){
                valwins+=1;
            }
        }
        if(valwins==0||valsize==0){
            valwins=1;
            valsize=1;
        }
        
        double x = 100*(ownWins/ownSize)*(typeWins/typeSize)*(pipeWins/pipeSize)*(accWins/accSize)*(valwins/valsize);

        system.debug('X = ' +x);
                
        return x;
    }
}

 

Rahul BorgaonkarRahul Borgaonkar

Dear Lee,

 

You can use Limit methods to get governers

 

example - myDMLLimit = Limits.getDMLStatements(); check page 413 - Force.com Apex Code Developer's Guide for details.

 

Thanks,

 

Rahul

Hengky IlawanHengky Ilawan

Hi Lee,

 

50K is the total number of records retrieved by SOQL queries, not the number of SOQL queries.

 

http://www.salesforce.com/us/developer/docs/apexcode/Content/apex_gov_limits.htm

 

Btw, how do you call this class from your trigger?

And what's the trigger event that call this class (before/after etc)?

 

-Hengky-

Lee_CampbellLee_Campbell

My apologies, I wasn't completely clear. The "sum" I performed is a very generous estimate of the maximum number of records that should've been retrieved by all of the queries performed.

 

The trigger simply says:

 

Trigger ForecastProb on Opportunity(before insert){
    for(Opportunity o:trigger.new){
        o.ForecastProb__c = ForecastProb.getProb(field,field,field,field,field);
     }
}

 

Lee_CampbellLee_Campbell

*with the appropriate field names and the correct function name and all that stuff...

Hengky IlawanHengky Ilawan

Hi Lee,

 

I see that you are calling your class inside a For loop in your trigger. And in your class, there are a few SOQL calls already. So basically you are making SOQL calls inside a loop, which is not a good idea.

 

I suggest that you pass the trigger.new to your class and process the list in there. 

 

-Hengky-

Lee_CampbellLee_Campbell
List<Opportunity> b = [select Probability from Opportunity where OwnerID =: trigger.new[i].OwnerID and IsClosed=true];	
    	for(Opportunity a:b){
        	ownSize+=1;
            if(a.Probability==100){
            	ownWins+=1;
         	}
     	}

 Do you mean the above? I'm not sure how this could work... if I insert more than one opportunity, I need to make the comparisons between each of their different OwnerIDs and those OwnerIDs' historic opportunity wins/losses, which produces the same number of queries, surely?

 

Or am I missing the point?

Lee_CampbellLee_Campbell

So, I tried the code below, modified so that I think, I'm "passing" trigger.new, but I'm still over my 50000 limit. I'm completely at a loss...

 

trigger FC_ProbabilityTrigger on Opportunity (before insert, before update) { 	
    double ownWins = 0;
    double ownSize = 0;
    double typeWins = 0;
    double typeSize = 0;
    double pipeWins = 0;
    double pipeSize = 0;
    double accWins = 0;
    double accSize = 0;
    double valwins = 0;
    double valsize = 0;
        
    for(integer i = 0;i<trigger.new.size();i++){
    	List<Opportunity> b = [select Probability from Opportunity where OwnerID =: trigger.new[i].OwnerID and IsClosed=true];	
    	for(Opportunity a:b){
        	ownSize+=1;
            if(a.Probability==100){
            	ownWins+=1;
         	}
     	}
    	List<Opportunity> oppType = [select Probability from Opportunity where Type =: trigger.new[i].Type and IsClosed=true order by CloseDate desc limit 50000];
    	for(Opportunity opb:oppType){       		
    		typeSize+=1;
            if(opb.Probability==100){
	            typeWins+=1;
    	    }
        }
    	List<Opportunity> pipe = [select Probability from Opportunity where Market_Pipeline__c =: trigger.new[i].Market_Pipeline__c and IsClosed=true];
    	for(Opportunity c : pipe){
          	pipeSize+=1;
            if(c.Probability==100){
	            pipeWins+=1;
    	    }
        }
        List<Opportunity> accType = [select Probability from Opportunity where AccountID =: trigger.new[i].AccountID and isClosed = true];
        for(Opportunity d : accType){
           	accSize+=1;
            if(d.Probability==100){
               	accWins+=1;
           	}
        }
        if(trigger.new[i].Total_Oppty_Amount__c<20000){
            List<Opportunity> valOpp = [select Probability from Opportunity where Total_Oppty_Amount__c < 20000 and IsClosed = true];
            for(Opportunity a: valOpp){//
                valsize+=1;
                if(a.Probability==0){
                    valwins+=1;
                }
            }
        }
        else if(trigger.new[i].Total_Oppty_Amount__c<75000&&trigger.new[i].Total_Oppty_Amount__c>=20000){
            List<Opportunity> valOpp = [select Probability from Opportunity where Total_Oppty_Amount__c < 75000 and Total_Oppty_Amount__c >= 20000 and IsClosed = true];
            for(Opportunity a:valOpp){
                valsize+=1;
                if(a.Probability==100){
                    valwins+=1;
                }
            }
        }
        else {
            List<Opportunity> valOpp = [select Probability from Opportunity where Total_Oppty_Amount__c >= 75000 and IsClosed = true];
            for(Opportunity a:valOpp){
            valsize+=1;
            if(a.Probability==100){
                valwins+=1;
            }
        	}
        }
}
        
    if(ownWins == 0||ownSize==0){
         ownSize=1;
         ownWins=1;
    }
    if(typeWins==0||typeSize==0){
       	typeSize = 1;
       	typeWins = 1;
    }
    if(pipeSize==0||pipeWins==0){
        pipeSize = 1;
        pipeWins = 1;
    }
    if(accSize==0||accWins==0){
       accSize = 1;
       accWins = 1;
    }
    if(valwins==0||valsize==0){
        valwins=1;
        valsize=1;
     }
        
     double x = 100*(ownWins/ownSize)*(typeWins/typeSize)*(pipeWins/pipeSize)*(accWins/accSize)*(valwins/valsize);
	 for(Opportunity a:trigger.new){
           a.Chance_of_Winning__c = x;
    }
    
}

 

 Any suggestions?

Hengky IlawanHengky Ilawan

Hi Lee,

 

You are still getting the limit exception because basically you are still calling the SOQL queries in a loop.

 

I would use a map to store the Size and Wins for each queries, something like this:

private class SizeWinStruct {
    public double size = 0;
    public double wins = 0;
}

Map<Id, SizeWinStruct> sizeWinsByOwnerMap = new Map<Id, SizeWinStruct>();

// get the list of ownerids from trigger.new
Set<Id> ownerIds = new Set<Id>();
for (Opportunity opp : trigger.new) {
    ownerIds.add(opp.OwnerId);
}

for (Opportunity opp : [
        SELECT Probability FROM Opportunity WHERE OwnerID = :ownerIds AND IsClosed = true]) {

    SizeWinStruct ownMap = sizeWinsByOwnerMap.get(opp.OwnerId);

    if (ownMap != null) {
        ownMap.size++;
        if (opp.Probability == 100) {
            ownMap.wins++;
        }
    }
    else {
        ownMap = new SizeWinStruct();
        ownMap.size = 1;
        if (opp.Probability == 100) {
            ownMap.wins = 1;
        }
        sizeWinsByOwnerMap.put(opp.OwnerId, ownMap);
    }
}

The above code is only for the first factor. You can use similar code for the rest of the factors.

 

Note: the above code is not tested.

 

-Hengky-

Lee_CampbellLee_Campbell
for(Opportunity opp:trigger.new){
     opp.Chance_of_Winning__c = sizeWinsbyOnwerMap.get(opp.OwnerID,**wins**)/sizeWinsbyOwnerMap.size();
}

 

Hello again,

 

I don't fully follow the code, by which I mean, I don't know how I'm supposed to use the numbers stored by the map at the end. Outside each of the five loops of the type you've done, I need to do something like the above (but with all of the variables included to form my "average"), but I don't know to access the "wins" of the map. I don't understand maps all that well, can you help me fit this final piece of the puzzle?

 

All the best,

Lee

 

 

Hengky IlawanHengky Ilawan

Hi Lee,

 

Could you post up your code, please?

Thanks.

 

 

-Hengky-

Lee_CampbellLee_Campbell

So my code is a few repetitions of what you posted initially for the different dependent variables I'm trying to model about. Here it is:

 

trigger FC_Probability on Opportunity (before insert) {

private class SizeWinStruct {
    public double size = 0;
    public double wins = 0;
}

Map<Id, SizeWinStruct> sizeWinsByOwnerMap = new Map<Id, SizeWinStruct>();

// get the list of ownerids from trigger.new
Set<Id> ownerIds = new Set<Id>();
for (Opportunity opp : trigger.new) {
    ownerIds.add(opp.OwnerId);
}

for (Opportunity opp : [
        SELECT Probability FROM Opportunity WHERE OwnerID = :ownerIds AND IsClosed = true]) {

    SizeWinStruct ownMap = sizeWinsByOwnerMap.get(opp.OwnerId);

    if (ownMap != null) {
        ownMap.size++;
        if (opp.Probability == 100) {
            ownMap.wins++;
        }
    }
    else {
        ownMap = new SizeWinStruct();
        ownMap.size = 1;
        if (opp.Probability == 100) {
            ownMap.wins = 1;
        }
        sizeWinsByOwnerMap.put(opp.OwnerId, ownMap);
    }
            
}
    Map<String, SizeWinStruct> sizeWinsByTypeMap = new Map<String, SizeWinStruct>();

// get the list of ownerids from trigger.new
Set<String> opptypes = new Set<String>();
for (Opportunity opp : trigger.new) {
    opptypes.add(opp.Type);
}

for (Opportunity opp : [
        SELECT Probability FROM Opportunity WHERE Type = :opptypes AND IsClosed = true]) {

    SizeWinStruct typeMap = sizeWinsByTypeMap.get(opp.Type);

    if (typeMap != null) {
        typeMap.size++;
        if (opp.Probability == 100) {
            typeMap.wins++;
        }
    }
    else {
        typeMap = new SizeWinStruct();
        typeMap.size = 1;
        if (opp.Probability == 100) {
            typeMap.wins = 1;
        }
        sizeWinsByTypeMap.put(opp.Type, typeMap);
    }
            
}
    Map<Id, SizeWinStruct> sizeWinsByAccountMap = new Map<Id, SizeWinStruct>();

// get the list of ownerids from trigger.new
Set<Id> accIds = new Set<Id>();
for (Opportunity opp : trigger.new) {
    accIds.add(opp.AccountID);
}

for (Opportunity opp : [
        SELECT Probability FROM Opportunity WHERE AccountID = :accIds AND IsClosed = true]) {

    SizeWinStruct accMap = sizeWinsByAccountMap.get(opp.AccountId);

    if (accMap != null) {
        accMap.size++;
        if (opp.Probability == 100) {
            accMap.wins++;
        }
    }
    else {
        accMap = new SizeWinStruct();
        accMap.size = 1;
        if (opp.Probability == 100) {
            accMap.wins = 1;
        }
        sizeWinsByAccountMap.put(opp.AccountID, accMap);
    }
            
}
    Map<Id, SizeWinStruct> sizeWinsByPipelineMap = new Map<Id, SizeWinStruct>();

// get the list of ownerids from trigger.new
Set<String> pipelines = new Set<String>();
for (Opportunity opp : trigger.new) {
    pipelines.add(opp.Market_Pipeline__c);
}

for (Opportunity opp : [
        SELECT Probability FROM Opportunity WHERE Market_Pipeline__c = :pipelines AND IsClosed = true]) {

    SizeWinStruct pipeMap = sizeWinsByPipelineMap.get(opp.Market_Pipeline__c);

    if (pipeMap != null) {
        pipeMap.size++;
        if (opp.Probability == 100) {
            pipeMap.wins++;
        }
    }
    else {
        pipeMap = new SizeWinStruct();
        pipeMap.size = 1;
        if (opp.Probability == 100) {
            pipeMap.wins = 1;
        }
        sizeWinsByPipelineMap.put(opp.Market_Pipeline__c, pipeMap);
    }
            
}
    Map<Id, SizeWinStruct> sizeWinsByValueMap = new Map<Id, SizeWinStruct>();

// get the list of ownerids from trigger.new
Set<double> values = new Set<double>();
for (Opportunity opp : trigger.new) {
    values.add(opp.Total_Oppty_Amount__c);
}
//and the for loop that should go here has baffled me, because I don't understand maps well enough...

    for(Opportunity opp:trigger.new){
        //write the value of the probability to the opportunity itself...
        opp.Chance_of_Winning__c = 100*sizewinsbyownermap.get(opp.OwnerID)*sizewinsbyaccountmap.get(opp.AccountID)*sizewinsbytypemap.get(opp.Type)*sizewinsbypipelinemap.get(opp.Market_Pipeline__c);
        //the above line contains illegal assignments, from SizeWinStrcut to "decimal"
    }
    
}

 What I don't understand is how I actually get the probability out at the end from the map...

 

Lee

Lee_CampbellLee_Campbell

Apologies, your original comments are present before each new Set.

Lee_CampbellLee_Campbell

Also, the "key" that maps my probabilities to my opportunity values is a little sketchy is it not? I.e. every other value that constitutes part of a map is discrete, whereas my opportunity value is continuous, and, therefore more difficult to map. I'm thinking I could create a dummy formula field that's a string or something to replicate the effect of the bucketing I had previously employed (where I said if my value was over x, do this, if it was between x and y, do something else and so on...).

 

Whatever... I'm waffling. I'll figure that part out. All I really want to know is how to arithmetically use the values stored in the map.

Hengky IlawanHengky Ilawan

Hi Lee,

 

Here is how to calculate the X at the end of your original code with Map

 

// double x = 100*(ownWins/ownSize)*(typeWins/typeSize)*(pipeWins/pipeSize)*(accWins/accSize)*(valwins/valsize);

for (Opportunity opp : trigger.new) {
	double x = 100 * 
		(sizeWinsByOwnerMap.get(opp.OwnerId).size / sizeWinsByOwnerMap.get(opp.OwnerId).wins) *
		(sizeWinsByTypeMap.get(opp.Type).size / sizeWinsByTypeMap.get(opp.Type).wins) *
		(sizeWinsByPipelineMap.get.get(opp.Type).size / sizeWinsByPipelineMap.get.get(opp.Type).wins) *
		...
		...
		
	// assign x to your opportunity custom field
	opp.ForecastProb__c = x;
}

 

-Hengky-

Hengky IlawanHengky Ilawan

Yes, it is possible to create a formula field to 'bucket' the value. 

 

Below formula field returns text data type:

 

IF (Total_Oppty_Amount__c < SOMEVALUE, 'Small',
   IF (Total_Oppty_Amount__c < SOMEBIGGERVALUE, 'Medium',
      'Large'
   )
)

Then in your query you can just use it like this:

 

SELECT Probability FOM Opportunity WHERE MyFormulaField = 'Small' AND IsClosed = TRUE

 

-Hengky-