Take the 2-minute tour ×
Database Administrators Stack Exchange is a question and answer site for database professionals who wish to improve their database skills and learn from others in the community. It's 100% free, no registration required.

I have recently inherited a codebase with a large amount of Stored Procedures. The system they are supporting is encountering numerous performance problems which I am looking in to.

A number of the Stored Procedures have a pattern like this:

  1. Create Temp table
  2. Build up dynamic SQL query to insert a bunch of records, e.g.

    DECLARE @sql VARCHAR(MAX)
    SET @sql = 'INSERT INTO @tempTable
    SELECT SomeColumn, SomeColumn2, SomeColumn3, etc FROM MyTable'
    
    IF @someParam = [SomeValue]
        SET @sql = @sql + 'WHERE SomeColumn = [SomeValue]';
    
    IF @someOtherParam = [SomeOtherValue]
        SET @sql = @sql + 'WHERE SomeOtherColum = [SomeOtherValue]';
    
  3. Execute this dynamic sql

    EXEC(@sql);
    
  4. Select from the temp table and bring in a bunch of additional information to return to the client.

    SELECT
    
       ...
    
    FROM
    
       @tempTable
       INNER JOIN ...
    

My immediate thoughts are:

  • There is dynamic SQL, so no cache plans, meaning plans generated every time.
  • There is an INSERT SELECT pattern, so table locking is more likely to be an issue.

I have re-written some of the Stored Procedures in this way instead:

    SELECT 

       ...

    FROM

        MyTable 
        INNER JOIN ...

    WHERE

    (
        @someParam != SomeValue
        OR
        SomeColumn = SomeValue
    )
    AND
    (
        @someOtherParam != SomeOtherValue
        OR
        SomeOtherColumn = SomeOtherValue
    )        

From comparing execution plans and client statistics in SQL Management Studio, I have not sped the Stored Procedures up so I am apprehensive about suggesting wholesale re-writes of all Stored Procedures.

I am trying to set up some profiling of a live customer scenario, but as yet have been unable to prove my thoughts.

Can anyone offer any confirmation of the theory behind my thoughts, or any better ways of proving my suspicions?

The problem is I have read that dynamic SQL is not always a closed case - i.e. it depends on how it is used. My understanding of locking also falls down at the fact that nowhere can I get 100% confirmation of how this type of INSERT SELECT will lock tables.

share|improve this question
    
This looks awfully like an "Optional Parameters" problem. If you want the best performance then you write a stored procedure for each iteration of the query and call the correct one based on the values supplied. Whilst this might be a pain to write, you will benefit from the greatest performance and be able to tune each instance separately. –  gvee Sep 16 '13 at 10:43
    
As for locking.. You could use the (NOLOCK) hint on each table. Make sure you read up on this first though. –  gvee Sep 16 '13 at 10:44
    
@gvee Yep, optional parameters. But there are billions of them! Will keep in mind though, thanks. As for NOLOCK hint - I have used them to tweak performance in the past, but I know they are only supposed to be a last resort, right? –  OffHeGoes Sep 16 '13 at 10:51
    
@OffHeGoes it depends on your requirements. NOLOCK will not lock the table as it only reads committed rows. –  gvee Sep 16 '13 at 11:01
1  
@gvee I didn't think it was as simplistic as that, from what I have read. See this "NOLOCK queries also run the risk of reading "phantom" data, or data rows that are available in one database transaction read but can be rolled back in another." –  OffHeGoes Sep 16 '13 at 11:07

5 Answers 5

up vote 15 down vote accepted

There is dynamic SQL, so no cache plans, meaning plans generated every time

Not necessarily true. Dynamic SQL can (and does) use cached plans just as well as static SQL. For dynamic search conditions resolving to dynamic SQL is oft the right answer. See Dynamic Search Conditions in T-SQL for more details.

There is an INSERT SELECT pattern, so table locking is more likely to be an issue.

Not necessarily true, specially with a @tempTable

have re-written some of the Stored Procedures in this way instead

Using multiple OR conditions like that is an anti-pattern. You are forcing the query optimizer to come up with a plan that works for any value of all those parameters. Usually the only solution is a scan, ignoring any index. The original code was better.

Can anyone offer ... any better ways of proving my suspicions?

Yes. Measure. Use a methodology like Waits and Queues. Don't relly on your intuition. Find the bottlenecks and address them accordingly.

share|improve this answer
    
I am talking about the lock on MyTable in the case of INSERT INTO @TempTable SELECT FROM MyTable - Is this not introducing an uneccessary Update lock on MyTable? –  OffHeGoes Sep 16 '13 at 10:56
2  
Is not an update lock to start with. Is a read (SELECT) so will lock according to your isolation level for reads. A table scan may escalate, but that is the due to the scan (ie. your WHERE conditions in the SELECT). INSERT has nothing to do with either. –  Remus Rusanu Sep 16 '13 at 10:59
    
