+ Start a Discussion
Austin MayerhoferAustin Mayerhofer 

More efficient SOQL Query?

I have a custom object, food__c, and a webhook that inserts foods to our database.

I have food__c objects already in the database with product reference numbers, and when the webhook inserts foods, it will add the food__c regardless of whether the reference number already exists, in which case I would like it to update the quantity of the already existing food__c item, but instead I have been writing a trigger to check when something is inserted.
 
trigger InvoiceEntry on food__c (before insert, after insert) {
    if (Trigger.isBefore) {
        
        List<food__c> foods = [SELECT Quantity__c, Product_Reference__c FROM food__c];
        
        for (food__c f : Trigger.New) {
            if (f.Product_Reference__c != null) {
                for (food__c stored_food : foods) {
                    if (f.Product_Reference__c == stored_food.Product_Reference__c) {
                        stored_food.Quantity__c += f.Quantity__c;
                        update stored_food;

                    }
                }
            }
        }
    }
    if (Trigger.isAfter) {
        List<food__c> foods = [SELECT Quantity__c, Product_Reference__c FROM food__c];
        List<food__c> same_food = new List<food__c>();
        for (food__c f : Trigger.New) {
            if (f.Product_Reference__c != null) {
                for (food__c stored_food : foods) {
                    if (f.Product_Reference__c == stored_food.Product_Reference__c) {
                        //System.debug(stored_food.Quantity__c + ': ' + foods.size());
                        same_food.add(stored_food);
                    }
                }
            }
            
            if (same_food.size() > 1) {
                if (same_food[0].Quantity__c < same_food[1].Quantity__c) {
                    delete same_food[0];
                }
                else if (same_food[0].Quantity__c > same_food[1].Quantity__c) {
                    delete same_food[1];
                }
                else {
                    delete same_food[0];
                }
            }
        }
    }

This works but seems horribly inefficient. I am trying to find a quick way to check, when an object is inserted, if its product reference number already exists in the database or not. If it already exists, update the quantity and don't insert the object. If it doesn't exist, just insert.

With my current code I'm looking through every item in the backend, and doing this for every food__c that is inserted, leading to O(n^2) runtime.

How can I make my code more efficient, and is there a better way to prevent something from being inserted?

To prevent duplicate food__c's from being entered, I tried using addError for food__c's that already existed in the database, but this would send an error back through the webhook and roll back the entire round of inserts.

Thank you.
Abhishek BansalAbhishek Bansal
Hi Austin,

Please find the updated efficient code as per your requirement:
trigger InvoiceEntry on food__c (after insert) {
	Set<String> productReferenceSet = new Set<String>();
	
	for (food__c f : Trigger.New) {
        if (f.Product_Reference__c != null) {
			productReferenceSet.add(f.Product_Reference__c);
		}
	}
	
	if(productReferenceSet.size() > 0) {
		Map<String, food__c> mapOfProdRefWithFood = new Map<String, food__c>();
		for(food__c food : [SELECT Quantity__c, Product_Reference__c FROM food__c where Product_Reference__c IN: productReferenceSet]) {
			if(!mapOfProdRefWithFood.containsKey(food.Product_Reference__c)) {
				mapOfProdRefWithFood.put(food.Product_Reference__c, food);
			}
		}
		List<Food__c> updateFoodList = new List<Food__c>();
		Set<Id> setOfFoodToDelete = new Set<Id>();
		Food__c updatedFood;
		for (food__c f : Trigger.New) {
			if (f.Product_Reference__c != null && mapOfProdRefWithFood.containsKey(f.Product_Reference__c)) {
				updatedFood = mapOfProdRefWithFood.get(f.Product_Reference__c);
				updatedFood.Quantity__c += f.Quantity__c;
				updateFoodList.add(updatedFood);
				setOfFoodToDelete.add(f.Id);
			}
		}
		if(updateFoodList.size() > 0) {
			update updateFoodList;
		}
		if(setOfFoodToDelete.size() > 0) {
			delete [Select Id from food__c where Id IN: setOfFoodToDelete];
		}
	}
}

Please take care of the syntax errors and API name of teh fields. Let me know if you need any further help or information on this.

Thanks,
Abhishek Bansal.
Andrew GAndrew G
Ok,

First, do you need every food in the back end, or just the food records for the food that is incoming from the hook?

I think the later, so start by getting just those foods.
List<Id> prIds = new List<Id>();  //a list of Product References for the incoming records

        for (food__c f : Trigger.New) {
            if (f.Product_Reference__c != null) {
                prIds.add(f.Product_Reference__c);
            }
        }

        List<food__c> foods = [SELECT Quantity__c, Product_Reference__c FROM food__c WHERE Product_Reference__c IN :prIds];
Next, i'm just working out why have you jumped to an after insert Trigger?  I'm assuming that the inserted record is fundamentally a Food record, but all we are doing is updating the existing record if it exists?  And Product_Reference__c is some sort of external Id to ID the food record or food type?
In which case we now convert the List to a Map using the Product_Reference as a Key
 
Map<String, Integer> foodQtyMap = new Map<String,Integer>();
for (food__c f : foods) {
    foodQtyMap.put(f.Product_Reference__c, Quantity);
}
Now we can loop the incoming records, if they exist in the Map, update the new record and Mark the previous record for deletion
so something like:
 
List<food__c> foodToRemove = new List<food__c>();

for( food__c f : trigger.new) {
    if (foodQtyMap.containsKey(f.Product_Reference__c) {
           f.Qty = f.Qty + foodQtyMap.get(f.Product_Reference__c).Qty; 
           foodToRemove.add(foodQtyMap.get(f.Product_Reference__c);
    }
}

delete foodToRemove;
Note, i'm marking the older record for deletion as off the top of my head I can't think how to remove the incoming record from the trigger without stopping your entire update process, as noted in your last paragraph.  So let the new records come in and be updated, and delete the older records.  If you wanted some history, you could create some fields for say "previous stock level" and "date level updated" and write them in your trigger.

Note that by using the before trigger in its entirity, there is no need for any updates and by not relying on both a before and after trigger context you will make the trigger more effiecient also.  So make sure to update the trigger context:
trigger InvoiceEntry on food__c (before insert)


Note, there is one logic flaw in the above.  If you incoming foods have 2 records with the same Product Reference, you will get unusual results. I.e. that will cause a duplicate in the food insertion.

Perhaps to prevent that if its a possibilty would be to insert an extra step where you pump the trigger list into a Set and then compare List.size() and Set.size() and abort the import.  Or if i can think more elegantly then maybe to Loop the list pushing to a Set and if there is a duplicate, add the qty to the record in the Set, change the incoming record to have a different Product Reference (e.g. DELETEFOOD) (remember as part of the trigger it will insert into the database) and then have a batch that deletes those records.  Or perhaps now have the After Insert trigger which does a look for those records and deletes them.

Hopefuly that gives you some food for thought (pun intended).


Regards

Andrew
 
Andrew GAndrew G
Hi Abhishek

If we desk check that code you supplied, we notice that you are using an after insert as context.  So therefore the records have been inserted.  When we then do a query, we pick up the recently inserted records plus the pre-existing records.  One of the vagary of the SOQL query is that there is no guarantee on the order of the returned results unless we use an ORDER BY statement.  Since we then insert into a map if it doesn't exist, and we don't know the order, we may be actually inserting the newer record into the MAP instead of the older record as the remainder of the code expects.

Austin
If you wish to pick up Abhishek's code, use something like
ORDER BY Product_Reference__c, CreatedDate
In this way the older record should come first, and therefore you guarantee that it is inserted first. 
 
Andrew GAndrew G
second issue for Abhishek.
What happens when a new food that never existed previously is inserted?  Since your query is against the inserted record, the "new" food record, will be returned and inserted into the MAP.  So the inserted record has it's quantity added to itself, so therefore is incorrectly doubled.

Austin
if you want to follow an after insert model, then you will need to insert an additional check that confirms that the record in the MAP does not have the same record Id as the record in the trigger.
So maybe something like:
for (food__c f : Trigger.New) {
			if (f.Product_Reference__c != null && mapOfProdRefWithFood.containsKey(f.Product_Reference__c)) {
				updatedFood = mapOfProdRefWithFood.get(f.Product_Reference__c);
if (updatedFood.Id <> f.Id) {
				updatedFood.Quantity__c += f.Quantity__c;
				updateFoodList.add(updatedFood);
				setOfFoodToDelete.add(f.Id);
}
			}
		}

 
Abhishek BansalAbhishek Bansal
Hi Andrew,

Yes you are right, it was a mistake a my end because its always hard to write a logic without having the real scenarios in front of you to test. I appreciate you point out the issues in my code so that I can learn more. 
But instead of having all these checks we can simply add one more condition in our query so that it will query all the records where Id Not In trigger.new. After adding this check all the things will be sorted out.

Austin just update the below line:
for(food__c food : [SELECT Quantity__c, Product_Reference__c FROM food__c where Product_Reference__c IN: productReferenceSet])
TO:

for(food__c food : [SELECT Quantity__c, Product_Reference__c FROM food__c where Product_Reference__c IN: productReferenceSet AND ID NOT IN: trigger.new])

This change will take care of all the issues mentioned by Andrew.

One more request Austin, please atleast give response on your questions or close them when you got your answer. We also need your support to make this platform better as you need ours.

Thanks,
Abhishek Bansal.
Andrew GAndrew G
Hi Abhishek.

Nice pick up as a solution.  My only thought there is that trigger.new would be a list of objects.  Do we need to convert it first to a list of Ids so that the comparison to the Id field is more accurate?  Or perhaps use oldMap
 
for(food__c food : [SELECT Quantity__c, Product_Reference__c FROM food__c 
WHERE Product_Reference__c IN: productReferenceSet AND 
ID NOT IN: trigger.newMap.keySet()])

And on a side note, since the code is updating the records(object) that initiated the trigger, do you feel that after trigger is still the best option?  As a rough rule of thumb I use, when updating the same record we use before trigger but if updating a related object(record) we use after trigger ?  This would save the issue of the updated records invoking the trigger again 

regards 
Andrew

Thanks for the discussion.  It always helps to see other's view of a solution.
Apologies to Austin if we seem to have hijacked your thread.
Abhishek BansalAbhishek Bansal
Hi Andrew,

We can use a list of records to compare with the ids, there is no need to convert the list in to list of ids as Salesforce automatically handles this internally.
On your second question, yes we use befoe insert for the same records but here the case is to delete the inserted records if it has duplicate reference number so it can only be done in the after insert case.
Yes, updating the same object records will cause the before update trigger to fire but that can be handled if proper conditions are used in the update trigger like to check if a particular value is changed than only run the code. And also even if we use the before insert case we still need to use the update statement to update the existing records with the same reference number.

Hope this clears all your questions!

Yes we have hijacked this thread but I dont think Austin is paying any attentions to it. May be he is trying the solution given by us so lets wait for his response.

Thanks,
Abhishek Bansal.
Austin MayerhoferAustin Mayerhofer
Hey guys, thank you so much for the amazing responses. I've been receiving email notifications but haven't quite had time to try out the queries due to us trying to get an MVP out, and the trigger efficiency is something for us to worry about later.

However, the quality of your guys' responses is so high that I'll try these out tomorrow afternoon. Much much appreciated!