+ Start a Discussion
Dan NorfleetDan Norfleet 

Aggregate Query Question

I have a process that will read through an order when it is updated and determine if there are duplicate sku's on the order.  If there are duplicates, I delete the duplicated row to leave only a single row for each Order / SKU.  My code is working, however, when I run a large number of orders through a single time, I get the Too Many SOQL query exception.  I know it is because I have a select within the For Loop.

I am having trouble separating the using the aggragate query results as paramaters in the second query.  Salesforce is new to me and not used to this restriction.  Can someone look at my code and suggest the best way to accomplish eliminating the Select from the For loop?

Following is my current code, I appreciate any suggestions.

Main ask, how do I use all the rows returned from the Aggragate results in ccOrders as parm values in the OrdItemDelTemp Select Statement?

Dan

/******************************************************************************
 * Determine if any of the orders have duplicate Items.
 * @param List<Id> orderIds 
 */
    public static void checkDuplicateOrderItems( List<Id> orderIds )
    {
        // Identify Orders within the list that have duplicate items - they will have a count > 1
        AggregateResult[] ccOrders =
             [SELECT ccrz__order__c ord_id
                     ,ccrz__product__r.ccrz__sku__c ord_sku
                     ,ccrz__orderlinetype__c ord_sku_type
                     ,ccrz__category__r.ccrz__categoryid__c ord_sku_catid
                     ,ccrz__category__r.name ord_sku_catname
                     ,COUNT(Id) dupitemcnt
              FROM ccrz__E_OrderItem__c 
              WHERE ccrz__Order__r.Id IN :orderIds 
              GROUP BY ccrz__order__c
                          ,ccrz__product__r.ccrz__sku__c
                          ,ccrz__orderlinetype__c
                          ,ccrz__category__r.ccrz__categoryid__c
                          ,ccrz__category__r.name
              HAVING Count(Id) > 1];

        // Exit Method loop if there are no duplicates in the orders being processed
        if( ccOrders.isEmpty() ) return;

        // Define list to store duplicates to be deleted.  
        List<ccrz__E_OrderItem__c> OrdItemDel = new List<ccrz__E_OrderItem__c>();
        ccrz__E_OrderItem__c OrdItemDelTemp = new ccrz__E_OrderItem__c();
        
        // For each duplicate identified in the order, add the duplicate rows to a list to delete.
        // The rows are deleted after the FOR loop. 
        for (AggregateResult ar : ccOrders)  {
             // using the duplicate and reading all but the last occurence of the sku into the list.  So
             // if a sku is on an order 2 times, it will only take the first occurrence and delete it.
             // The query will keep the order item with the highest quantity (if the quantity ordered is
             // different.
             OrdItemDelTemp =
                [SELECT id,
                          ccrz__product__r.ccrz__sku__c,
                          ccrz__orderlinetype__c,
                          ccrz__category__r.ccrz__categoryid__c,
                          ccrz__category__r.name,
                          ccrz__quantity__c
                 FROM ccrz__E_OrderItem__c
                 WHERE ccrz__order__c = :((ID)ar.get('ord_id'))
                   AND ccrz__product__r.ccrz__sku__c = :((String)ar.get('ord_sku'))
                         AND ccrz__orderlinetype__c = :((String)ar.get('ord_sku_type'))
                             AND ccrz__category__r.ccrz__categoryid__c = :((String)ar.get('ord_sku_catid'))
                         ORDER BY ccrz__quantity__c
                         LIMIT :((Integer)ar.get('dupitemcnt')) - 1];
                         
                         OrdItemDel.add(OrdItemDelTemp);
        }
        System.debug('**** DUPLICATE ORDER ITEMS TO DELETE:  '+ OrdItemDel);
        try 
            {
                            delete OrdItemDel;
            } 
            catch (Exception e) 
            {
                          System.debug('cc_HDPC_TriggerOrder.checkDuplicateOrderItems - ERROR DELETING DUPLICATE ORDER ITEMS - MSG = ' + e.getMessage().substring(1,255));
            }

        System.debug( 'EXIT: TriggerCC_HDPC_Order.checkDuplicateOrderItems()' );
    }
}

 
Best Answer chosen by Dan Norfleet
Steven NsubugaSteven Nsubuga
Below is some code I whipped up, you will have to check it for typos and such but it shows the flow I alluded to in my first comment. 
list<AggregateResult> badorders = [SELECT ccrz__order__c ord_id
                     ,ccrz__product__r.ccrz__sku__c ord_sku
                     ,ccrz__orderlinetype__c ord_sku_type
                     ,ccrz__category__r.ccrz__categoryid__c ord_sku_catid
                     ,ccrz__category__r.name ord_sku_catname
                     ,COUNT(Id) dupitemcnt
              FROM ccrz__E_OrderItem__c 
              WHERE ccrz__Order__r.Id IN :orderIds 
              GROUP BY ccrz__order__c
                          ,ccrz__product__r.ccrz__sku__c
                          ,ccrz__orderlinetype__c
                          ,ccrz__category__r.ccrz__categoryid__c
                          ,ccrz__category__r.name
              HAVING Count(Id) > 1];

