function readOnly(count){ }
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
+ Start a Discussion
bohemianguy100bohemianguy100 

problem with query inside a for loop

I'm using a query insdie a for loop to return the most recent date for a set of records.  I'm hitting governor limits because the query is inside the for loop.  I can normally externalize a query outside of a for loop using a map, but in this particular scenario, I'm not sure how to accomplish it.

 

Here is the trigger:

 

trigger SalesInvoiceTotalsToOpportunity on Sales_Invoice__c (after insert, after update) {

	Set<Id> opIds = new Set<Id>();
	for (Sales_Invoice__c si : Trigger.new) {
		opIds.add(si.Opportunity__c);
	}
	
	List<Sales_Invoice__c> salesInvoiceList = [Select Opportunity__c, Date__c, Payment_Posting_Date__c, Subtotal__c, Subtotal_Paid__c From Sales_Invoice__c Where Opportunity__c in: opIds];
	
	Map<Id, List<Sales_Invoice__c>> oppyToSalesInvoiceMap = new Map<Id, List<Sales_Invoice__c>>();
	
	for (Sales_Invoice__c salesInvoice : salesInvoiceList) {
		
		List<Sales_Invoice__c> salesInvoices = oppyToSalesInvoiceMap.get(salesInvoice.Opportunity__c);
		if (salesInvoices == null) { 
			
			salesInvoices = new List<Sales_Invoice__c>();
			oppyToSalesInvoiceMap.put(salesInvoice.Opportunity__c, salesInvoices);
		}
		salesInvoices.add(salesInvoice);
	}
	
	List<Opportunity> opps = [Select Date_Paid__c, Date_Invoiced__c, Total_Invoiced_Amt__c, Total_Paid_Amt__c From Opportunity Where Id in: opIds];

	Map<Id, Opportunity> oppMap = new Map<Id, Opportunity>();
	
	for (Opportunity opp : opps) {
		oppMap.put(opp.Id, opp);
	}
	
	List<Opportunity> oppsToUpdate = new List<Opportunity>();
	
	for (Id oppyId : oppyToSalesInvoiceMap.keySet()) {
		Decimal invoicedAmt = 0;
		Decimal totalPaid = 0;
		List<Sales_Invoice__c> salesInvoices = oppyToSalesInvoiceMap.get(oppyId);
		Set<Id> setIds = new Set<Id>();
		for (Sales_Invoice__c salesInvoice : salesInvoices) {
			invoicedAmt =  invoicedAmt + salesInvoice.Subtotal__c;
			totalPaid = totalPaid + salesInvoice.Subtotal_Paid__c;
			setIds.add(salesInvoice.Id);
		}
		
		List<Sales_Invoice__c> pymtPostingDate = [Select Payment_Posting_Date__c From Sales_Invoice__c Where Id in: setIds AND Payment_Posting_Date__c <> null ORDER BY Payment_Posting_Date__c DESC LIMIT 1];
		List<Sales_Invoice__c> pymtDate = [Select Date__c From Sales_Invoice__c Where Id in: setIds AND Date__c <> null ORDER BY Date__c DESC LIMIT 1];
		
		Opportunity o = oppMap.get(oppyId);
		o.Total_Invoiced_Amt__c = invoicedAmt;
		o.Total_Paid_Amt__c = totalPaid;
		if (pymtPostingDate.size() > 0) {
			o.Date_Paid__c = pymtPostingDate[0].Payment_Posting_Date__c;
		}
		if (pymtDate.size() > 0) {
			o.Date_Invoiced__c = pymtDate[0].Date__c;
		}
		oppsToUpdate.add(o);
		
	}
	
	update oppsToUpdate;
	

}

 

Notice the two queries inside the for loop.  The query returns a list that contains the most recent date for the field specified.  I cannot figure out how to get the query outside of the for loop because I need to run the query inside the loop to get the list of Sales_Invoice__c for each opportunity.

 

Is there a way around this?  I was thinking I could use a map somehow, but I cannot figure out how to do it.

 

Thanks for any help.

Best Answer chosen by Admin (Salesforce Developers) 
gtindugtindu

