1

I have a simple trigger on a custom object called Show__c. Inside of Show__c, I have a checkbox Update_Show_Players__c that's unchecked by default. The trigger checks to see if Update_Show_Players__c is checked and if so it executes a bunch of business logic and inserts a new record into a junction object.

There are two ways that can cause the trigger to be executed: (1) by manually going into an individual record and checking the box next to "Update Show Players" or (2) by bulk uploading an update to a group of Show__c records where in Update_Show_Players__c is changed to TRUE.

When I use the first method, the trigger works flawlessly and the desired new record(s) appear in the junction object.

When I use the second method, it fails. The error given by DataLoader on each record is:

ERROR: UpdateShowPlayers: execution of AfterUpdatecaused by: System.DmlException: Insert failed. First exception on row 0 with id a0Ji000000C6l9VEAR; first error: INVALID_FIELD_FOR_INSERT_UPDATE, cannot specify Id in an insert call: [Id]Trigger.UpdateShowPlayers: line 28, column 1

There are two things that are interesting here: (a) the same id is given in each error message. Why would that be? (b) why is a trigger that works fine on an individual record not work in bulk? What am I doing wrong in my code?

Code is here. The "line 28" referred to in the error is the insert statement:

trigger UpdateShowPlayers on Show__c (after update) {
    List<Show_Player__c> spList = new List<Show_Player__c>();

    for (Show__c s : trigger.new) {
        if (s.Update_Show_Players__c == true) {
            List<Team__c> result = [select id, (select id, Name from Players__r) from Team__c where Id IN (select Team__c from Score__c where Show__c = :s.id)];
            List<AggregateResult> scores = [select Team__r.id, count(Total_Score__c), max(Date__c) from Score__c where Show__c = :s.id group by Team__r.id];

            for (Team__c t : result) {
                if (t.Players__r.size() > 0) {
                    for (Player__c p : t.Players__r) {
                        Show_Player__c sp = new Show_Player__c();
                        sp.Show__c = s.id;
                        sp.Player__c = p.id;

                        for (AggregateResult sc : scores) {
                            if (sc.get('id') == t.id) {
                                sp.Number_of_Times_Attended__c = (Decimal)sc.get('expr0');
                                sp.Date_Last_Seen__c = (Date)sc.get('expr1');
                            }
                        }

                        spList.add(sp);                     
                    }
                }
            }
            if (!spList.isEmpty()) insert spList;
        }
    }
}
  • 2
    On a my phone now, so will try to look at code in a bit, but in general you should try to refactor this code to use Apex best practices. You have both SOQL and DML within a loop here so you are very likely to run into governor limit issues. – Chris Duncombe Nov 5 '14 at 20:59
  • 1
    I started to write an answer, then realized I'd not properly read all the details. Your trigger isn't bulkified. You'll want to collect all your IDs first, then run your queries before doing the inserts. That's why its showing the same Id. This trigger can only operate on one record at a time. That's why it fails in bulk insert mode. You have 2 queries per record so will hit governor limits. – crmprogdev Nov 5 '14 at 21:02
  • I guess I don't understand the messaging around governor limits then. I've written triggers before where I've hit limits and been told something like "Too many SOQL queries." I totally understand that I could hit governor limits here and acknowledge that the code isn't properly bulkified. However, the error message doesn't say I'm hitting a limit, so I'm confused. – eyedar Nov 5 '14 at 21:07
  • 3
    I find it easier to simply write a trigger from the start using bulkified code, as in creating lists, sets and maps rather than thinking in terms of executing an operation on a single record. You may only have 200 records now but 99.9% of the time you can expect that number to always continue to grow, so learning to think in terms of a bulkified design context can be very helpful. It will save you a lot of time in rewriting/refactoring code to bulkify it later. – crmprogdev Nov 5 '14 at 22:12
  • 1
    Could not agree more @crmprogdev. It then becomes second nature, and then you have a hard time trying to write a trigger that isn't bulkified. – Chris Duncombe Nov 6 '14 at 0:40
2

If there is more than one Show__c object being handled by the trigger, then when the first one has been handled it may result in an object being added to the spList and the list being inserted into the database. Part of the insert process is to set an ID on the inserted object.

Then if the second Show__c object results in another object being added to the spList, the insert is done again for both the objects in the list. As the first one already has an ID, an error results as inserting an SObject that has an ID is not allowed.

The "quick fix" is to move the spList insert out of the trigger.new loop. But as others have commented and you are already aware, other bulkification is also advisable.

| improve this answer | |
  • Ahhh! Of course...I see now that the way I have it would attempt to insert the same record over and over again. Makes total sense now. Thanks. – eyedar Nov 5 '14 at 21:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.