improving performance - how to pass a table as parameter in sql server 2005?

Question

I have been monitoring the execution of my stored procedures in my webservers in order to tune them for best performance.

I will call the procedure that has most executions per day as myprocedure and its code is here:

create  procedure [dbo].[myprocedure] (  
    @Tier1 varchar(10),  
    @LanguageID int,  
    @SeasonItemID VARCHAR(5) = NULL,
    @ListNoteTypeCode varchar(max),  
    @CacheDuration int output )  
    with execute as 'myuser'  
as

declare @NoteTypeCodeTable table (  
    NoteTypeCode varchar(50) )  
declare @ListNoteTypeCodeXml xml  

set @ListNoteTypeCodeXml = convert(xml, @ListNoteTypeCode)  

insert  into @NoteTypeCodeTable ( NoteTypeCode )  
        select  BulletPoint.NoteTypeCode.value('.', 'varchar(50)') as NoteTypeCode  
        from    @ListNoteTypeCodeXml.nodes('/BulletPoint/NoteTypeCode') as BulletPoint ( NoteTypeCode )  


select  pbp.Tier1, pbp.LanguageId, pbp.NoteText, pbp.NoteTypeCode,  
        pbp.NoteGroup, pbp.SortOrder  
from    dbo.ProductBulletPoint pbp  
join    @NoteTypeCodeTable ntc on pbp.NoteTypeCode = ntc.NoteTypeCode  
where   Tier1 = @Tier1 and  
        LanguageId = @LanguageID and
        (
        SeasonItemId = @SeasonItemID  
        or
        @SeasonItemID is null
        )

select  @CacheDuration = Duration  
from    dbo.CacheDuration  
where   [Key] = 'Product'

GO

This procedure receives a parameter @ListNoteTypeCode that is used inside the procedure to convert into a table variable all the possible values, most of the time one single value.

the table variable, later is used in a join.

how can I improve the performance of this stored procedure? the bit with the xml part is high cpu intensive, is there another way to pass a table to a procedure, making best usage of CPU?

I still have to run a good trace and identify how often this procedure results in more than one value in @ListNoteTypeCode, but regardless of this I still need to find a solution.

I have set up a server side trace in order to identify the possible ways this procedure is generally called. The trace can be seen here.

So far I have only seen variations of the following(with no value to @ListNoteTypeID):

declare @p5 int  
set @p5=NULL  
exec dbo.udpProductBulletPointSelectByTier1NoteTypeCode 
@Tier1=N'AR740',
@LanguageID=1,
@SeasonItemID=N'16HSM',
@ListNoteTypeCode=N'',
@CacheDuration=@p5 output  
select @p5

that produces the following query plan in XML:

query plan procedure dbo.udpProductBulletPointSelectByTier1NoteTypeCode

If I find anything different I will update this question.

I am using sql server 2005 enterprise edition.

The new procedure

Based on the answer by srutzky and many of the comments and other answers, I came out with the following new procedure:

CREATE PROCEDURE [DENORMV2].[udpProductBulletPointSelectByTier1NoteTypeCode] (  
    @Tier1 VARCHAR(10),  
    @LanguageID INT,  
    @SeasonItemID VARCHAR(5) = NULL,
    @ListNoteTypeCode XML,  
    @CacheDuration INT OUTPUT )  
    WITH EXECUTE AS 'webUserWithRW'  
AS

        SELECT  pbp.Tier1, pbp.LanguageId, pbp.NoteText, pbp.NoteTypeCode,  
                pbp.NoteGroup, pbp.SortOrder  
        FROM    dbo.ProductBulletPoint pbp  

        WHERE   Tier1 = @Tier1 
          AND   LanguageId = @LanguageID 
          AND   (      SeasonItemId = @SeasonItemID  
                  OR
                       @SeasonItemID is null
                )

          AND pbp.NoteTypeCode IN (

                 SELECT  NoteTypeCode=BulletPoint.NoteTypeCode.value('./text()[1]', 'varchar(50)')
                   FROM  @ListNoteTypeCode.nodes('/BulletPoint/NoteTypeCode') AS BulletPoint ( NoteTypeCode )

          )

SELECT  @CacheDuration = Duration  
FROM    dbo.CacheDuration  
WHERE   [Key] = 'Product'
GO

when I run the following, in order to compare both procedures:

SET STATISTICS IO ON
SET STATISTICS TIME ON


use US16HSMMProduct_ORIGINAL
go

declare @p5 int  set @p5=86400  
exec dbo.udpProductBulletPointSelectByTier1NoteTypeCode 
@Tier1=N'WW099',
@LanguageID=3,
@SeasonItemID=N'16AUT',
@ListNoteTypeCode=N'<BulletPoint><NoteTypeCode>GarmentComposition</NoteTypeCode><NoteTypeCode>FootwearAccessoryComposition</NoteTypeCode></BulletPoint>',
@CacheDuration=@p5 output  select @p5

use US16HSMMProduct_AFTER_CHANGES
go
declare @p5 int  set @p5=86400 
exec DenormV2.udpProductBulletPointSelectByTier1NoteTypeCode
@Tier1=N'WW099',
@LanguageID=3,
@SeasonItemID=N'16AUT',
@ListNoteTypeCode=N'<BulletPoint><NoteTypeCode>GarmentComposition</NoteTypeCode><NoteTypeCode>FootwearAccessoryComposition</NoteTypeCode></BulletPoint>',
@CacheDuration=@p5 output  select @p5

I get this from SQL Sentry plan explorer (partial view):

and this is from sql server (partial view)

Answer 1

Based on your code alone, I would suggest changing the XQuery in the .value() function to be ./text()[1] instead of just .. Doing this will get you closer to the efficiency of attribute-based XML:

select  BulletPoint.NoteTypeCode.value('./text()[1]', 'varchar(50)') as NoteTypeCode

Other minor considerations:

• Change @ListNoteTypeCode datatype to be XML so that you do not need the CONVERT step.
• If you can't pass in an XML type, at the very least change the @ListNoteTypeCode datatype to be NVARCHAR(MAX) so that no implicit conversion needs to be done since XML is UTF-16, same as NVARCHAR / NCHAR.

Major consideration: if the datatype of the ProductBulletPoint.NoteTypeCode column is really VARCHAR (which it seems to be since there is no implicit conversion in the execution plan), then:

• Ideally you shouldn't do string comparisons on codes. Codes should be defined in a lookup table that you can translate into a numeric key that will be the value that is stored in the ProductBulletPoint table, just like the @LanguageID. So there should be a NoteTypeCodes table similar to:

  CREATE TABLE dbo.NoteTypeCodes
  (
    NoteTypeCodeID TINYINT NOT NULL
                   CONSTRAINT [PK_NoteTypeCodes] PRIMARY KEY,
    NoteTypeCode   VARCHAR(50) NOT NULL
  );
  

  Then, it would be best to have the app code pass in a list of the IDs so you would only need to split them with a simple string splitter into @NoteTypeCodeTable which would have a single TINYINT column. Otherwise, you would need to split the code names and JOIN to NoteTypeCodes to get the ID values to insert into @NoteTypeCodeTable.

• If you can't change the NoteTypeCode column to be NoteTypeCodeID TINYINT, then at the very least consider using a binary Collation (i.e. one ending in _BIN2 if available) on that column since it is highly doubtful that there is any need for case-insensitive and/or culturally-aware comparisons of codes. You can alter the column using:

  ALTER TABLE [dbo].[ProductBulletPoint]
    ALTER COLUMN [NoteTypeCode] {current_datatype}
    COLLATE Latin1_General_100_BIN2 {current_NULL_or_NOT-NULL_setting};
  

  You might need to drop and re-create any indexes that use the [NoteTypeCode] column. But this one-time fix will mean that you don't need to add COLLATE Latin1_General_100_BIN2 to any queries using this column as Collation Precedence will take care of it.

  However, you will have to normalize the code values as they are inserted / updated by calling LOWER on the incoming values. You can do this in each stored procedure that does an INSERT and/or UPDATE, or you can handle it automatically in one spot via an AFTER INSERT, UPDATE Trigger. Then you just need to remember to use LOWER on values use in joins and WHERE conditions.

• This advice would most likely also apply equally to the SeasonItemId column and @SeasonItemID input parameter.

Based on the posted XML execution plan and what you found while doing the trace, it would seem that what might possibly be the most impacting factor is that the Stored Procedure is being called with @ListNoteTypeCode being set to empty string. This results in no rows getting inserted into the @NoteTypeCodeTable Table Variable. And, because @NoteTypeCodeTable is used as a filter (since it is Inner Joined to ProductBulletPoint), that results in no rows getting returned from the main query. Hence, any work being done by this stored procedure is all for nothing if @ListNoteTypeCode is ever set to empty string.

Given the frequency of calls to this Stored Procedure, you need to do one of the following:

1. Since @ListNoteTypeCode is a required parameter and an empty string is not a valid value, this could indicate a bug in the app code. If this is a bug, fix the app code to not call this stored procedure when @ListNoteTypeCode is empty.

2. If for some reason it is valid for the app code to call this Stored Procedure with @ListNoteTypeCode being empty, then when that empty value is passed in, skip the actual processing and simply return an empty result set (this will appear to behave the same to all callers, hence no app code will break if any code expects to get a result set when passing in an empty string):

   create procedure [dbo].[myprocedure] (
   ...
   as
   
   IF (@ListNoteTypeCode <> '')
   BEGIN
     declare @NoteTypeCodeTable table ( NoteTypeCode varchar(50) )
     ...
     select  pbp.Tier1, pbp.LanguageId, pbp.NoteText, pbp.NoteTypeCode,
             pbp.NoteGroup, pbp.SortOrder
     from    dbo.ProductBulletPoint pbp
     join    @NoteTypeCodeTable ntc on...
     ...
   END;
   ELSE
   BEGIN
     select  pbp.Tier1, pbp.LanguageId, pbp.NoteText, pbp.NoteTypeCode,
             pbp.NoteGroup, pbp.SortOrder
     from    dbo.ProductBulletPoint pbp
     WHERE   1 = 0;
   END;
   
   select  @CacheDuration = Duration
   ...
   

Answer 2

Can we see an example execution plan? Is the column NoteTypeCode really varchar(50) in the table ProductBulletPoint? - seems strangely long for a "code" and could be some implicit type conversion here. If you are able to change the calling code as well, consider having a simple version for single code lookups. Why are you checking the CacheDuration table 500,000 times (presumably more as this is just for "Product") per day? Maybe there is another way you can handle that as well? +1 for (./text())[1] and +1 for not using XML for a simple list.