Trigger works most of the time, but sometimes I get this error: System.LimitException: Too many query rows: 50001

As people have mentioned in the comments already, I think the big issue here is that your trigger isn't what Salesforce calls "Bulk Friendly". "bulkifying" code pretty much boils down to making sure you don't have queries (SOQL or SOSL) or DML inside of loops.

It's hard to say what, precisely, is causing your error here (and why you aren't running into the DML or Query limit first) because there are other factors that can affect the outcome (Process Builder and Triggers don't mix well, is there a trigger on SAP_Sales_Order__c?, what other triggers might be run?, workflow rules with field updates cause update triggers to run [potentially for a second time], etc...).

At any rate though, what will help is to bulkify your trigger. The general 'bulk' pattern involves using collections (Sets, Lists, and Maps), and usually looks like this:

trigger MyTrigger on MyObject__c (before update){ // When you need to query something, it's a good idea to gather all of the data // you'll be using in your WHERE clause ahead of time. // We do that by declaring a List (a Set would work too), and iterating over data // that we already have. // In this loop, we're only concerned about extracting data for use in our query. List<Id> topLevelIds = new List<Id>(); for(MyObject__c myRec :Trigger.new){ // Note that if your 'someField__c' is really the Id field of the object // you're iterating over, you don't need this loop (or the List<Id> // for that matter) because that information is available through // Trigger.newMap.keySet() (on all update triggers, after insert triggers, // and after undelete triggers) // This approach (iterating over trigger.new and storing field information) // is useful when, for example, someField__c points to a common record of // a third object (say, TopLevel__c) that both MyObject__c and ChildObject__c // have a relationship with. topLevelIds.add(myRec.someField__c); } // Yes, looping over Trigger.new incurs some overhead, but we have a _lot_ more // CPU time to work with than we do queries. // Also, the above loop is 'tight'. // There isn't much happening in it, and the things that are happening are fast // operations. // Now that we've gathered our information, we can query once to gather all the // supplemental data we need. List<ChildObject__c> relatedData = [SELECT Id, special_field__c FROM ChildObject__c WHERE TopLevel__c IN :topLevelIds]; // If the field you're filtering your query on matches the Ids of the records // in your trigger, as is the case with child records (lookup or master-detail), // you could simply use trigger.newMap.keySet() directly in your query. //List<ChildObject__c> relatedData = [SELECT Id, special_field__c FROM ChildObject__c WHERE Parent_Lookup__c IN :Trigger.newMap.keySet()]; // Now, if we want to update records, the thing to do is to store the changes // we're making in-memory in another collection. // After we're done making changes, we can DML update all the records we changed // in one shot. List<OtherObject__c> otherObjsToUpdate = new List<OtherObject__c>(); for(OtherObject__c otherObj :relatedData){ MyObject__c parentRecord = Trigger.newMap.get(otherObj.MyObject_Lookup__c); if(parentRecord.some_field__c == 'some value'){ otherObj.different_field__c = 'different value'; otherObjsToUpdate.add(otherObj); } } // Now that we've made all of our changes, it's safe to update our records. update otherObjsToUpdate; } 

Let's apply that to your trigger

trigger putOppIdOnSalesOrder2 on Opportunity (after update) { // First, some commentary on your existing trigger /* for (Opportunity PrNoId : trigger.new){ if (PrNoId.cilioproject__c!=Null && PrNoId.cilioproject__c!=''){ Set<String> OP = new set<string>(); // It's a good idea to always include curly braces, even if you can // omit them. // It's very easy to miss that something is part of a loop or an // if/else otherwise. // On top of that, you're already iterating over trigger.new. // Iterating over the same thing twice is wasteful. for (Opportunity o : trigger.new){ OP.add(o.cilioproject__c); } // Queries in loops are bad. // This one will be easy to extract out of the loop List<SAP_Sales_Order__C> SO = [select SAP_Project_no__c from SAP_Sales_Order__C Where SAP_Sales_Order__C.SAP_Project_no__c IN : OP]; //updates the list of sales orders' lookup field with the id of the opportunity being updated //Project__c is the lookup field for (SAP_Sales_Order__c Pno : SO){ Pno.Project__c=PrNoId.id; // DML inside of a loop (a nested loop, at that) is bad. // Having a collection declared/initialized outside of the loop // is the way to go. update (Pno); } } else { Set<string> codes = new set<string>(); // Again, you have the right idea, adding Ids to a set. // Easy enough to extract out of the outer loop. for(Opportunity trip: trigger.new){ codes.add(trip.Id); } // Another query in a loop, another query that's easy to extract List<SAP_Sales_Order__c> ProjNo = [SELECT Project__c FROM SAP_Sales_Order__c WHERE SAP_Sales_Order__C.Project__c IN : codes ]; for (SAP_Sales_Order__c rem : ProjNo){ rem.Project__c=Null; // Again, putting rem into a List<SAP_Sales_Order__c> and doing // the DML outside of all loops is the way to go. update (rem); } } }*/ // Begin bulkified code // I can't discern precisely what your trigger is actually trying to accomplish. // What follows is my best guess. // Declaring/initializing collections outside of loops. Set<String> OP = new set<string>(); Set<string> codes = new set<string>(); // We can populate both of your collections with a single loop for(Opportunity trip: trigger.new){ if (trip.cilioproject__c!=Null && PrNoId.cilioproject__c!=''){ OP.add(trip.cilioproject__c); } else { codes.add(trip.Id); } } // Query outside of loops // There's no need to prefix fields of SAP_Sales_Order__c in a query on // 'SAP_Sales_Order__c' List<SAP_Sales_Order__C> SO = [SELECT SAP_Project_no__c FROM SAP_Sales_Order__C WHERE SAP_Project_no__c IN :OP]; List<SAP_Sales_Order__c> ProjNo = [SELECT Project__c FROM SAP_Sales_Order__c WHERE Project__c IN :codes]; // Declare collection(s) to hold the updates to the records so we can update // in a single shot. List<SAP_Sales_Order__c> soRecsToSetProject = List<SAP_Sales_Order__c>(); List<SAP_Sales_Order__c> soRecsToResetProject = List<SAP_Sales_Order__c>(); // This is where your trigger starts not making sense to me. // Your original trigger was updating each Sales Order multiple times (one // time per Opportunity being updated). // If you had code that actually did something with that, you should reconsider // your approach. // If you don't have code that made use of each new Project__c value, then // the code I have here is equivalent to what you had. for(SAP_Sales_Order__c so :SO){ so.Project__c = trigger.new[trigger.new.size()].Id; soRecsToSetProject.add(so); } // Is there any overlap between the Sales Orders in this list and the ones in // your other List (SO)? // If so, that's another indication that you should reconsider your approach // to solving whatever problem you're trying to solve. for(SAP_Sales_Order__c so :ProjNo){ so.Project__c = null; soRecsToResetProject.add(so); } // DML outside of loops update soRecsToSetProject; update soRecsToResetProject; }