Sign Up ›

Browse by Topic
ShowAll Questionssorted byDate Posted
+ Start a Discussion
crikketlangcrikketlang 

Too many SOQL queries: 101

I know these have been posted a million times and I'm sorry to post another one but I cant seem to wrap my head around where the loop is happening here. I know it's a bad idea to put the queries into the loop, but i dont feel the loop should even be looping on itself to cause the 101 error. 

I am posting the complete code for the trigger in hopes someone can point out the issue. As of now pretty much any batch update im doing is failing due to this little bugger. I know there is a better way to do this.

Any help is appreciated.

trigger relationshipHierarchyTrigger on Account (after update) {
  for(Account acct : Trigger.new){
    String recType = arupFunctions.getRecordTypeName(acct.recordtypeid);
    if( recType == 'Health System' || recType == 'Joint Venture' || recType == 'VISN' ){
          Account oldAcct = Trigger.oldMap.get(acct.ID);    
          Account newAcct = Trigger.newMap.get(acct.ID);
          
          //If the GPO has changed, change all the accounts that
          // are part of the health system to that GPO
          if(oldAcct.GPO_Primary__c != newAcct.GPO_Primary__c || oldAcct.Alliance__c != newAcct.Alliance__c){
        List<Account> accts = [SELECT a.Id, a.GPO_Primary__c, a.Name
                FROM Account a WHERE a.Health_System__c = :acct.ID];
        if(accts != null){
          for (Integer i = 0; i < accts.size(); i++){
            accts[i].GPO_Primary__c = acct.GPO_Primary__c;
            accts[i].Alliance__c = acct.Alliance__c;
          }
          update(accts);
        }
          }
    } else if( recType == 'Alliance' || recType == 'Group' ){
          Account oldAcct = Trigger.oldMap.get(acct.ID);    
          Account newAcct = Trigger.newMap.get(acct.ID);

          if(oldAcct.GPO_Primary__c != newAcct.GPO_Primary__c){
        List<Account> accts = [SELECT a.Id, a.GPO_Primary__c, a.Name
                FROM Account a WHERE a.Alliance__c = :acct.ID];
        if(accts != null){
          for (Integer i = 0; i < accts.size(); i++){
            accts[i].GPO_Primary__c = acct.GPO_Primary__c;
          }
          update(accts);
        }
          }
    }
  }    
}

 

Best Answer chosen by Admin (Salesforce Developers) 
Sean TanSean Tan

I can't tell you 100% since the rest of the code is bulk safe but I would look specifically at this line:

 

String recType = arupFunctions.getRecordTypeName(acct.recordtypeid);

 

I assume this is doing a query to the database to get the record type name, therefore is probably querying for each account. You'll need to make this bulk safe... I can't give you exact details since I can't see the code but something like this for the method would work (Haven't tested it out myself):

 