I hope this is your solution (You might have to forgive if i'm missing a parenthesis [or have too many] a quick tidy should do it: 

trigger SalesInvoiceTotalsToOpportunity on Sales_Invoice__c (after insert, after update) {

	Set<Id> opIds = new Set<Id>();
	for (Sales_Invoice__c si : Trigger.new) {
		opIds.add(si.Opportunity__c);
	}
	
	AggregateResult[] salesInvoiceList = [Select Opportunity__c, MAX(Date__c) Date__c, MAX(Payment_Posting_Date__c) Payment_Posting_Date__c, SUM(Subtotal__c) Subtotal__c, SUM(Subtotal_Paid__c) Subtotal_Paid__c From Sales_Invoice__c Where Opportunity__c in: opIds GROUP BY Opportunity__c];
	
	Map<Id, AggregateResult> oppyToSalesInvoiceMap = new Map<Id, AggregateResult>();
	
	for (AggregateResult salesInvoice : salesInvoiceList) {
		if (!oppyToSalesInvoiceMap.containsKey((Id)salesInvoice.get('Opportunity__c')) 
			oppyToSalesInvoiceMap.put((Id)salesInvoice.get('Opportunity__c'), salesInvoice);
	}
	
	Map<Id, Opportunity> oppMap = new Map<Id, Opportunity>([Select Id, Date_Paid__c, Date_Invoiced__c, Total_Invoiced_amt__c, Total_Paid_Amt__c FROM Opportunity WHERE Id in :opIds]);
	
	List<Opportunity> oppsToUpdate = new List<Opportunity>();
	
	for (AggregateResult ar : oppyToSalesInvoiceMap.values()) {
		if (oppMap.containsKey((Id)ar.get('Opportunity__c')) {
			Opportunity o = oppMap.get((Id)ar.get('Opportunity__c'));
			o.Total_Invoiced_Amt__c = (Decimal)ar.get('Subtotal__c');
			o.Total_Paid_Amt__c = (Decimal)ar.get('Subtotal_Paid__c');
			o.Date_Paid__c = (Date)ar.get('Payment_Posting_Date__c');
			o.Date_Invoiced__c = (Date)ar.get('Date__c');

			oppsToUpdate.add(o);
		}
	}

	if (!oppsToUpdate.isEmpty())
		update oppsToUpdate;
}

All Answers

SargeSarge

Hi,

 

    You can definatley avoid the two queries inside the for loop. Here is how you cn do it.

 

1) Declare and initialize a map of Sales_Invoice__c after you query for Sales_Invoice (please include all the necessary columns at this point of query) at the first place (Line 7)

 

Map<Id, Sales_Invoice__c> salesInvoiceMap = new Map<Id,Sales_Invoice__c>();
salesInvoiceMap.putAll(salesInvoiceList);

2) Since the queries inside the for loop is referring to the Id you already queried for, you can refer to this map valriable,  salesInvoiceMap,  for getting the records rather than querying it again.

 

3) I assume you are searching for a Sales_Invoice__c record which has Payment Processing date not null.

    To accomplish that, you can search it for in the "salesInvoice" list variable based on your "Payment Processing date not null criteria" and use it to update opportunity.

 

 

Hope this helps.

Let me know if it worked or not.

 

Cheers..

 

bohemianguy100bohemianguy100

I'm not sure I understand.  I need to specifically query a set of records for a specific opportunity to get the most recent date. The criteria is a set of ids I get within my for loop.  I've removed some of the extra code for brevity, but you can refer to the code in my originally post to see all of it.

 

 

for (Id oppyId : oppyToSalesInvoiceMap.keySet()) {

		List<Sales_Invoice__c> salesInvoices = oppyToSalesInvoiceMap.get(oppyId);
		Set<Id> setIds = new Set<Id>();
		for (Sales_Invoice__c salesInvoice : salesInvoices) {
			setIds.add(salesInvoice.Id);
		}
		
		List<Sales_Invoice__c> pymtPostingDate = [Select Payment_Posting_Date__c From Sales_Invoice__c Where Id in: setIds AND Payment_Posting_Date__c <> null ORDER BY Payment_Posting_Date__c DESC LIMIT 1];
		List<Sales_Invoice__c> pymtDate = [Select Date__c From Sales_Invoice__c Where Id in: setIds AND Date__c <> null ORDER BY Date__c DESC LIMIT 1];
}		

 

