Apex Code Development You need to sign in to do that
Don't have an account?
bakum Can I write this trigger without tripping the governor limit on SOQL queries?
Hi,
Background: Customer is a travel agency. They have a custom object called Reservation__c, and on that object are two fields, Guest_Name__c (lookup to Account), and Departure_Date__c (date).
They want to be able to report on the accounts that have not booked a reservation within the last six months. I cannot get this with a straight report, so I had the bright idea to put a field on the Account called Most_Recent_Reservation__c (lookup to Reservation__c), and a workflow on Reservation__c. When a Reservation__c is created or edited a workflow compares Reservation.Departure_Date__c and Account.Most_Recent_Reservation__r.Departure_Date__c and applies some logic to update Account.Most_Recent_Reservation__c with that Reservation ID or not. The customer could then report off the Most_Recent_Reservation__c.Departure_Date__c and Bob's my Uncle. The logic for when to update works. The first problem is I can't do a cross object update with a workflow.
So my second bright idea is to use the workflow to set a flag on Reservation__c, and trap for that flag with a trigger. The trigger can then do the cross object update no problem.
The second problem is I've done this sort of thing before and run into governor limits when doing bulk updates.
So, the question is can I write this trigger some way so as not to hit the governor limit? Here's the trigger code:
trigger update_most_recent_reservation on Reservation__c (after insert, after update)
{
//make it bulk safe
for (Integer i=0; i<Trigger.new.size(); i++)
{
Reservation__c res = Trigger.new[i];
//check to see if the update flag is set to true
if (res.Update_Guest_Most_Recnt_Resrvation_Flag__c)
{
//double check that it's appropriate to update the field
if (res.Guest_Name__c != null && res.Departure_Date__c != null)
{
//find the right account to update and update it
Account guest = [select Id from Account where Id = :res.Guest_Name__c];
guest.Most_Recent_Reservation__c = res.Id;
update guest;
}
}
//set the update flag on Reservation__c back to false
res.Update_Guest_Most_Recnt_Resrvation_Flag__c = FALSE;
update res;
}
}
Background: Customer is a travel agency. They have a custom object called Reservation__c, and on that object are two fields, Guest_Name__c (lookup to Account), and Departure_Date__c (date).
They want to be able to report on the accounts that have not booked a reservation within the last six months. I cannot get this with a straight report, so I had the bright idea to put a field on the Account called Most_Recent_Reservation__c (lookup to Reservation__c), and a workflow on Reservation__c. When a Reservation__c is created or edited a workflow compares Reservation.Departure_Date__c and Account.Most_Recent_Reservation__r.Departure_Date__c and applies some logic to update Account.Most_Recent_Reservation__c with that Reservation ID or not. The customer could then report off the Most_Recent_Reservation__c.Departure_Date__c and Bob's my Uncle. The logic for when to update works. The first problem is I can't do a cross object update with a workflow.
So my second bright idea is to use the workflow to set a flag on Reservation__c, and trap for that flag with a trigger. The trigger can then do the cross object update no problem.
The second problem is I've done this sort of thing before and run into governor limits when doing bulk updates.
So, the question is can I write this trigger some way so as not to hit the governor limit? Here's the trigger code:
trigger update_most_recent_reservation on Reservation__c (after insert, after update)
{
//make it bulk safe
for (Integer i=0; i<Trigger.new.size(); i++)
{
Reservation__c res = Trigger.new[i];
//check to see if the update flag is set to true
if (res.Update_Guest_Most_Recnt_Resrvation_Flag__c)
{
//double check that it's appropriate to update the field
if (res.Guest_Name__c != null && res.Departure_Date__c != null)
{
//find the right account to update and update it
Account guest = [select Id from Account where Id = :res.Guest_Name__c];
guest.Most_Recent_Reservation__c = res.Id;
update guest;
}
}
//set the update flag on Reservation__c back to false
res.Update_Guest_Most_Recnt_Resrvation_Flag__c = FALSE;
update res;
}
}
-David
Account guest = [select Id from Account where Id = :res.Guest_Name__c];
To do that, first create a Set with all Guest_Name__c, then query for Accounts where Guest_Name__c is in the Set.
Drop the accounts in a Map with the Key the guestname and the value object the account.
Then in your loop you can get to the accounts via the Map, without having to query for them ...
Hope this works & helps :-)
-David
that's awesome but I don't quite understand...
Guest_Name__c is a lookup field, and a set only contains primitives...so something like this?
Set(Id) myAccounts = new Set<Id>(...I'm not sure what to put here...is it a query of all Guest_Name__c's?)
Then, I'm not sure how you got from the Set to the Map of Account objects.
I guess what I don't understand is the Trigger has the field Guest_Name__c which is the Account ID that I need to update. So I need to pull that out of the trigger. But in order to update it, don't I need to do a query to get it into an object?
Thanks,
-mb
The Set you need to create contains the ID's of all the Guest_Name__c Id's that come in via Trigger.new.
Something like :
Set<Id> allguests = new Set<Id>(); for(Reservation__c res:Trigger.new) { allguests.add(res.Guest_Name__c); //perhaps Guest_Name__c.Id }Now you have a nice set of Id's that will allow you to get all the accounts that are relevant for the incoming Reservation__c, in just one query :
Then in your loop instead of doing the query you just go :
Account guest = accountsmap.get(res.Guest_Name__c);
Edit :I just saw that there's one more DML operation in your loop : update res; (needs to get out of the loop too)
That's easy to solve : Instead of updating these, drop them in a list and do one update query with the list after your loop.
Hope this helps.
David
P.S. : Since you already have the Id of the Account in the res.Guest_Name__c, you might even try and not do the account Map query altogether. You could just in your loop create a new Account(), set the Id of that to Guest_Name__c, fill in the Most_Recent_Reservation__c field and update it (since you have the Id it will update the correct account and not touch the fields you didn't specify ... does this make sense ?)
Message Edited by David VP on 10-25-2008 11:23 AM
Message Edited by David VP on 10-25-2008 11:23 AM
I'm getting an error I don't understand. here's the new trigger:
trigger update_most_recent_reservation on Reservation__c (before update)
{
Set<Id> allguests = new Set<Id>();
//create a set of all the Guest_Name__c's in the trigger
for(Reservation__c res:Trigger.new)
{
allguests.add(res.Guest_Name__c);
}
//use one query to create a map of the set above to actual account objects that we will use for updating.
Map<Id, Account> accountsmap = new Map<Id, Account>([Select a.Id, a.Name From Account a where a.Id in :allguests]);
// Create an empty list of String
List<Account> myAccountUpdates = new List<Account>();
//make it bulk safe
for (Integer i=0; i<Trigger.new.size(); i++)
{
Reservation__c res = Trigger.new[i];
//check to see if the update flag is set to true
if (res.Update_Guest_Most_Recnt_Resrvation_Flag__c)
{
//double check that it's appropriate to update the field
if (res.Guest_Name__c != null && res.Departure_Date__c != null)
{
//find the right account from the map
Account guest = accountsmap.get(res.Guest_Name__c);
//update that account's field
guest.Most_Recent_Reservation__c = res.Id;
//add that account to a list of accounts to update outside of the for loop
myAccountUpdates.add(guest);
}
//set the update flag on Reservation__c back to false
//before trigger so no DML needed
res.Update_Guest_Most_Recnt_Resrvation_Flag__c = FALSE;
}
}
//now update accounts from list
update myAccountUpdates;
}
here's the test method:
public class update_recent_res_flag_test {
static testMethod void myTest()
{
Account guest = new Account();
guest.FirstName = 'Jim';
guest.LastName = 'Jones';
guest.RecordTypeId = '012400000005Os8';
insert guest;
Reservation__c res1 = new Reservation__c();
res1.Name = '123456';
res1.Guest_Name__c = guest.Id;
res1.Departure_Date__c = Date.newInstance(2008,11,11);
insert res1;
Account guestTest = [select Id, Most_Recent_Reservation__c from Account where Id=:guest.Id];
System.assertEquals(guestTest.Most_Recent_Reservation__c,res1.Id);
Reservation__c resTest = [select Update_Guest_Most_Recnt_Resrvation_Flag__c
from Reservation__c
where Id=:res1.Id];
System.assertEquals(resTest.Update_Guest_Most_Recnt_Resrvation_Flag__c,FALSE);
Reservation__c res2 = new Reservation__c();
res2.Name = '123457';
res2.Guest_Name__c = guest.Id;
res2.Departure_Date__c = Date.newInstance(2008,12,11);
insert res2;
guestTest = [select Id, Most_Recent_Reservation__c from Account where Id=:guest.Id];
System.assertEquals(guestTest.Most_Recent_Reservation__c,res2.Id);
res1.Departure_Date__c = Date.newInstance(2009,11,11);
update res1;
guestTest = [select Id, Most_Recent_Reservation__c from Account where Id=:guest.Id];
System.assertEquals(guestTest.Most_Recent_Reservation__c,res1.Id);
}
}
it generates this error:
System.DmlException: Update failed. First exception on row 0 with id 001R0000009ecnEIAQ; first error: INVALID_FIELD_FOR_INSERT_UPDATE, Account: bad field names on insert/update call: Name: [Name]
Trigger.update_most_recent_reservation: line 41, column 5 (which is this line: update myAccountUpdates;)
Obviously there are bad field names in my list, but I'm not sure how that's possible. I'm only setting one field onto that list and it worked fine doing the DML udpate one by one in the for loop. ANy suggestions?
-mb
P.S. I'm a PHP guy so this syntax is really a learning experience for me. I really appreciate the help.
Do a system.debug(myAccountUpdates) and watch your system log window closely. It will show you the account list and all it's field values. You might get a clue from that.
-David
It looks to me...like...
your 'myAccountUpdates' array consists of Account records pulled from the 'accountsmap' map. In each of these records, you are updating its Most_Recent_Reservation__c field.
But the 'accountsmap' map was created via a 'select Id , Name from Account...' SOQL statement.
In order to be able to modify a field on a record that you've pulled from the DB, you need to have included that field in your SOQL statement; i.e. your 'accountsmap' select should be 'select Id , Name , Most_Recent_Reservation__c...'. Try that and see if it squashes your problem.
I used to be creating the map like this:
Map<Id, Account> accountsmap = new Map<Id, Account>([select a.Id,a.Name From Account a where a.Id in :allguests]);
then I tried like this:
Map<Id, Account> accountsmap = new Map<Id, Account>([select a.Id,a.Name ,a.Most_Recent_Reservation__c From Account a where a.Id in :allguests]);
but this actually worked:
Map<Id, Account> accountsmap = new Map<Id, Account>([select a.Id,a.Most_Recent_Reservation__c From Account a where a.Id in :allguests]);
and the problem has since disappeared. I don't really know why but it has.
Anyway, thanks to everyone who helped me. I understand a LOT more about writing bulk safe triggers that actually do things now. And that's awesome.