1

I am currently in the process of bulkifying some trigger code. On a high level, I receive a list of Contact objects, then I need to find the Account associated with it, and count the number of primary contacts that Account has.

This was the original implementation:

public class ContactTriggerHelper {
    public static void AllocatePrimaryContactOnInsert(List<Contact> contacts) {
        List<Account> accountsToUpdate = new List<Account>();
        for(Contact contact: contacts) {
            if(contact.AccountId != null) {
                Account account = [select Id from Account where Id = :contact.AccountId];
                integer count1 = [ select count() from Contact where Primary__c = true and AccountId = :contact.accountId ];
                integer count2 = [ select count() from AccountContactRole where IsPrimary = true and AccountContactRole.AccountId = :contact.accountid ];
                if(count1 + count2 == 0) {
                    //Do some stuff to the account
                    accountsToUpdate.add(account);
                }
            }
        }
        update accountsToUpdate;
    }
}

Obviously, this runs into governor limits very quickly, so I'm trying to bulkify the queries by getting everything into one query. This is how far I've gotten so far:

public static void AllocatePrimaryContactOnInsert(List<Contact> contacts, Map<Id,Contact> newMap) {
        List<Contact> contactInfo = [select Id, 
                                        (select Id from Account),
                                        (select count() from Contact where Primary__c = true) count1,
                                        (select count() from AccountContactRole where IsPrimary = true) count2
                                     from Contact
                                     where Id in :newMap.keySet()
                                    ];
...
}

Unfortunately, I'm running into a roadblock here where I don't know the next step. I don't even know if I'm getting the account correctly, and how do I get the counts from the Contact and AccountContactRole tables properly? I would have naively assumed that I would need to have something like (select count() from Contact where Primary__c = true and AccountId = :acc.Id) but I don't have access to the account's Id from the subquery.

Thank you!

2 Answers 2

3

First thing to note

Right away, the isPrimary field on AccountContactRelation (ACR for short) is not writable. It is automatically set, and becomes true if the AccountId on the ACR record is the same as the AccountId field on the related Contact.

An ACR record is automatically created (if a suitable ACR doesn't already exist) when you set or update the AccountId field on the Contact.

Your count on that one will always be 1 or 0.

On using aggregate functions in subqueries

Salesforce does not allow us to do that.

Since a subquery in SOQL returns a List<SObject> embedded in each result of the outer query, you could get counts by calling the .size() method on that embedded list, but that's not safe to do (it'll fail if there are enough child records to warrant an interal call to queryMore())

The safe way to deal with subquery results is a nested loop. The outer loop iterates over the parent object, and the inner loop iterates over the related child records. You can then increment a counter there (in the inner loop).

You also likely need to consider more than just the Contacts you're given

If I had to guess, I'd say that you're not going to be working on all of the contacts in your org in a single shot. I'd imagine that you'd want to consider all of the contacts related to the Accounts you happen to encounter though.

You probably want to figure out all of the Accounts you need to worry about, and then structure your query around those account ids.

As you continue working on the Salesforce platform, you'll start to see that a lot of things can be described as using the loop-gather-query pattern

// Declare something so we can store data
Set<Id> targetIds = new Set<Id>();

// Iterate over a collection
for(Contact c :trigger.new){
    // Gather your desired information
    targetIds.add(c.AccountId);
}

// and finally, perform your query based on the information you gathered
for(Account a :[SELECT Id, Name FROM Account WHERE Id IN :targetIds]){
    // fancy work done here
}

Putting that together to form a recommendation

I don't think that you want to base your query around the Contact object. Instead, I think that an aggregate query on AccountContactRelation is likely what you're looking for.

It doesn't use subqueries, but taking this approach removes a lot of the manual work involved in counting things (and foists that work onto SOQL instead) at the cost of being slightly less pleasant to work with the resulting data.

That query might look something like this:

List<AggregateResult> arList = [SELECT AccountId, COUNT(Id) primaryCount
FROM AccountContactRelation
WHERE (IsPrimary = true OR Contact.Primary__c = true) AND AccountId IN :accountIds
GROUP BY AccountId];

I think AccountContactRelation is the SObject to concern yourself with here because it allows you access to data from both Contact and Account.