Ok, thanks for confirming about the locking - I could not find details on this. –  OffHeGoes Sep 16 '13 at 11:11
1  
Really, really helpful link to Erland Sommarskog! Should have thought to look there as previously been through his Error handing –  OffHeGoes Sep 16 '13 at 11:29
3  
Do you sleep Mr. Remus? Hat's off to your contributions here. –  Kermit Sep 16 '13 at 12:47

Actually, I think the original works better.

Having "or" clause in where statements usually slows down performance noticeably.

And I don't think it's true that the dynamic sql query gets reevaluated each time. As far as I remember, the DBMS will cache the query plan based on query text. But if you modify even one space in it, it will get reevaluated. So, to optimize the dynamic query it would definitely be better if you used variables in the text (@myValue) instead of concatenating their values (you pass the variables values when calling the exec).

As for the temp table - it's a variable, which means it's a bit faster/less resource demanding than the actual temp table (e.g. #table). We had recently a similar problem where the server took long times to execute a complicated query and the optimization turned out to be to first trim down results and after that do the rest of the joins, which is exactly what they're doing here. When this turns out to be the issue, one first tries to do a statistics update for all tables, but if that doesn't do the trick, then this would actually be the solution.

share|improve this answer
    
Yes, I plan to change EXEC for sp_executeSql, although will be sure to refer to this to decide what gets added as a parameter, and what gets 'hardcoded' depending on the plans. –  OffHeGoes Sep 16 '13 at 15:19
    
n many cases the same table is joined in the original INSERT SELECT and then again in the final SELECT so I will need to see in my case whether the temp table is required or not. –  OffHeGoes Sep 16 '13 at 15:20

There is dynamic SQL, so no cache plans, meaning plans generated every time.

This has been untrue since SQL Server 7.0, which is very old.

There is an INSERT SELECT pattern, so table locking is more likely to be an issue.

Temp table has separate lock. The insert won't lock worse than when you do that directly in a select. Main point is that this means the select only starts after the insert, which means initial data comes later (insert has to FINISH, not delivering data first). On top for no gain you put pressure on tmpdb which is the db quite likely to get too much IO prssure. I would not do that—and I would argue it is bad code—your arguments are not valid.

share|improve this answer
    
Probably should have explained I'm not a DBA - originally posted on SO –  OffHeGoes Sep 16 '13 at 10:57
    
Interesting that you talk about tempdb and IO pressure as this is one of the things we have noticed - high IO. Would you say that the use of the Temp Table could be a contributing factor? –  OffHeGoes Sep 16 '13 at 11:14
1  
Yes. I would not use them unless I have to - and in this example there is no benefit gained by using tempdb at all. This is a lot of table creation and deletion - with zero benefit (in this example - i am not against them in general). –  TomTom Sep 16 '13 at 11:48
    
In many cases the same table is joined in the original INSERT SELECT and then again in the final SELECT so I think that my first port of call is to analyse whether the temp table is required. –  OffHeGoes Sep 16 '13 at 13:04
1  
just providing more background info –  OffHeGoes Sep 16 '13 at 13:23

Dynamic SQL runs as fast as 'compiled' SQL in recent editions of SQL Server (for the purposes of us SQL layman at least) so simply don't worry about that until all other options have been explored. The main argument against dynamic SQL is maintainability (readability, testability, clarity, etc).

In support of this, you've written the code out to the generated conclusion and performance is roughly the same.

Whenever I look at SQL I'm always looking at the same patterns for serious performance bottlenecks - nested queries, joins, and where clauses. Roughly in that order.

You don't have any nested queries, but you do have joins and where clauses. Without seeing you real database it's extremely difficult to identify problems but the simplest change you can make is index optimization.

  1. Run query analyzer (in the Tools menu), and read the suggestion
  2. Run the query without any WHERE clauses (but just select COUNT(1) to avoid all the print-to-screen overhead). Is it quicker, or the same? If it's quicker, then the WHERE clauses are causing problems. If it's the same, then it's likely the JOIN
  3. I'll guess that it's there WHERE statements, so simply identify if you need indexes on any of the columns and what type of indexes. Indexing is a whole other question and depends a whole lot on how the table is used in other parts of the system, but you can test it by adding some indexes on the most-frequently filtered columns to test performance improvement.

Remember the downsides of indexes - slower insert & update performance, more disk space used.

share|improve this answer

Basically I suggest, what a commenter already wrote. Write a statement for each IF-condition. Not that much work:

IF @someParam = [SomeValue]
BEGIN 
  INSERT INTO @tempTable
  SELECT SomeColumn, SomeColumn2, SomeColumn3, etc 
  FROM MyTable
  WHERE SomeColumn = [SomeValue]
END;

IF @someOtherParam = [SomeOtherValue]
BEGIN 
  INSERT INTO @tempTable
  SELECT SomeColumn, SomeColumn2, SomeColumn3, etc 
  FROM MyTable
  WHERE SomeOtherColum = [SomeOtherValue]
END;

Now there is no dynamic stuff. Also consider an indexes on columns of the where clauses, the select could potentially run faster. Should you also do updates depending on conditions you should go for a merge statement.

I was wondering: why would an INSERT SELECT provoke more locks than a SELECT?

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.