+ Start a Discussion

A trigger to set PricebookEntryId, how to query efficiently?

Here is the scenario:

Upon the insert of an OpportunityLineItem the PricebookEntry field will not be defined. A before insert trigger will need to populate this value. On the insert there is a custom field that lists the product name, product names are unique.

Sounds simple right? but I can't find an efficient way to query the pricebook entries without returning more records than are required.

The Setup:

Opp ABC (Id = 7777, Pricebook2Id = 1)
Opp DEF (Id = 8888, Pricebook2Id = 2)

Widget1 (ID = 1234)
Widget2 (ID = 5678)

PBE1 (Pricebook2Id = 1, Name = Widget1, Product2Id = 1234)
PBE2 (Pricebook2Id = 1, Name = Widget2, Product2Id = 5678)
PBE3 (Pricebook2Id = 2, Name = Widget1, Product2Id = 1234)
PBE4 (Pricebook2Id = 2, Name = Widget2, Product2Id = 5678)

Line Items being inserted:
OpportunityLineItem(OpportunityId = 7777, Product_Name__c = Widget1)
OpportunityLineItem(OpportunityId = 8888, Product_Name__c = Widget2)

On the OpportunityLineItems being inserted I need a trigger to set the PricebookEntryId field. Below is this trigger:


trigger RenewalLineInsert on OpportunityLineItem (before insert) {
	Set<String> productNames = new Set<String>();
	Set<Id> oppIds = new Set<Id>();
	Map<Id,Id> oppPricebookMap = new Map<Id,Id>();
	Map<Id,Map<String,Id>> pricebookProductMap = new Map<Id,Map<String,Id>>();
	//Add product names and opp ids to sets for future queries
	for(OpportunityLineItem oli : trigger.new){
	//Query the related opportunities and get the pricebook2Id, put in map OppId -> Pricebook2Id
	for(Opportunity opp : [select Id, Pricebook2Id from Opportunity where Id IN :oppIds]){
	//Query the pricebook entries, add to a map Pricebook2Id -> (Product Name -> PricebookEntryId)
	for(PricebookEntry pbe : [select Id, Name, Pricebook2Id from PricebookEntry where Name IN :productNames AND Pricebook2Id IN :oppPricebookMap.values()]){
		//Check to see if the map of pricebooks to producs contains an innter map, if not create
		if(pricebookProductMap.get(pbe.Pricebook2Id) == null){
			pricebookProductMap.put(pbe.Pricebook2Id,new Map<String,Id>());
		//Add Product to map
	//Loop through the inserted Line Items and set the PricebookEntryId field
	for(OpportunityLineItem oli : trigger.new){
		Id pricebookId = oppPricebookMap.get(oli.OpportunityId);
		oli.PricebookEntryId = pricebookProductMap.get(pricebookId).get(oli.Product_Name__c);

There is a problem with this trigger and it is the query on the pricebook entries table. We are only inserting two line items so ideally we when querying the pricebook entries we should only get two results. But this query with the two IN filters will actually return all four pricebook entries even though we only need two.

This trigger works but it could be better and I am obsessed with my code being efficient. If you know of a way to make this trigger better and only query the actual number of PricebookEntry records needed I will buy you a bear at Dreamforce.


David SchachDavid Schach

1. Great code with a great explanation

2. I don't see a way to query fewer than four PriceBookEntries because you need to account for the smallest possible set of returned values... short of doing a query on each iteration (which is a no-no).

Basically, running through the logic, I don't think there is a way to set - even expressed in pseudocode - a situation in which a single query can return only two records.

The source of these "extra records" is the fact that you are creating a junction object, and so have to allow for all combinations of each of your variables - and there's no way around that.  And you're not helped by the intervening "PriceBookEntry" object sitting between OLI and Product2 - so even if you set the Product_Name__c field to a unique external ID, there's no composite primary key on the PBE object.

Of course, it's horrible to say that something can't be done because all it takes is one person showing that it can and I look like a fool, but I'll take that chance.

Sorry!  Wish it were better news.




Ya, I pretty much came to the same conclusions with the only other possible option being to query each line individualy, which obviously isn't really an option at all.


I too don't think there is a better way to do this but maybe somewhere....someone has a super great trick up their sleeve.


I think rather than a select .. where name in :productName and priceBook2Id in :pricebooks you'll need to do select ... where (name = pname1 and pricebook2Id= pb2Id) or ( name = pname2 and pricebook2 = pb2Id2)


i'm assuming you can match up which product name and pricebook Id's go together. (which i think you can, but its late and i haven't had enough coffee today)




Thought about doing that but  the number of line items being inserted can vary in size, it coiuld be 1-200. This would require dyanmic SOQL and one really long SOQL statement. I'm worried I might hit the maximum length of a SOQL statement or get a filter criteria too complex error.






Did you get any resolution on this. We have a requirement to be able to insert opportunities with opportunity line items using the apex dataloader on a nightly schedule and can't find any good way of doing it.


Any help would be approciated.