How to optimize this dynamic stored procedure

I have a stored procedure which is retrieving data using 20 tables. Sample of the procedure:

     CREATE PROCEDURE GetEnquiries 
        @EnquiryDate    DATETIME     = NULL

     DECLARE @querySELECT      VARCHAR(MAX) = ''
     DECLARE @queryWHERE         VARCHAR(MAX) = ''
     DECLARE @queryExtraColumns     VARCHAR(MAX) = ''
     DECLARE @queryReturnResults    VARCHAR(MAX) = ''

     --Create temp table
     SET    @querySELECT = '
             CREATE   TABLE #tempResults 
              EnquiryId     INT, 
              Cost         Decimal(18,2),
              CustomerName    VARCHAR(50),
              EnquiryStatus   VARCHAR(50),
              ContactNumber   VARCHAR(50),
              NumberOfVisits  INT 
             ) '

     --Insert into temp table
     SET    @querySELECT = '      
             INSERT INTO #tempResults 
              EnquiryId     , 
              Cost         ,
              CustomerName    ,
              EnquiryStatus   ,
             ) '
     SET    @querySELECT = '
              e.EnquiryId     , 
              e.Cost       ,
              c.CustomerName  ,
              e.EnquiryStatus ,
           FROM  Enquiry e
              INNER JOIN Customers c ON e.CustomerId = c.CustomerId '

     -- WHERE 
     IF(@EnquiryDate IS NOT NULL)
           SET @queryWHERE = @queryWHERE + ' CONVERT(VARCHAR(10),e.EnquiryDate,23) >= '  + ''''+ CONVERT(VARCHAR(10),@EnquiryDate,23) + ''''

     --- There are at least 14 parameters used in WHERE operation the above is just one of them
     -- Count NumberOfVisits
      SET   @queryExtraColumns = '
           ;WITH NumberOfVisits AS
             SELECT  t.EnquiryId, COUNT(EnquiryId) AS NumberOfVisits 
             FROM NumberOfVisits v 
                  INNER JOIN #tempResults t ON v.EnquiryId = t.EnquiryId
             GROUP   BY t.EnquiryId

       UPDATE    #tempResults
       SET     NumberOfVisits = u.NumberOfVisits
       FROM  #tempResults t
          INNER JOIN NumberOfVisits u ON u.EnquiryId = t.EnquiryId


     -- return the results
     SET    @queryReturnResults = '
              EnquiryId     , 
              Cost         ,
              CustomerName    ,
              EnquiryStatus   ,
              ContactNumber   ,
           FROM  #tempResults t 

     -- Combine all the strings + DROP the temp table
     -- PRINT(  @querySELECT + ' WHERE ' +   @queryWHERE + @queryExtraColumns +   @queryReturnResults + '  DROP TABLE #tempResults ') 
      EXEC( @querySELECT + ' WHERE ' +  @queryWHERE + @queryExtraColumns +  @queryReturnResults + '  DROP TABLE #tempResults ') 

Some facts:

  • The above procedure is the simple form of the Stored procedure i am working on.

  • I am using SQL Server 2008

  • My Actual procedure has 15 parameters, all of them are used in WHERE clause. If the value is provided for a parameter, the parameter is included in the WHERE clause otherwise not.

  • There are at least 10 columns whos value comes from the GROUP BY condition like the one "NumberOfVisits" given in the above procedure.

  • I have indexes on all the Primary Keys & Foreign Keys.

  • I have indexes on all the columns that are used in the WHERE clause.

  • I have indexes on all the columns that are used in the GROUP BY clause.


  • Is this is according to the best practice to create dynamic stored procedures following above pattern?

  • I got the output SQL of this procedure by using: -- PRINT( @querySELECT + ' WHERE ' + @queryWHERE + @queryExtraColumns + @queryReturnResults + ' DROP TABLE #tempResults ') when i run that SQL it took the same time that was taken by the stored procedure, why? is the SQL should take less time?

  • Is the above is the best practice to get the value of summary columns("NumberOfVisits") ?

  • is the above is the best way to create the WHERE clause dynamically?

  • Can i avoid the use of Temporary table by using some alternate in the above scenario?

  • What can i do to optimize this procedure?

Please forgive me, if my question is NOT clear or not a proper question.

Thanks for your valuable time & help.
more ▼

asked Nov 16, 2011 at 10:26 AM in Default

yaqubonnet gravatar image

247 16 17 20

(comments are locked)
10|1200 characters needed characters left

2 answers: sort oldest

In general, instead of building a string and executing it, which can be subject to SQL injection attacks, I'd suggest building a string with parameters and using sp_executesql to execute it, providing the parameters. It's more work, but it's more secure, AND, it's more likely to see improved plan reuse.

No, the query is the long part of the execution. Building a few strings is pretty pain free.

In terms of summary columns, no, that's an extra pass. Instead I'd perform a join against a sub-select (usually, sometimes breaking it down into steps can work better).

Nope. See above. sp_executesql is better.

Yeah, plan it out as a single select statement. The temp table is just holding for the aggregation which can be done as a derived table.

Optimize? Tougher question. Just seeing what you have, the CONVERT statement on e.EnquiryDate is going to cause a scan, no matter what. If that's a datetime column, you need to compare that to a datetime value, no conversions. Beyond that, I couldn't say without seeing the whole queries, execution plans, data, structure, all that.

In general, these catch all type queries are extremely problematic. Instead of doing this, identify the common patterns that will inevitably exist, these three columns always come in, this one only comes in when another one comes in, etc., and then build three or four different procs that take take into account these patterns and use this as a wrapper proc to determine which of those procs it should go to. It'll be work to set up, but no more than this, and it will perform better and will be easier to maintain.
more ▼

answered Nov 16, 2011 at 11:36 AM

Grant Fritchey gravatar image

Grant Fritchey ♦♦
103k 19 21 74

@Grant Fritchey: Great Thanks
Nov 16, 2011 at 11:43 AM yaqubonnet
(comments are locked)
10|1200 characters needed characters left
Your answer
toggle preview:

Up to 2 attachments (including images) can be used with a maximum of 524.3 kB each and 1.0 MB total.

New code box

There's a new way to format code on the site - the red speech bubble logo will automatically format T-SQL for you. The original code box is still there for XML, etc. More details here.

Follow this question

By Email:

Once you sign in you will be able to subscribe for any updates here



Answers and Comments

SQL Server Central

Need long-form SQL discussion? SQLserverCentral.com is the place.



asked: Nov 16, 2011 at 10:26 AM

Seen: 780 times

Last Updated: Nov 16, 2011 at 10:26 AM