Set<ID> ord_ids = new Set<ID>();
Set<string> ord_skus = new Set<string>();
Set<string> ord_sku_types = new Set<string>();
Set<string> ord_sku_catids = new Set<string>();

//loop over every row we found, and add the dupe identifiers to sets so we can find them again in another query.
for(AggregateResult ar : badorders) {

        ord_ids.add((ID)ar.get('ord_id'));
        ord_skus.add(string.valueOf(ar.get('ord_sku')));
        ord_sku_types.add(string.valueOf(ar.get('ord_sku_type')));
        ord_sku_catids.add(string.valueOf(ar.get('ord_sku_catid')));
}
//Now run a query that gets the ID's of the offending orders.  You MUST
//order the records by the fields you are using to identify them as dupes.
list<ccrz__E_OrderItem__c> listorders = [SELECT id,
                          ccrz__product__r.ccrz__sku__c,
                          ccrz__orderlinetype__c,
                          ccrz__category__r.ccrz__categoryid__c,
                          ccrz__category__r.name,
                          ccrz__quantity__c 
                          FROM ccrz__E_OrderItem__c
                          WHERE ccrz__order__c IN :ord_ids
                   AND ccrz__product__r.ccrz__sku__c IN :ord_skus
                         AND ccrz__orderlinetype__c IN :ord_sku_types
                             AND ccrz__category__r.ccrz__categoryid__c IN :ord_sku_catids
                         ORDER BY ccrz__order__c, ccrz__product__r.ccrz__sku__c, ccrz__orderlinetype__c, ccrz__category__r.ccrz__categoryid__c];


//Remove non dupes
list<ccrz__E_OrderItem__c> realDupes = new list<ccrz__E_OrderItem__c>();
for(ccrz__E_OrderItem__c listorder : listorders) {

        for(integer x = 0; x <  badorders.size(); x++) {
            AggregateResult ar = badorders[x];
            if ((ID)ar.get('ord_id') == listorder.Id && string.valueOf(ar.get('ord_sku')) == listorder.ccrz__product__r.ccrz__sku__c && string.valueOf(ar.get('ord_sku_type')) == listorder.ccrz__orderlinetype__c && string.valueOf(ar.get('ord_sku_catid')) == listorder.ccrz__category__r.ccrz__categoryid__c) {
                realDupes.add(listorder);
            }
        }
        
}
//Since we don't want to delete ALL the orders we just found (we have to leave one behind) we create another list
//of orders that will actually get deleted.
list<ccrz__E_OrderItem__c> ordersToDelete = new list<ccrz__E_OrderItem__c>();

//A variable to hold the subject of the last order looked at. If it matches, then it's a dupe.
//hence the reason we needed to sort by subject
string last_Id = 'placeholder123';
string last_ord_sku = 'placeholder123';
string last_ord_sku_type = 'placeholder123';
string last_ord_sku_catid= 'placeholder123';

for(ccrz__E_OrderItem__c realDupe : realDupes) {

    //here is that logic I was talking about. If the current order name
    //is the same as the one we just looked at, then obviously it's a dupe
    //and should get deleted. Otherwise don't do anything cause it's the one order
    //we want to leave behind.
    if(realDupe.Id == last_Id && realDupe.ccrz__product__r.ccrz__sku__c == last_ord_sku && realDupe.ccrz__orderlinetype__c == last_ord_sku_type && realDupe.ccrz__category__r.ccrz__categoryid__c == last_ord_sku_catid) {
    
        ordersToDelete.add(realDupe);
    }
    //set the variable for the next iteration
    last_Id = realDupe.Id;
    last_ord_sku = realDupe.ccrz__product__r.ccrz__sku__c;
    last_ord_sku_type = realDupe.ccrz__orderlinetype__c;
    last_ord_sku_catid = realDupe.ccrz__category__r.ccrz__categoryid__c;
}
//If there is anything to delete
if(!ordersToDelete.isEmpty()) {

    //delete those suckers.
    delete ordersToDelete;
}

 

All Answers

Steven NsubugaSteven Nsubuga
This code as it is now hits the too many SOQL queries limit whenever the ccOrders list is of size 100 and above.
1 cheeky way to get round this is to add a LIMIT 90 to the query that populates it.

The right way would be to iterate through the ccOrders and store the relevant ord_id, ord_sku, ord_sku_type and ord_sku_catid in sets. Then you would have 1 SOQL  to return all orders with these variables. The trick would then be to iterate through all the returned orders, eliminating the non duplicates and then deleting the duplicates. 
 
