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:

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

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

PricebookEntries:
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){
		productNames.add(oli.Product_Name__c);	
		oppIds.add(oli.OpportunityId);
	}
	
	//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]){
		oppPricebookMap.put(opp.Id,opp.Pricebook2Id);
	}
	
	//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()]){
		system.debug(pbe);
		
		//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
		pricebookProductMap.get(pbe.Pricebook2Id).put(pbe.Name,pbe.Id);
	}
	
	//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.

Thanks,
Jason