The query that populates salesInvoiceList does not return the correct record I need.  I'm using an order by clause with a limit of 1 to return the most recent date for several sales invoices for a specific opportunity.  The query on line 7 will not give me that result.

 

Let me know if that helps explain or if I need to give more detail.

 

Thanks for the help.  I appreciate it.

 

Cheers.

gtindugtindu

I'm not sure why it is you are querying in the for loop - i see that you have a list/set of SalesInvoices already (variable: salesInvoices).  If you are looking for the the oldest dates for those two fields i suggest you look at aggregate functions

 

List<Sales_Invoice__c> salesInvoices = [Select MIN(Payment_Posting_Date__c), MIN(Date__c) From Sales_Invoice__c Where Id in: setIds GROUP BY Id];
bohemianguy100bohemianguy100

I tried the MAX aggregate function in the soql, but it isn't returning the correct value.

 

Here is an example that might help explain:

 

I have Opportunity A

This Opportunity has 5 Sales Invoice records

I have to total the invoiced amount for those 5 records and populate into the opportunity

I have to total the amount paid for those 5 records and populate into the opportunity

I have to obtain the most recent payment posting date for those 5 records and populate in the opportunity

I have to obtain the most recent payment date for those 5 records and populate in the opportunity

 

The logic in the trigger is working correctly and would work in bulk if I can pull those queries out of the for loop.

 

I'm getting the opportunity id stored in the oppySalesInvoiceMap keyset to return my Sales Invoive list.  Then I get the set of ids from that list to use to run the query to get the most recent dates.  I'm depending on the opportunity id from the map to get the sales invoice list, so how would I pull those queries out of the loop if I am dependent on the opportunity id to get the appropriate sales invoice list?

 

What am I missing?

 

I appreciate the help.  Thank you.

gtindugtindu

My apologies - i had the code before improperly setup please try the following

 

 

List<AggregateResult> aggregateData = [Select Opportunity__c, MAX(Payment_Posting_Date__c) Payment_Posting_Date__c, MAX(Date__c) Date__c,  SUM(Amount_Paid__c) Amount_Paid__c, SUM(Amount_Invoiced__c) Amount_Invoiced__c From Sales_Invoice__c WHERE Id in :setIds  GROUP BY Opportunity__c]);

 

You need to return a list of AggregateResult objects... and grouped on (ideally) the opportunity field.  Not seeing the rest of your code the above is a bit of a pieced together. 

 

.

 

bohemianguy100bohemianguy100

Hi gtindu,

 

Here is the current code for my trigger:

 

trigger SalesInvoiceTotalsToOpportunity on Sales_Invoice__c (after insert, after update) {

	Set<Id> opIds = new Set<Id>();
	for (Sales_Invoice__c si : Trigger.new) {
		opIds.add(si.Opportunity__c);
	}
	
	List<Sales_Invoice__c> salesInvoiceList = [Select Opportunity__c, Date__c, Payment_Posting_Date__c, Subtotal__c, Subtotal_Paid__c From Sales_Invoice__c Where Opportunity__c in: opIds];
	
	Map<Id, List<Sales_Invoice__c>> oppyToSalesInvoiceMap = new Map<Id, List<Sales_Invoice__c>>();
	
	for (Sales_Invoice__c salesInvoice : salesInvoiceList) {
		
		List<Sales_Invoice__c> salesInvoices = oppyToSalesInvoiceMap.get(salesInvoice.Opportunity__c);
		if (salesInvoices == null) { 
			
			salesInvoices = new List<Sales_Invoice__c>();
			oppyToSalesInvoiceMap.put(salesInvoice.Opportunity__c, salesInvoices);
		}
		salesInvoices.add(salesInvoice);
	}
	
	List<Opportunity> opps = [Select Date_Paid__c, Date_Invoiced__c, Total_Invoiced_Amt__c, Total_Paid_Amt__c From Opportunity Where Id in: opIds];

	Map<Id, Opportunity> oppMap = new Map<Id, Opportunity>();
	
	for (Opportunity opp : opps) {
		oppMap.put(opp.Id, opp);
	}
	
	List<Opportunity> oppsToUpdate = new List<Opportunity>();
	
	for (Id oppyId : oppyToSalesInvoiceMap.keySet()) {
		Decimal invoicedAmt = 0;
		Decimal totalPaid = 0;
		List<Sales_Invoice__c> salesInvoices = oppyToSalesInvoiceMap.get(oppyId);
		Set<Id> setIds = new Set<Id>();
		for (Sales_Invoice__c salesInvoice : salesInvoices) {
			invoicedAmt =  invoicedAmt + salesInvoice.Subtotal__c;
			totalPaid = totalPaid + salesInvoice.Subtotal_Paid__c;
			setIds.add(salesInvoice.Id);
		}
		
		List<Sales_Invoice__c> pymtPostingDate = [Select Payment_Posting_Date__c From Sales_Invoice__c Where Id in: setIds AND Payment_Posting_Date__c <> null ORDER BY Payment_Posting_Date__c DESC LIMIT 1];
		List<Sales_Invoice__c> pymtDate = [Select Date__c From Sales_Invoice__c Where Id in: setIds AND Date__c <> null ORDER BY Date__c DESC LIMIT 1];
		
		Opportunity o = oppMap.get(oppyId);
		o.Total_Invoiced_Amt__c = invoicedAmt;
		o.Total_Paid_Amt__c = totalPaid;
		if (pymtPostingDate.size() > 0) {
			o.Date_Paid__c = pymtPostingDate[0].Payment_Posting_Date__c;
		}
		if (pymtDate.size() > 0) {
			o.Date_Invoiced__c = pymtDate[0].Date__c;
		}
		oppsToUpdate.add(o);
		
	}
	
	update oppsToUpdate;
	

}

 