Steven NsubugaSteven Nsubuga
Below is some code I whipped up, you will have to check it for typos and such but it shows the flow I alluded to in my first comment. 
list<AggregateResult> badorders = [SELECT ccrz__order__c ord_id
                     ,ccrz__product__r.ccrz__sku__c ord_sku
                     ,ccrz__orderlinetype__c ord_sku_type
                     ,ccrz__category__r.ccrz__categoryid__c ord_sku_catid
                     ,ccrz__category__r.name ord_sku_catname
                     ,COUNT(Id) dupitemcnt
              FROM ccrz__E_OrderItem__c 
              WHERE ccrz__Order__r.Id IN :orderIds 
              GROUP BY ccrz__order__c
                          ,ccrz__product__r.ccrz__sku__c
                          ,ccrz__orderlinetype__c
                          ,ccrz__category__r.ccrz__categoryid__c
                          ,ccrz__category__r.name
              HAVING Count(Id) > 1];

Set<ID> ord_ids = new Set<ID>();
Set<string> ord_skus = new Set<string>();
Set<string> ord_sku_types = new Set<string>();
Set<string> ord_sku_catids = new Set<string>();

//loop over every row we found, and add the dupe identifiers to sets so we can find them again in another query.
for(AggregateResult ar : badorders) {

        ord_ids.add((ID)ar.get('ord_id'));
        ord_skus.add(string.valueOf(ar.get('ord_sku')));
        ord_sku_types.add(string.valueOf(ar.get('ord_sku_type')));
        ord_sku_catids.add(string.valueOf(ar.get('ord_sku_catid')));
}
//Now run a query that gets the ID's of the offending orders.  You MUST
//order the records by the fields you are using to identify them as dupes.
list<ccrz__E_OrderItem__c> listorders = [SELECT id,
                          ccrz__product__r.ccrz__sku__c,
                          ccrz__orderlinetype__c,
                          ccrz__category__r.ccrz__categoryid__c,
                          ccrz__category__r.name,
                          ccrz__quantity__c 
                          FROM ccrz__E_OrderItem__c
                          WHERE ccrz__order__c IN :ord_ids
                   AND ccrz__product__r.ccrz__sku__c IN :ord_skus
                         AND ccrz__orderlinetype__c IN :ord_sku_types
                             AND ccrz__category__r.ccrz__categoryid__c IN :ord_sku_catids
                         ORDER BY ccrz__order__c, ccrz__product__r.ccrz__sku__c, ccrz__orderlinetype__c, ccrz__category__r.ccrz__categoryid__c];


//Remove non dupes
list<ccrz__E_OrderItem__c> realDupes = new list<ccrz__E_OrderItem__c>();
for(ccrz__E_OrderItem__c listorder : listorders) {

        for(integer x = 0; x <  badorders.size(); x++) {
            AggregateResult ar = badorders[x];
            if ((ID)ar.get('ord_id') == listorder.Id && string.valueOf(ar.get('ord_sku')) == listorder.ccrz__product__r.ccrz__sku__c && string.valueOf(ar.get('ord_sku_type')) == listorder.ccrz__orderlinetype__c && string.valueOf(ar.get('ord_sku_catid')) == listorder.ccrz__category__r.ccrz__categoryid__c) {
                realDupes.add(listorder);
            }
        }
        
}
//Since we don't want to delete ALL the orders we just found (we have to leave one behind) we create another list
//of orders that will actually get deleted.
list<ccrz__E_OrderItem__c> ordersToDelete = new list<ccrz__E_OrderItem__c>();

//A variable to hold the subject of the last order looked at. If it matches, then it's a dupe.
//hence the reason we needed to sort by subject
string last_Id = 'placeholder123';
string last_ord_sku = 'placeholder123';
string last_ord_sku_type = 'placeholder123';
string last_ord_sku_catid= 'placeholder123';

for(ccrz__E_OrderItem__c realDupe : realDupes) {

    //here is that logic I was talking about. If the current order name
    //is the same as the one we just looked at, then obviously it's a dupe
    //and should get deleted. Otherwise don't do anything cause it's the one order
    //we want to leave behind.
    if(realDupe.Id == last_Id && realDupe.ccrz__product__r.ccrz__sku__c == last_ord_sku && realDupe.ccrz__orderlinetype__c == last_ord_sku_type && realDupe.ccrz__category__r.ccrz__categoryid__c == last_ord_sku_catid) {
    
        ordersToDelete.add(realDupe);
    }
    //set the variable for the next iteration
    last_Id = realDupe.Id;
    last_ord_sku = realDupe.ccrz__product__r.ccrz__sku__c;
    last_ord_sku_type = realDupe.ccrz__orderlinetype__c;
    last_ord_sku_catid = realDupe.ccrz__category__r.ccrz__categoryid__c;
}
//If there is anything to delete
if(!ordersToDelete.isEmpty()) {

    //delete those suckers.
    delete ordersToDelete;
}

 
This was selected as the best answer
Dan NorfleetDan Norfleet
Steven, thanks so much for such detail.  I really appreciate the help.  Am now working through what you have sent, I really appreciate your help.

I knew I wasn't doing it correctly with the logic in the For loop.  Was trying to first get it to work, then change the logic to be Salesforce friendly - was struggling a bit with that.  Thank you

Dan