Apex Code Development You need to sign in to do that
Don't have an account?
ministe2003 Bulk Trigger and too many soql queries
Hi all
I was having issues with too many SOQL queries in a trigger. I'm trying to mass update opportunities which I think the limit of is 19, so I changed the code to work with a Batchable class. Then I got too many concurrent batches errors. So in my trigger, I query the AsyncApexJob object and only set batches to run when the queue is clear.
Now I'm back to having too many SOQL queries because I'm querying AsyncApexJob too often! AAGH!!
Is there a way around this?
Regards
Steven
Managed to sort it, hurrah!! Is there a more fulfilling feeling than sorting something that's taken foreveeeeer??
Here's my finished trigger, thanks for all your help. I'll mark this as the solution but really CaptainObvious' first post on Page 1 got me going, I wouldnt have got this without that post!
The final issue I had to resolve was the fact I wasnt updating the live opportunity but rather the one in a map, so I scrapped the map and made a list of opps in the Trigger loop as I was going along, then after gathering partner/account data, looped through the opps and made the match. One less SOQL query and one less embedded loop.
trigger SetUKReseller on Opportunity (before update) {
// Need to pull off the primary partners on UK Opps for a report so this field will populate
// Reseller__c with the primary partner if there is one, or if not populate it with the account name.
// This field is inivisible to the UK and these values arent stored in the picklist.
//Create a set of Opportunity and Account Ids
//These will only be populated with 'UK Opps'
Set<Id> OppIDs = new Set<Id>();
Set<Id> AccIDs = new Set<Id>();
List<Opportunity> lOpps = new List<Opportunity>();
for (Opportunity Opp :Trigger.New) {
//only take place if type is UK Opportunity
if (Opp.RecordTypeID == '012300000004uS3AAI') {
//Add this opportunity to the set
if (!OppIDs.contains(Opp.Id)) {
OppIDs.add(Opp.Id);
}
//Add the Account ID to the set
if (!AccIDs.contains(Opp.AccountId)) {
AccIDs.add(Opp.AccountId);
}
lOpps.add(Opp);
}
}
//Continue only if there are opps in the set
if(OppIDs.size() > 0) {
//put information into maps
//Retrieve 'Account' fields:
Map<ID,Account> AFields = new Map<ID,Account>([SELECT Id, Name FROM Account WHERE Id in:AccIDs]);
//Retrieve 'Partner' fields:
Map<ID,Partner> PFields = new Map<ID,Partner>([SELECT Id, AccountToId, AccountTo.Name, OpportunityID
FROM Partner
WHERE IsPrimary = TRUE AND OpportunityID in:OppIDs]);
//Now that we have all information we'll need,
//we can finally loop through each opportunity
for(integer i = 0; i < lOpps.size();i++){
Boolean foundPartner = false;
//First see if there's a Partner
for(Partner p : PFields.Values()) {
if (p.OpportunityID==lOpps[i].id) {
lOpps[i].Reseller__c = PFields.get(p.Id).AccountTo.Name;
foundPartner = true;
break; //exit from the loop if a partner was found
}
}
//If a partner was not found, use the Account Name instead
if (foundPartner == false){
lOpps[i].Reseller__c = AFields.get(lOpps[i].AccountId).Name;
}
}
}
}
All Answers
Sure can, was going to but didnt know if you were allowed to. Assumed you could but I know some boards which are funny about it:
Trigger:
trigger SetUKReseller on Opportunity (before update) {
// Need to pull off the primary partners on UK Opps for a report so this field will populate
// Reseller__c with the primary partner if there is one, or if not populate it with the account name.
// This field is inivisible to the UK and these values arent stored in the picklist.
private Partner ptnr;
for (Opportunity Opp :Trigger.New) {
// first check the record type, Only apply to UK Opp
if (Opp.RecordTypeID == '012300000004uS3')
{
SetUKReseller_Batch batch = new SetUKReseller_Batch(Opp.ID);
//if job not running, queue a new job
Integer runningJobs = [SELECT count() FROM AsyncApexJob WHERE (status = 'Queued' OR status = 'Processing') AND ApexClass.Name = 'SetUKReseller_Batch'];
if(runningJobs < 1) id batchinstanceid = database.executeBatch(batch, 19);
}
}
}
Class:
global class SetUKReseller_Batch implements Database.Batchable<sObject>{
//Allows SetUKReseller to update opportunities without hitting SOQL limit
private String query;
private String OppID;
private Partner ptnr;
global SetUKReseller_Batch(String OppID){
this.OppID = OppID;
}
global Database.QueryLocator start(Database.BatchableContext BC){
query = 'SELECT Id,Reseller__c FROM Account WHERE ID = :'+OppID+' LIMIT 1';
return Database.getQueryLocator(query);
}
global void execute(Database.BatchableContext BC, List<Sobject> scope){
for(sobject sOpp : scope){
try{
ptnr = [SELECT Id,AccountToID FROM Partner WHERE IsPrimary = TRUE AND OpportunityID = :OppID LIMIT 1];
}catch(Exception e){
// no need to do anything, just means no primary partner exists.
// this is dealt with below.
}
if(ptnr != NULL){
Account acnt = [SELECT Id, Name FROM Account WHERE Id = :ptnr.AccountToID];
sOpp.put('Reseller__c',acnt.Name);
}else{
ID accID = (ID)(sOpp.get('AccountID'));
Account acnt = [SELECT Id, Name FROM Account WHERE Id = :accID];
sOpp.put('Reseller__c',acnt.Name);
}
}
}
global void finish(Database.BatchableContext BC){}
}
you get the error because you are querying the data inside the for loop....as soon as the counter reaches 20 it throws the error... so avoid the query inside the for loop....
yes im querying AsyncApexJob inside the trigger loop but surely I have to? Otherwise how will the count value get updated for my test?
Thanks
Is this a typo?
query = 'SELECT Id,Reseller__c FROM Account WHERE ID = :'+OppID+' LIMIT 1';
perhaps you meant FROM Opportunity?
Anyway, as Shailesh mentioned, you can run your queries outside of loops and you can implement this without batch apex.
The following is not tested (may need some tweaking), but I hope it illustrates the idea:
trigger SetUKReseller on Opportunity (before update) { // Need to pull off the primary partners on UK Opps for a report so this field will populate // Reseller__c with the primary partner if there is one, or if not populate it with the account name. // This field is inivisible to the UK and these values arent stored in the picklist. //Create a set of Opportunity and Account Ids //These will only be populated with 'UK Opps' Set<Id> OppIDs = new Set<Id>(); Set<Id> AccIDs = new Set<Id>(); for (Opportunity Opp :Trigger.New) { /* You may want to add a condition where if the reseller has already been set, * dont attempt to re-set it. Otherwise, this trigger will run every time you * edit the opportunity. * Also, you should probably retrieve the recordtypeid dynamically rather than * hardcoding it. */ if (Opp.RecordTypeID == '012300000004uS3') { //Add this opportunity to the set if (!OppIDs.contains(Opp.Id)) { OppIDs.add(Opp.Id); } //Add the Account ID to the set if (!AccIDs.contains(Opp.AccountId)) { AccIDs.add(Opp.AccountId); } } } //Continue only if there are opps in the set if(OppIDs.size() > 0) { /* Retrieve information you may need into maps * You can then iterate through the maps without additional queries * (Notice how this is done outside any loop) */ //Retrieve 'Account' fields: Map<ID,Account> AFields = new Map<ID,Account>([ SELECT Id, Name FROM Account WHERE Id in:AccIDs]); //Retrieve 'Partner' fields: Map<ID,Partner> PFields = new Map<ID,Partner>([ SELECT Id, AccountToId, AccountTo.Name, OpportunityID FROM Partner WHERE IsPrimary = TRUE AND OpportunityID in:OppIDs]); //Retrieve 'Opportunity' fields: Map<ID,Opportunity> OFields = new Map<ID,Opportunity>([ SELECT Id, Reseller__c, AccountId FROM Opportunity WHERE Id in :OppIDs]); //Now that you have all information you'll need, you can finally loop //through each opportunity: for(Opportunity opp: OFields.values()){ Integer foundPartner=0; //First see if there's a Partner for(Partner p : PFields.Values()) { //additional logic may be needed here.... if (p.OpportunityID==opp.id) { opp.reseller__c = PFields.get(p.Id).AccountTo.Name; foundPartner++; break; //exit from the loop if a partner was found } } //If a partner was not found, use the Account Name instead if (foundPartner==0){ opp.reseller__c = AFields.get(opp.AccountId).Name; } } } }
Hope that helps!
Thanks for that, looks good! I've only been using SF since December and only really been developing in it for a couple of months and I've had a bit of difficulty understanding Maps but now you've shown me a practical use for them!
I'll start changing my trigger to look like your example now, thanks!
I take it from the example that in a for (Opportunity Opp :Trigger.New) {} loop, it runs every opportunity which is to be updated, all at once before it comes out of the loop?
Also, how do you put code into those boxes? I tried [code][/code] as well as <code></code> but neither worked in mine.
Thanks again!
Hi all, having other issues now. Because I'm coming out of the loop in order to do the rest of the work, i need to go back into the loop to update the opportunity. However this means having to save a link between the opportunity and reseller string to use once im back inside the loop. But now i'm getting "too many script statement" errors when updating a bunch of opps with the dataloader, can anyone see a way of sorting that?
trigger SetUKReseller on Opportunity (before update) {
// Need to pull off the primary partners on UK Opps for a report so this field will populate
// Reseller__c with the primary partner if there is one, or if not populate it with the account name.
// This field is inivisible to the UK and these values arent stored in the picklist.
//Create a set of Opportunity and Account Ids
//These will only be populated with 'UK Opps'
Set<Id> OppIDs = new Set<Id>();
Set<Id> AccIDs = new Set<Id>();
Map<String,String> OppToReseller = new Map<String,String>();
for (Opportunity Opp :Trigger.New) {
//only take place if type is UK Opportunity
if (Opp.RecordTypeID == '012300000004uS3AAI') {
//Add this opportunity to the set
if (!OppIDs.contains(Opp.Id)) {
system.debug('STE - Opp added to set');
OppIDs.add(Opp.Id);
}
//Add the Account ID to the set
if (!AccIDs.contains(Opp.AccountId)) {
system.debug('STE - Acc added to set');
AccIDs.add(Opp.AccountId);
}
}
}
//Continue only if there are opps in the set
if(OppIDs.size() > 0) {
//put information into maps
//Retrieve 'Account' fields:
Map<ID,Account> AFields = new Map<ID,Account>([SELECT Id, Name FROM Account WHERE Id in:AccIDs]);
system.debug('STE - Acc fields retrieved');
//Retrieve 'Partner' fields:
Map<ID,Partner> PFields = new Map<ID,Partner>([SELECT Id, AccountToId, AccountTo.Name, OpportunityID
FROM Partner
WHERE IsPrimary = TRUE AND OpportunityID in:OppIDs]);
system.debug('STE - Partner fields retrieved');
//Retrieve 'Opportunity' fields:
Map<ID,Opportunity> OFields = new Map<ID,Opportunity>([SELECT Id, Reseller__c, AccountId
FROM Opportunity WHERE Id in :OppIDs]);
system.debug('STE - Opp fields retrieved');
//Now that we have all information we'll need,
//we can finally loop through each opportunity
for(Opportunity oppToTest: OFields.values()){
String ResellerField = '';
Boolean foundPartner = false;
//First see if there's a Partner
for(Partner p : PFields.Values()) {
if (p.OpportunityID==oppToTest.id) {
ResellerField = PFields.get(p.Id).AccountTo.Name;
foundPartner = true;
system.debug('STE - partner found');
break; //exit from the loop if a partner was found
}
}
//If a partner was not found, use the Account Name instead
if (foundPartner == false){
system.debug('STE - partner not found');
ResellerField = AFields.get(oppToTest.AccountId).Name;
}
OppToReseller.put(oppToTest.Id, ResellerField);
}
}
for (Opportunity Opp :Trigger.New) {
//iterate through Opp Ids to find match
for(String s : OppToReseller.keySet()){
//when we find a matching opp, set the reseller from the value
if(s == Opp.Id){
Opp.Reseller__c = OppToReseller.get(s);
}
}
}
}
Instead of
OppToReseller.put(oppToTest.Id, ResellerField);
try
oppToTest.reseller__c = ResellerField;
Because your trigger is "before update", this should be all you need to set the field.
When using this extra loop at the end
for (Opportunity Opp :Trigger.New) { ... }
you're actually iterating over every single opportunity, not just the 'UK Opps'.
No I'm definitely updating. What I've done now is made a list of Opportunities during the first trigger loop and when I get the reseller values I'm applying the values to those opps rather than those in the Maps, which seems to work.
However once again when doing a bulk update im still getting too many script errors :( Didnt realise something so simple would be so hard!
trigger SetUKReseller on Opportunity (before update) {
// Need to pull off the primary partners on UK Opps for a report so this field will populate
// Reseller__c with the primary partner if there is one, or if not populate it with the account name.
// This field is inivisible to the UK and these values arent stored in the picklist.
//Create a set of Opportunity and Account Ids
//These will only be populated with 'UK Opps'
Set<Id> OppIDs = new Set<Id>();
Set<Id> AccIDs = new Set<Id>();
Map<String,String> OppToReseller = new Map<String,String>();
List<Opportunity> lOpps = new List<Opportunity>();
for (Opportunity Opp :Trigger.New) {
//only take place if type is UK Opportunity
if (Opp.RecordTypeID == '012300000004uS3AAI') {
//Add this opportunity to the set
if (!OppIDs.contains(Opp.Id)) {
system.debug('STE - Opp added to set');
OppIDs.add(Opp.Id);
}
//Add the Account ID to the set
if (!AccIDs.contains(Opp.AccountId)) {
system.debug('STE - Acc added to set');
AccIDs.add(Opp.AccountId);
}
lOpps.add(Opp);
}
}
//Continue only if there are opps in the set
if(OppIDs.size() > 0) {
//put information into maps
//Retrieve 'Account' fields:
Map<ID,Account> AFields = new Map<ID,Account>([SELECT Id, Name FROM Account WHERE Id in:AccIDs]);
system.debug('STE - Acc fields retrieved');
//Retrieve 'Partner' fields:
Map<ID,Partner> PFields = new Map<ID,Partner>([SELECT Id, AccountToId, AccountTo.Name, OpportunityID
FROM Partner
WHERE IsPrimary = TRUE AND OpportunityID in:OppIDs]);
system.debug('STE - Partner fields retrieved');
//Retrieve 'Opportunity' fields:
Map<ID,Opportunity> OFields = new Map<ID,Opportunity>([SELECT Id, Reseller__c, AccountId
FROM Opportunity WHERE Id in :OppIDs]);
system.debug('STE - Opp fields retrieved');
//Now that we have all information we'll need,
//we can finally loop through each opportunity
for(Opportunity oppToTest: OFields.values()){
String ResellerField = '';
Boolean foundPartner = false;
//First see if there's a Partner
for(Partner p : PFields.Values()) {
if (p.OpportunityID==oppToTest.id) {
ResellerField = PFields.get(p.Id).AccountTo.Name;
foundPartner = true;
system.debug('STE - partner found');
break; //exit from the loop if a partner was found
}
}
//If a partner was not found, use the Account Name instead
if (foundPartner == false){
system.debug('STE - partner not found');
ResellerField = AFields.get(oppToTest.AccountId).Name;
}
for(integer i = 0; i < lOpps.size();i++){
if(lOpps[i].Id == oppToTest.Id){
lOpps[i].Reseller__c = ResellerField;
}
}
}
}
}
Managed to sort it, hurrah!! Is there a more fulfilling feeling than sorting something that's taken foreveeeeer??
Here's my finished trigger, thanks for all your help. I'll mark this as the solution but really CaptainObvious' first post on Page 1 got me going, I wouldnt have got this without that post!
The final issue I had to resolve was the fact I wasnt updating the live opportunity but rather the one in a map, so I scrapped the map and made a list of opps in the Trigger loop as I was going along, then after gathering partner/account data, looped through the opps and made the match. One less SOQL query and one less embedded loop.
trigger SetUKReseller on Opportunity (before update) {
// Need to pull off the primary partners on UK Opps for a report so this field will populate
// Reseller__c with the primary partner if there is one, or if not populate it with the account name.
// This field is inivisible to the UK and these values arent stored in the picklist.
//Create a set of Opportunity and Account Ids
//These will only be populated with 'UK Opps'
Set<Id> OppIDs = new Set<Id>();
Set<Id> AccIDs = new Set<Id>();
List<Opportunity> lOpps = new List<Opportunity>();
for (Opportunity Opp :Trigger.New) {
//only take place if type is UK Opportunity
if (Opp.RecordTypeID == '012300000004uS3AAI') {
//Add this opportunity to the set
if (!OppIDs.contains(Opp.Id)) {
OppIDs.add(Opp.Id);
}
//Add the Account ID to the set
if (!AccIDs.contains(Opp.AccountId)) {
AccIDs.add(Opp.AccountId);
}
lOpps.add(Opp);
}
}
//Continue only if there are opps in the set
if(OppIDs.size() > 0) {
//put information into maps
//Retrieve 'Account' fields:
Map<ID,Account> AFields = new Map<ID,Account>([SELECT Id, Name FROM Account WHERE Id in:AccIDs]);
//Retrieve 'Partner' fields:
Map<ID,Partner> PFields = new Map<ID,Partner>([SELECT Id, AccountToId, AccountTo.Name, OpportunityID
FROM Partner
WHERE IsPrimary = TRUE AND OpportunityID in:OppIDs]);
//Now that we have all information we'll need,
//we can finally loop through each opportunity
for(integer i = 0; i < lOpps.size();i++){
Boolean foundPartner = false;
//First see if there's a Partner
for(Partner p : PFields.Values()) {
if (p.OpportunityID==lOpps[i].id) {
lOpps[i].Reseller__c = PFields.get(p.Id).AccountTo.Name;
foundPartner = true;
break; //exit from the loop if a partner was found
}
}
//If a partner was not found, use the Account Name instead
if (foundPartner == false){
lOpps[i].Reseller__c = AFields.get(lOpps[i].AccountId).Name;
}
}
}
}