function readOnly(count){ }
Sign Up ›
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
Learn More

Browse by Topic
ShowAll Questionssorted byDate Posted
+ Start a Discussion
HARSHIL U PARIKHHARSHIL U PARIKH 

SOQL Query question for best practice

Hello Developers!

I have a trigger on contact which rollsup number of contact on account.
Trigger works fine but however I have a question.
Trigger Code: 
Trigger ContacstTotalCount On Contact(After Insert, After Update, After Delete, After Undelete){
    
    Set<ID> AccountIds = New Set<ID>();
    
    If(Trigger.IsInsert || Trigger.IsUpdate || Trigger.IsUndelete){
        For(Contact C:Trigger.New){
            AccountIds.add(C.AccountID);
        }
    }
    If(Trigger.IsDelete){
        For(Contact C:Trigger.Old){
            AccountIds.add(C.AccountID);
        }
    }
    
    List<Account> AccountListToUpdate = New List<Account>();
    
    For(Account a: [Select Id, total_contacts__c, (Select Id FROM Contacts) FROM Account WHERE ID = :AccountIds]){
        
        a.total_contacts__c = a.contacts.size();
        AccountListToUpdate.add(a);
    }
    try{
    Update AccountListToUpdate;
    }
    Catch(Exception E){
    System.Debug('Thrown Exception is: ' + E.getMessage());
    }
    
}

In line 18 I am having SOQL in parameters of FOR loop. Would that be okay or I should be write something like following? 
List<Account> FetchingActs = [Select Id, total_contacts__c, (Select Id FROM Contacts) FROM Account WHERE ID = :AccountIds];
    
    For(Account a: FetchingActs ){
        
        a.total_contacts__c = a.contacts.size();
        AccountListToUpdate.add(a);
    }
I mean... if I take the above approach then wouldn't it only work for first 50,000 Accounts?
And also what happens when each account has 5 contacts then would query fetch 50,000 accounts plus 5 contact from each?
Thank You!
 
Best Answer chosen by HARSHIL U PARIKH
Eric PepinEric Pepin
If you know that you need to query/update more than 50K rows, then you may want to try a Batch Job.
When running Scheduled Apex, you can specify a batch value (e.g. 200) and Salesforce will process all records in the specified batch size.
This will allow you to work with large sets of data and avoid Salesforce limits.

In your trigger, you will only be processing the Accounts that are being inserted/updated/deleted.
There will be no way to get around the limitations in this context if you are indeed trying to query more than 50K accounts.
Also, in this particular case, the 'After Update' trigger context is not necessary for this code as performing updates will not change the count of Contacts on an Account.
That can be removed from the trigger declaration if it is not needed elsewhere.
Code example below. (untested, but very similar to your original code)
  
trigger ContactsTotalCount On Contact(after insert, after delete, after undelete) {
    
    Set<Id> accountIds = New Set<Id>();
    for(Contact c : Trigger.IsDelete ? Trigger.Old : Trigger.New) {
        accountIds.add(c.AccountId);
    }
    
    List<Account> accountListToUpdate = New List<Account>();
    for(Account a : [SELECT Id, Total_Contacts__c, (SELECT Id FROM Contacts) FROM Account WHERE Id = :accountIds]) {
        a.Total_Contacts__c = a.Contacts.size();
        accountListToUpdate.add(a);
    }
    
    try{
        if(!accountListToUpdate.isEmpty()) {
        	update accountListToUpdate;    
        }
    }
    catch(Exception e) {
    	system.debug('Thrown Exception is: ' + e.getMessage());
    }
}

 

All Answers

Eric PepinEric Pepin
The first option (SOQL directly in FOR loop without explicitly creating List) is preferred as it uses less Heap. The number of Accounts updated will depend on the number of Accounts passed into Trigger. If you are updating 50,000 records at a time, you may run into problems with Limits, but any limit issues will have nothing to do with the decision of where to put the SOQL query.
HARSHIL U PARIKHHARSHIL U PARIKH
The reason I asked this question is because I had a trigger on child which updates 4 different fields on parent and I was taking above approach which is to put SOQL in the parameters of FOR loop and it was throwing me an error when I update the actual child record.