The WHERE IsPrimary = true OR Contact.Primary__c = true part helps ensure that you aren't double-counting records (in your old approach, a contact with Primary__c set to true with AccountId set as well could contribute twice to the primary contact count).

Bonus

One of the common failure modes people encounter when trying to do manual rollup summaries like this is that the count (or sum, or whatever aggregate function you're using) won't update when the final child record(s) are removed from the parent record.

If a given account has no related ACR records, then the query won't return any ACR records for that Account (which makes sense).

The solution I use for that is to assume that every parent record Id I come across should have a default value set for it (typically 0). If there are child records that contribute to a count, we'll override that default '0' value before updating the parent.

Map<Id, Account> accountsToUpdate = new Map<Id, Account>();

// Now our loop/gather part of the solution serves a secondard purpose
for(Contact c :trigger.new){
    accountsToUpdate.put(c.AccountId, new Account(Id = c.AccountId, My_Count__c = 0));
}

// get rid of any null records so they don't cause issues
accountsToUpdate.remove(null);

for(AggregateResult ar :[<aggregateResult query here>]){
    Account result = new Account(
        Id = (Id)ar.get('AccountId');
        My_Count__c = (Decimal)ar.get('<alias of aggregate function column>')
    );

    accountsToUpdate.put(result.Id, result);
}
1

Using subqueries does not sound like it will help you accomplish your goal, and on top of that, it will still hit governor limits because each subquery counts as well. Your first approach has the right idea, but the SOQL queries are in the wrong place.

We never (I repeat NEVER) want to have a SOQL query inside a for loop. We will hit our governor limits easily (e.g., your current approach will hit limits once you try to work with 34 or more contacts). Instead, we want to rely on the in operator in our WHERE clauses. Combining a little from both of your examples to get all accounts related to contacts, we would get:

(in ContactTrigger.trigger)
trigger ContactTrigger on Contact(after insert) {
    ContactTriggerHelper.AllocatePrimaryContactOnInsert( Map<Id, Contact>(Trigger.newMap) );
}

(in ContactTriggerHelper.cls)
public class ContactTriggerHelper {
.
.
    public static void AllocatePrimaryContactOnInsert(Map<Id, Contact> newMap) {
        /* build a set of IDs that will store the account IDs on all new contacts */
        Set<Id> parentAccountIds = new Set<Id>();
        for ( Contact c : newMap.values() ) {
            parentAccountIds.add(c.AccountId);
        }

        /* perform the SOQL query using the new set of IDs in the 'WHERE' clause */
        Map<Id, Account> parentAccounts = new Map<Id, Account>( [ SELECT Id
                                                                    FROM Account
                                                                   WHERE Id in :parentAccountIds ]
    }
.
.
}

This small bit of code takes the Map of contacts we get from the trigger, stores each related account ID in a new Set, and then performs a query using that Set to get all related account records.

_Note: you do not NEED to use the map context variable nor create a map of ID to account the way I show here, but I prefer to do it to easily get record IDs by using the .keySet() method on maps.

We can repeat this process for the other two queries you have (I will leave those to you), and this should give us all of the information we need without performing queries inside a for loop (one of the biggest no-no's in Apex coding). You will need to find a reliable, efficient way to store the results from the queries to reference the correct accounts and contacts, though.

There are lots of patterns that people have come up with over the years to help bulkify triggers. The one I use most frequently is here, but like I said there are plenty to pick from. I suggest finding one that you think suits your needs the best.

2
  • Oops, looks like I'm late to the party - @derek-f has a winning answer here. Commented Jul 8, 2020 at 19:04
  • 2
    Having multiple takes/approaches is still helpful. One thing to note is that subqueries count towards a separate limit than regular queries. Salesforce (confusingly) calls the subquery limit "Aggregate" queries, and our limit on those is 300 per transaction. An outer query with 1 subquery (I think both parent-child subqueries and semi/anti-joins count) would count as 1 of 100 SOQL queries (counting the outer query), and 1 of 300 Aggregate queries (counting only the inner queries). The number of returned rows in both "normal" and "aggregate" queries counts towards the same 50k limit though.
    – Derek F
    Commented Jul 8, 2020 at 19:40

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .