function readOnly(count){ }
Sign Up ›
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
Learn More

Browse by Topic
ShowAll Questionssorted byDate Posted
+ Start a Discussion
tschatztschatz 

Too many SOQL Queries Help Please!

Hi All,
 
I'm really banging my head with this one. I understand why I'm getting the error. My code is making too many SOQL queries. I've read the reference manual and understand that in some way I need to use Sets and Maps to get around making so many SOQL calls, but for the life of me, I can't figure out how to work this in to my code. To me it seems that I need to make one SOQL call to get a piece of information that I need to make the next SOQL call and so forth. Could someone PLEASE look at my code and show me using my code how I could change this to avoid the too many SOQL queries error? I've seen examples of how it's done but can not apply that to my code. My brain is not working in that direction so please don't point me to other examples. I'm not getting it.
 
Thank you in advance for your much appreciated help.
 
_t
 
Code:
public class CP_OpportunityClosedWon {
 public static void CP_createNewCPUser(Opportunity[] opps){
  String cName;
    Contact c;
    OpportunityLineItem[] oli = new OpportunityLineItem[]{};  //initialized to fix null pointer error
    for (Opportunity o:opps){
      if (o.Order_Status__c == 'Shipped' || o.Order_Status__c == 'Definite') {
    ID cID = [select contactid from opportunitycontactrole where role = 'Ship To' AND opportunityid = :o.id].contactId;
       c = [select id, title from contact where id = :cID];
    oli = [select id from opportunitylineitem where opportunityid = :o.Id];
   }
    }
  //oli is null if the status is not shipped or definite which causes an exception here
    for (OpportunityLineItem li : oli){
      ID pID = [Select o.PricebookEntryId, o.PricebookEntry.Product2Id from OpportunityLineItem o where id = :li.id].PriceBookEntry.Product2Id;
      if(pID != null){
        Product2 p = [select id,name,downloadable__c from product2 where id = :pID];
           if(p.downloadable__c == true){
          c.Portal_User__c = true;
          c.Portal_URL__c = c.title;
          update c;
        }
      }
    }
  }
}

 
GoForceGoGoForceGo
You need to figure out a way of moving the queries outside the loop – one every for each opportunity will lead to too many queries.
Some pointers...

You might have to use maps to make sure parallel lists don't create problems - see the platform cook book - I saw it somewhere in it.
May be use KeySet instead of explicity creating arrays of keys. 

Ids[] OpportunityIds = new Id[0];
for (Opportunity o:opps){
   OpportunityIds.add(o.id);
}
Id[[] ContactIds = [select contactIds from opportunitycontactrole where role = 'Ship To' AND Opportunityid in OpportunityIds]
Contact[] Contact_List = [select id, Title from contact where Id in ContactIds]
philbophilbo
So - in English terms, what you're trying to do is...

For each input Opportunity whose status is 'Shipped' or 'Definite',
    - Grab the associated contacts thru the Opp's OpportunityContactRole related list,
      whose Role is 'Ship To'
    - Look for any line items thru the Opp's OpportunityLineItem detail list,
      whose PriceBookEntry reference in turn refers to a Downloadable Product2
    - If there are any such Downloadable line items,
       - Mark each of the above list of contacts as 'Portal User'.

Is this more or less correct?
tschatztschatz
Thank you for the replys!
 
philbo, yes that is exactly what I am trying to accomplish.
 
_t
 
 
 
Ron HessRon Hess
for your first loop on opp, consider a single relationship query

for example
Select o.Opportunity.Amount, o.Opportunity.StageName, o.OpportunityId, o.Contact.LastName, o.ContactId From OpportunityContactRole o


will pull data that you need from 3 tables and avoid multiple queries

in the second loop, consider this relationship query
Select p.Product2.IsActive, p.Product2.ProductCode, p.Product2.Name, p.Product2Id, (Select OpportunityId From OpportunityLineItems where totalprice > 0.0) From PricebookEntry p

where you get product2 custom fields and  opportunity line item fields from a single query

hope this helps, you still have to work this into your code, but relation queries are very powerful, you should use them.
GoForceGoGoForceGo
Relationship queries are more elegent - but I thought they count as a separate query for Apex Governor limits purpose.
tschatztschatz
Hi GoForceGo.
 
Oddly with this Class the first for loop will never contain more than one item. I know there's no need to have this in a for loop, but it's also not hurting anything.
 
Anyway, I think I see where you are going with your example and would like to apply it to my second for loop, where there will be more than one item. However, when I apply the code, as you have written it, to my first for loop I get an unexpected token error when it is looking at the . . . AND Opportunityid in OpportunityIds. It doesn't know what OpportunityIds is. I also removed the extra [ at the start of that line, so that shouldn't be part of the cause. Any thoughts on what's causing this?
 
Ron, Thank you for the example of Relationship queries. I will try to incorporate that into my code as well, if possible.
 
Thanks.
 
_t
GoForceGoGoForceGo
Did you use : - i,e Opportunity in :OpportunityIds
tschatztschatz
Yes. But then I got an
 
Illegal assignment from LIST:SOBJECT:OpportunityContactRole to LIST:Id
 
So I tried:
 
Id[] ContactIds = [select contactId from opportunitycontactrole where role = 'Ship To' AND Opportunityid in :OpportunityIds].contactid;
 
and got this error:
 
Illegal assignment from Id to LIST:Id
 
_t
philbophilbo
Hey,

Here's an example of how you could go about this, with only three SOQL statements (no relationship queries) and one DML.
Note:  don't expect this code to compile first time round, but it should be pretty close to workable, and I think the underlying ideas
are sound.  It might be possible to optimize the logic further, but not too much further.

-philbo

public class CP_OpportunityClosedWon {
    public static void CP_createNewCPUser(Opportunity[] opps){

        Set<Id> oppIdSet = new Set<Id> {};
       
//      Grab only the opportunities with Shipped or Definite status.
//      ------------------------------------------------------------       
        for (Opportunity o:opps){
            if (o.Order_Status__c == 'Shipped' || o.Order_Status__c == 'Definite') {
                oppIdSet.add ( o.Id );
            }
        }
        if ( oppIdSet.isEmpty () ) {
            return;        // nothing to do!
        }
       
//      Get a set of Contact Ids for each of these Opportunities, with Ship To role.
//      ----------------------------------------------------------------------------       
        Map<Id , Set<Id>> contactIdsByOppId = new Map<Id , Set<Id>> {};
        for ( OpportunityContactRole ocr : [select OpportunityId , ContactId
                                            from OpportunityContactRole
                                            where Role = 'Ship To'
                                            and OpportunityId in :oppIdSet
                                        ] ) {
            if ( !contactIdsByOppId.containsKey ( OpportunityId ) ) {
                contactIdsByOppId.put ( OpportunityId , new Set<Id> {} );
            }
            contactIdsByOppId.get ( OpportunityId ).add ( ContactId );
        }
                   
//      Inspect the Opportunity Line Items for each of these Opportunities, looking for Line Items
//      associated with Downloadable Products.
//      If a Downloadable Line Item is found, mark the corresponding Opportunity's Contacts for
//      update.
//      ------------------------------------------------------------------------------------------     
        Set<Id> updContactIdSet = new Set<Id> {};
        for OpportunityLineItem oli : [select OpportunityId , PriceBookEntry.Product2.Downloadable__c
                                        from OpportunityLineItem
                                        where OpportunityId in :oppIdSet
                                    ] ) {
            if ( oli.PriceBookEntry.Product2 != Null
                    && oli.PriceBookEntry.Downloadable__c == True ) {
                updContactIdSet.addAll ( contactIdsByOppId.get ( OpportunityId ) );
            }
        }
       
//      Finally, grab the Contacts to update, and apply the updates.
//      ------------------------------------------------------------       
        Contact[] updContactList = [select Id , Title , Portal_User__c , Portal_URL__c
                                    from Contact
                                    where Id in :updContactIdSet
                                ];
        for ( Contact c : updContactList ) {
            c.Portal_User__c = True;
            c.Portal_URL__c = c.Title;
        }
        update updContactList;
    }
}
tschatztschatz
philbo!
 