trigger relationshipHierarchyTrigger on Account (after update) {
    if (!arupFunctions.isTriggerRunning) // make sure we enter this code only once
    {    
        arupFunctions.makeAlreadyUpdated();
        
        Map<Id, Account> changedPrimaryAllianceAccounts = new Map<Id, Account>{};
        Map<Id, Account> changedPrimaryAccounts = new Map<Id, Account>{};
        Map<Id, String> recordTypeNameMap = new Map<Id, String>{};
        for(Account acct : Trigger.new){
            recordTypeNameMap.put(acct.RecordTypeId, null);            
        }
        
        for (RecordType rt : [ SELECT Id, Name FROM RecordType WHERE Id IN :recordTypeNameMap.keySet())
        {
            recordTypeNameMap.put(rt.Id, rt.Name);
        }
        
        for (Account acct : Trigger.new) {
            String recType = recordTypeNameMap.get(acct.recordtypeid);
            if( recType == 'Health System' || recType == 'Joint Venture' || recType == 'VISN' ){
                Account oldAcct = Trigger.oldMap.get(acct.ID);                
                
                //If the GPO has changed, change all the accounts that
                // are part of the health system to that GPO
                if(oldAcct.GPO_Primary__c != acct.GPO_Primary__c || oldAcct.Alliance__c != acct.Alliance__c){
                    changedPrimaryAccounts.put(acct.Id, acct);                
                }
            } 
            else if( recType == 'Alliance' || recType == 'Group' ) {
                Account oldAcct = Trigger.oldMap.get(acct.ID);                
                
                if(oldAcct.GPO_Primary__c != acct.GPO_Primary__c) {
                    changedPrimaryAllianceAccounts.put(acct.Id, acct);                
                }
            }
        }
        
        Map<Id, Account> accountsToUpdate = new Map<Id, Account>{};
        
        for (Account a : [SELECT a.Id, a.GPO_Primary__c, a.Name, a.Health_System__c FROM Account a WHERE a.Health_System__c = :changedPrimaryAllianceAccounts.keySet()])
        {
            Account newInfo = changedPrimaryAllianceAccounts.get(a.Id);
            a.GPO_Primary__c = newInfo.GPO_Primary__c;
            a.Alliance__c = newInfo.Alliance__c;
            accountsToUpdate.put(a.Id, a);
        }
        
        for (Account a : [SELECT a.Id, a.GPO_Primary__c, a.Name, a.Health_System__c FROM Account a WHERE a.Health_System__c = :changedPrimaryAccounts.keySet()])
        {
            if (!accountsToUpdate.containsKey(a.Id))
            {
                Account newInfo = changedPrimaryAccounts.get(a.Id);
                a.GPO_Primary__c = newInfo.GPO_Primary__c;            
                accountsToUpdate.put(a.Id, a);
            }
        }
        
        if (!accountsToUpdate.isEmpty())
        {
            update accountsToUpdate.values();
        }
    }
}

 

 

All Answers

SurpriseSurprise

Are u trying to say your code is running recursively?.If that is the case then you have to use flag to ger rid of the recursion.

 

 
Sean TanSean Tan

Well couple things jump at me glaringinly, first is the record type function that you've written to get the name of the records type. Make sure that's not doing a query for every loop.

The next are the ones inside the for loops when you're changing the target accounts values for. You want to build a list of records to get the updates for and process it in a couple of SOQL statements... try this:

 

trigger relationshipHierarchyTrigger on Account (after update) {
    if (!arupFunctions.isRunningTrigger)
    {    
        arupFunctions.isRunningTrigger = true;
        
        Map<Id, Account> changedPrimaryAllianceAccounts = new Map<Id, Account>{};
        Map<Id, Account> changedPrimaryAccounts = new Map<Id, Account>{};
        
        for(Account acct : Trigger.new){
            String recType = arupFunctions.getRecordTypeName(acct.recordtypeid);
            if( recType == 'Health System' || recType == 'Joint Venture' || recType == 'VISN' ){
                Account oldAcct = Trigger.oldMap.get(acct.ID);                
                
                //If the GPO has changed, change all the accounts that
                // are part of the health system to that GPO
                if(oldAcct.GPO_Primary__c != acct.GPO_Primary__c || oldAcct.Alliance__c != acct.Alliance__c){
                    changedAccounts.put(acct.Id, acct);                
                }
            } 
            else if( recType == 'Alliance' || recType == 'Group' ) {
                Account oldAcct = Trigger.oldMap.get(acct.ID);                
                
                if(oldAcct.GPO_Primary__c != acct.GPO_Primary__c) {
                    changedPrimaryAccounts.put(acct.Id, acct);                
                }
            }
        }
        
        Map<Id, Account> accountsToUpdate = new Map<Id, Account>{};
        
        for (Account a : [SELECT a.Id, a.GPO_Primary__c, a.Name, a.Health_System__c FROM Account a WHERE a.Health_System__c = :changedPrimaryAllianceAccounts.keySet()])
        {
            Account newInfo = changedPrimaryAllianceAccounts.get(a.Id);
            a.GPO_Primary__c = newInfo.GPO_Primary__c;
            a.Alliance__c = newInfo.Alliance__c;
            accountsToUpdate.put(a.Id, a);
        }
        
        for (Account a : [SELECT a.Id, a.GPO_Primary__c, a.Name, a.Health_System__c FROM Account a WHERE a.Health_System__c = :changedPrimaryAccounts.keySet()])
        {
            //Doing this check as I assume the above processing takes priority over this... correct as you feel fit
            if (!accountsToUpdate.containsKey(a.Id))
            {
                Account newInfo = changedPrimaryAccounts.get(a.Id);
                a.GPO_Primary__c = newInfo.GPO_Primary__c;            
                accountsToUpdate.put(a.Id, a);
            }
        }
        
        if (!accountsToUpdate.isEmpty())
        {
            update accountsToUpdate.values();
        }
    }
}

 This can probably still be optimized a bit more... but this should get you in the right direction on making it bulk safe. This was also written on the fly so you may have to change things to get it to compile properly.

 

You will also, want to make sure since you're updating accounts, that you want to break out if you're already running. To do that you'll need to set a static property somewhere to break out later on subsequent calls...
crikketlangcrikketlang

Ok. Sorry for the delay, Had a few meetings to attend and such. So i went and fixed the minor changes in the code you suggested as well as added the recursive catch stuff into that functions class but i am still getting the same error. below you can find the code for both the class and trigger. This one is really a pain. lol

Class:

private static Boolean isTriggerRunning = false; // RSH this was added to enable controlling recursion on triggers
    
    public static Boolean hasAlreadyUpdated(){ // return if the trigger has already fired
      return isTriggerRunning;
    }
    
    public static void makeAlreadyUpdated(){ // we only allow it to be set to true for one instance
      isTriggerRunning = true;
    }

 Trigger:

trigger relationshipHierarchyTrigger on Account (after update) {
    if (!arupFunctions.isTriggerRunning) // make sure we enter this code only once
    {    
        arupFunctions.makeAlreadyUpdated();
        
        Map<Id, Account> changedPrimaryAllianceAccounts = new Map<Id, Account>{};
        Map<Id, Account> changedPrimaryAccounts = new Map<Id, Account>{};
        
        for(Account acct : Trigger.new){
            String recType = arupFunctions.getRecordTypeName(acct.recordtypeid);
            if( recType == 'Health System' || recType == 'Joint Venture' || recType == 'VISN' ){
                Account oldAcct = Trigger.oldMap.get(acct.ID);                
                
                //If the GPO has changed, change all the accounts that
                // are part of the health system to that GPO
                if(oldAcct.GPO_Primary__c != acct.GPO_Primary__c || oldAcct.Alliance__c != acct.Alliance__c){
                    changedPrimaryAccounts.put(acct.Id, acct);                
                }
            } 
            else if( recType == 'Alliance' || recType == 'Group' ) {
                Account oldAcct = Trigger.oldMap.get(acct.ID);                
                
                if(oldAcct.GPO_Primary__c != acct.GPO_Primary__c) {
                    changedPrimaryAllianceAccounts.put(acct.Id, acct);                
                }
            }
        }
        
        Map<Id, Account> accountsToUpdate = new Map<Id, Account>{};
        
        for (Account a : [SELECT a.Id, a.GPO_Primary__c, a.Name, a.Health_System__c FROM Account a WHERE a.Health_System__c = :changedPrimaryAllianceAccounts.keySet()])
        {
            Account newInfo = changedPrimaryAllianceAccounts.get(a.Id);
            a.GPO_Primary__c = newInfo.GPO_Primary__c;
            a.Alliance__c = newInfo.Alliance__c;
            accountsToUpdate.put(a.Id, a);
        }
        
        for (Account a : [SELECT a.Id, a.GPO_Primary__c, a.Name, a.Health_System__c FROM Account a WHERE a.Health_System__c = :changedPrimaryAccounts.keySet()])
        {
            if (!accountsToUpdate.containsKey(a.Id))
            {
                Account newInfo = changedPrimaryAccounts.get(a.Id);
                a.GPO_Primary__c = newInfo.GPO_Primary__c;            
                accountsToUpdate.put(a.Id, a);
            }
        }
        
        if (!accountsToUpdate.isEmpty())
        {
            update accountsToUpdate.values();
        }
    }
}

 

 
Sean TanSean Tan

I can't tell you 100% since the rest of the code is bulk safe but I would look specifically at this line:

 

String recType = arupFunctions.getRecordTypeName(acct.recordtypeid);

 

I assume this is doing a query to the database to get the record type name, therefore is probably querying for each account. You'll need to make this bulk safe... I can't give you exact details since I can't see the code but something like this for the method would work (Haven't tested it out myself):

 