So,, then wrote the SOQL outside of loop and it resolved and I am not sure why or how?

I am thinking that if I have SOQL in the parameter of FOR loop then it would fetch only 200 record Is it correct?
Or it would keep fetching 200 records over and over until unless it reaches to the all records?
 
HARSHIL U PARIKHHARSHIL U PARIKH
Another Question,

Let's say I have 100,000 Accounts in instance and each of them have 10 Contacts under their related list means total 100,000 Accounts and 1,000,000 Contacts.
Now If I have a query like below, 
List<Account> FetchActs = [Select Id, Name, (Select Id FROM Contact) FROM Account];
what's the output of query / What does the query fetch? 50,000 accounts and 500,000 Contacts?
 
Eric PepinEric Pepin
I believe that the query will attempt to return all Accounts (100,000) and all Contacts (1,000,000) within the Apex code...and will most likely throw an exception: System.LimitException: Too many query rows: 50001. (unless you specifically add a LIMIT clause to you SOQL query)
HARSHIL U PARIKHHARSHIL U PARIKH
Thank you for the replay and knowledge!
But then what's the best practic to write above trigger,

Should I put SOQL inside the parameter of FOR loop and define the limit? If yes, then how?
Or I should take the second approach where I am defining SOQL outside of the FOR loop and set the limit at the end like LIMIT 50,000
Even if I set the limit how would SOQL Query know that there are still another 50,000 pending accounts left and I need to get them as well?
Thank You very much!
Eric PepinEric Pepin
If you know that you need to query/update more than 50K rows, then you may want to try a Batch Job.
When running Scheduled Apex, you can specify a batch value (e.g. 200) and Salesforce will process all records in the specified batch size.
This will allow you to work with large sets of data and avoid Salesforce limits.

In your trigger, you will only be processing the Accounts that are being inserted/updated/deleted.
There will be no way to get around the limitations in this context if you are indeed trying to query more than 50K accounts.
Also, in this particular case, the 'After Update' trigger context is not necessary for this code as performing updates will not change the count of Contacts on an Account.
That can be removed from the trigger declaration if it is not needed elsewhere.
Code example below. (untested, but very similar to your original code)
  
trigger ContactsTotalCount On Contact(after insert, after delete, after undelete) {
    
    Set<Id> accountIds = New Set<Id>();
    for(Contact c : Trigger.IsDelete ? Trigger.Old : Trigger.New) {
        accountIds.add(c.AccountId);
    }
    
    List<Account> accountListToUpdate = New List<Account>();
    for(Account a : [SELECT Id, Total_Contacts__c, (SELECT Id FROM Contacts) FROM Account WHERE Id = :accountIds]) {
        a.Total_Contacts__c = a.Contacts.size();
        accountListToUpdate.add(a);
    }
    
    try{
        if(!accountListToUpdate.isEmpty()) {
        	update accountListToUpdate;    
        }
    }
    catch(Exception e) {
    	system.debug('Thrown Exception is: ' + e.getMessage());
    }
}

 
This was selected as the best answer
HARSHIL U PARIKHHARSHIL U PARIKH
So, In my original question above (the first one at the top), it doesn't matter which approach I would take right?
Whether I write SOQL outside the FOR or inside the FOR It would throw an error if I am inserting 51,000 contacts for single Account correct?
Eric PepinEric Pepin
Your original question was about best practice, and limits aside, the best option is the first option because it uses less Heap.
If we are talking about Limits, then neither approach will allow to circumvent the 50K query row limit.
HARSHIL U PARIKHHARSHIL U PARIKH
Thank you Eric for the valuable information!