You are a Rock & Roll GOD! Thank you! Wow!
 
You were right. I did have to make a few minor changes / edits, but it WORKS!
 
OMG I would have never been able to come up with that from the reference manual, but with this real-world example (at least real world for me) I think I will be able to understand these concepts moving forward.
 
Thank you again. You have saved at least 200 hairs from ending up on my pillow tomorrow morning.
 
Here's the updated code in case anyone else wants to follow this example.
 
Code:
public class CP_OpportunityClosedWon {
 public static void CP_createNewCPUser(Opportunity[] opps){
  Set<Id> oppIdSet = new Set<Id> {};
  //  Grab only the opportunities with Shipped or Definite status.
  //  ------------------------------------------------------------
  for (Opportunity o:opps){
   if (o.Order_Status__c == 'Shipped' || o.Order_Status__c == 'Definite') {
    oppIdSet.add ( o.Id );
   }
  }
  if ( oppIdSet.isEmpty () ) {
   return;
   // nothing to do!
  }
  //  Get a set of Contact Ids for each of these Opportunities, with Ship To role.
  //  ----------------------------------------------------------------------------
  Map<Id , Set<Id>> contactIdsByOppId = new Map<Id , Set<Id>> {};
  for (OpportunityContactRole ocr : [select OpportunityId, ContactId from OpportunityContactRole where Role = 'Ship To' and OpportunityId in :oppIdSet]) {
   if ( !contactIdsByOppId.containsKey ( ocr.OpportunityId ) ) {
    contactIdsByOppId.put ( ocr.OpportunityId , new Set<Id> {} );
   }
   contactIdsByOppId.get ( ocr.OpportunityId ).add ( ocr.ContactId );
  }
  //  Inspect the Opportunity Line Items for each of these Opportunities, looking for Line Items
  //  associated with Downloadable Products.
  //  If a Downloadable Line Item is found, mark the corresponding Opportunity's Contacts for
  //  update.
  //  ------------------------------------------------------------------------------------------
   Set<Id> updContactIdSet = new Set<Id> {};
  for (OpportunityLineItem oli : [select OpportunityId , PriceBookEntry.Product2.Downloadable__c from OpportunityLineItem where OpportunityId in :oppIdSet] ) {
   if ( oli.PriceBookEntry.Product2 != Null && oli.PriceBookEntry.Product2.Downloadable__c == True ) {
    updContactIdSet.addAll ( contactIdsByOppId.get ( oli.OpportunityId ) );
   }
  }
               //  Finally, grab the Contacts to update, and apply the updates.
        //  ------------------------------------------------------------
  Contact[] updContactList = [select Id , Title , Portal_User__c , Portal_URL__c from Contact where Id in :updContactIdSet ];
  for ( Contact c : updContactList ) {
   c.Portal_User__c = True;
   c.Portal_URL__c = c.Title;
  }
  update updContactList;
 }
}

 
Thanks to all who added to this thread. Your comments and advice is appreciated
 
_t


Message Edited by tschatz on 03-27-2008 12:42 PM
GoForceGoGoForceGo
From Language reference manual:

* In a SOQL query with parent-child relationship sub-queries, each parent-child relationship counts as an additional query.
These types of queries have a limit of three times the number for top-level queries. The row counts from these relationship
queries contribute to the row counts of the overall script execution.

I take it to mean that if the limit is queries 20, you can have upto 60 relationship queries??. - so it does help to write relationship queries!!

.
tschatztschatz
Great info! Thanks.
 
So I wonder why relationship queries count as less than regualr queries. Does SOQL treat each query as an open connection to the database and since the connection is already open the relationship query costs less?
 
_t