trigger relationshipHierarchyTrigger on Account (after update) {
    if (!arupFunctions.isTriggerRunning) // make sure we enter this code only once
    {    
        arupFunctions.makeAlreadyUpdated();
        
        Map<Id, Account> changedPrimaryAllianceAccounts = new Map<Id, Account>{};
        Map<Id, Account> changedPrimaryAccounts = new Map<Id, Account>{};
        Map<Id, String> recordTypeNameMap = new Map<Id, String>{};
        for(Account acct : Trigger.new){
            recordTypeNameMap.put(acct.RecordTypeId, null);            
        }
        
        for (RecordType rt : [ SELECT Id, Name FROM RecordType WHERE Id IN :recordTypeNameMap.keySet())
        {
            recordTypeNameMap.put(rt.Id, rt.Name);
        }
        
        for (Account acct : Trigger.new) {
            String recType = recordTypeNameMap.get(acct.recordtypeid);
            if( recType == 'Health System' || recType == 'Joint Venture' || recType == 'VISN' ){
                Account oldAcct = Trigger.oldMap.get(acct.ID);                
                
                //If the GPO has changed, change all the accounts that
                // are part of the health system to that GPO
                if(oldAcct.GPO_Primary__c != acct.GPO_Primary__c || oldAcct.Alliance__c != acct.Alliance__c){
                    changedPrimaryAccounts.put(acct.Id, acct);                
                }
            } 
            else if( recType == 'Alliance' || recType == 'Group' ) {
                Account oldAcct = Trigger.oldMap.get(acct.ID);                
                
                if(oldAcct.GPO_Primary__c != acct.GPO_Primary__c) {
                    changedPrimaryAllianceAccounts.put(acct.Id, acct);                
                }
            }
        }
        
        Map<Id, Account> accountsToUpdate = new Map<Id, Account>{};
        
        for (Account a : [SELECT a.Id, a.GPO_Primary__c, a.Name, a.Health_System__c FROM Account a WHERE a.Health_System__c = :changedPrimaryAllianceAccounts.keySet()])
        {
            Account newInfo = changedPrimaryAllianceAccounts.get(a.Id);
            a.GPO_Primary__c = newInfo.GPO_Primary__c;
            a.Alliance__c = newInfo.Alliance__c;
            accountsToUpdate.put(a.Id, a);
        }
        
        for (Account a : [SELECT a.Id, a.GPO_Primary__c, a.Name, a.Health_System__c FROM Account a WHERE a.Health_System__c = :changedPrimaryAccounts.keySet()])
        {
            if (!accountsToUpdate.containsKey(a.Id))
            {
                Account newInfo = changedPrimaryAccounts.get(a.Id);
                a.GPO_Primary__c = newInfo.GPO_Primary__c;            
                accountsToUpdate.put(a.Id, a);
            }
        }
        
        if (!accountsToUpdate.isEmpty())
        {
            update accountsToUpdate.values();
        }
    }
}

 

 

This was selected as the best answer
crikketlangcrikketlang

Whats funny is that we JUST found that GetRecordTypeName function and it was COMPLETELY wrong and running over and over and over again in the debug logs. We tested it by replacing it with just a map inside the trigger and the trigger worked perfectly. So the issue was with that and not really anything in the trigger itself (so to speak)

We are just finishing up the changes to that function in that class and then ill post the wrong code + the right code just in case anyone else wants to take a peek. 

Thank  you SO much for your help. This thing was really getting on my nerves and now i can go home and drink a beer in peace. Cheers good sir!
RocketRocket

Hey cricketlang,

 

Don't forget to put your final code and the one which was not working with recordtypes.
RocketRocket

 

Hey cricketlang,

 

What does the a.Health_system__c  after the where  in the below given query signifies or stands for ,Can u please explain?. 

 

for (Account a : [SELECT a.Id, a.GPO_Primary__c, a.Name, a.Health_System__c FROM Account a WHERE a.Health_System__c = :changedPrimaryAccounts.keySet()])
crikketlangcrikketlang

My apologies for the delay. Been pretty busy over here. Below you will find the code that is not so great, and the updated code that works. Note that this is just a function inside a shared class (arupFunctions) so its just the relevant piece to this thread. 

 

Code that was causing massive SOQL queries:

  global static String getRecordTypeName( String s ){
        String recordName = '';
        RecordType[] rec = [select id, Name from RecordType where id=:s];
        if( rec != null && rec.size() > 0 ){
            recordName = rec[0].Name;
        }
        return recordName;
    }

 
Updated code that utilizes maps. Note that I null out recordTypeMap before i create a new one:

private static Map<String,String> recordTypeMap = null;

global static String getRecordTypeName( String s ){ 
      if( recordTypeMap == null ){
        recordTypeMap = new Map<String,String>();
        RecordType[] typeList = [select id, Name from RecordType];
        for( RecordType rt: typeList )
          recordTypeMap.put( rt.Id, rt.Name );
      }
        String recordName = '';
        recordName = recordTypeMap.get( s );
        return recordName;
    }

 
crikketlangcrikketlang

Health_System__c is simply a field on the account level that we are pulling in. The trigger itself looks for a change in the GPO_Primary__c field, then takes that change (if it exists) and performs an update for any other records that have the same value in the Health_System__c field. 

So if the GPO gets changed on one account, we change it for every account under the same Health System. (Because the health system is all under the same GPO. If the gpo changes for one account under a health system, all the other accounts under that health system need to be updated too) The trigger just automates the change for all the other accounts.

Is that what you were looking to know? This code could easily be reused for anyone looking to change the value for multiple records when its updated on one that is related somehow (even if just by a picklist value and not an actual related object)