Based on your example, it looks like I would need to still include that query within the for loop because it would be referencing the setIds that is populated in the for loop.

 

Am I not understanding correctly?

 

Where would I place that query outside of my for loop based on my current code?

gtindugtindu

I hope this is your solution (You might have to forgive if i'm missing a parenthesis [or have too many] a quick tidy should do it: 

trigger SalesInvoiceTotalsToOpportunity on Sales_Invoice__c (after insert, after update) {

	Set<Id> opIds = new Set<Id>();
	for (Sales_Invoice__c si : Trigger.new) {
		opIds.add(si.Opportunity__c);
	}
	
	AggregateResult[] salesInvoiceList = [Select Opportunity__c, MAX(Date__c) Date__c, MAX(Payment_Posting_Date__c) Payment_Posting_Date__c, SUM(Subtotal__c) Subtotal__c, SUM(Subtotal_Paid__c) Subtotal_Paid__c From Sales_Invoice__c Where Opportunity__c in: opIds GROUP BY Opportunity__c];
	
	Map<Id, AggregateResult> oppyToSalesInvoiceMap = new Map<Id, AggregateResult>();
	
	for (AggregateResult salesInvoice : salesInvoiceList) {
		if (!oppyToSalesInvoiceMap.containsKey((Id)salesInvoice.get('Opportunity__c')) 
			oppyToSalesInvoiceMap.put((Id)salesInvoice.get('Opportunity__c'), salesInvoice);
	}
	
	Map<Id, Opportunity> oppMap = new Map<Id, Opportunity>([Select Id, Date_Paid__c, Date_Invoiced__c, Total_Invoiced_amt__c, Total_Paid_Amt__c FROM Opportunity WHERE Id in :opIds]);
	
	List<Opportunity> oppsToUpdate = new List<Opportunity>();
	
	for (AggregateResult ar : oppyToSalesInvoiceMap.values()) {
		if (oppMap.containsKey((Id)ar.get('Opportunity__c')) {
			Opportunity o = oppMap.get((Id)ar.get('Opportunity__c'));
			o.Total_Invoiced_Amt__c = (Decimal)ar.get('Subtotal__c');
			o.Total_Paid_Amt__c = (Decimal)ar.get('Subtotal_Paid__c');
			o.Date_Paid__c = (Date)ar.get('Payment_Posting_Date__c');
			o.Date_Invoiced__c = (Date)ar.get('Date__c');

			oppsToUpdate.add(o);
		}
	}

	if (!oppsToUpdate.isEmpty())
		update oppsToUpdate;
}
This was selected as the best answer
bohemianguy100bohemianguy100

For some reason, I keep getting invalid type for AggregateResult.  We are on Spring 10 in the sandbox.  I'm noticing in Eclipse that the version is showing 16 for the trigger.  Does that need to be updated?