SP- Performance improvement

  • I am trying to improve the performance of this stored procedure. Any suggestions on how to improve its performance?

    ---

    USE [Majestic]

    GO

    /****** Object: StoredProcedure [dbo].[MajesticMapping_MapDocument] ******/

    SET ANSI_NULLS ON

    GO

    SET QUOTED_IDENTIFIER ON

    GO

    ALTER PROCEDURE [dbo].[MajesticMapping_MapDocument]

    @CorrectedDocumentID INT,

    @OrderID INT = NULL, -- Optional

    @FillTempTable BIT = 0 -- If true, caller must supply #TEMP_TagValues table

    AS BEGIN

    SET NOCOUNT ON

    DECLARE @StartTime0 AS DATETIME

    SET @StartTime0 = GETDATE()

    IF @OrderID <= 0 SET @OrderID = NULL

    -- If @FillTempTable is true, make sure #TEMP_TagOutput looks like this, too:

    DECLARE @TagValues TABLE (

    FieldName VARCHAR(50),

    Value NVARCHAR(MAX),

    TagFileMappingID INT,

    FormatMethods VARCHAR(1000),

    CdaDataType VARCHAR(50)

    )

    DECLARE @GsmTagValues AS TABLE (

    Name VARCHAR(50),

    Value NVARCHAR(MAX)

    )

    DECLARE @UdfResults TABLE (Value NVARCHAR(MAX))

    DECLARE @ErrorMessage VARCHAR(1000)

    DECLARE @StartTime1 AS DATETIME

    DECLARE @StartTime2 AS DATETIME

    --DECLARE @TotalTime_HL7 AS FLOAT SET @TotalTime_HL7 = 0

    DECLARE @TotalTime_SuppressUdfs AS FLOAT SET @TotalTime_SuppressUdfs = 0

    DECLARE @TotalTime_FormatUdfs AS FLOAT SET @TotalTime_FormatUdfs = 0

    DECLARE @SubAccountID INT

    DECLARE @Value NVARCHAR(MAX)

    DECLARE @WhateverIds VARCHAR(MAX)

    DECLARE @UdfName NVARCHAR(MAX)

    DECLARE @UdfCall NVARCHAR(MAX)

    DECLARE @Pos INT

    DECLARE @Suppress BIT

    DECLARE @Sql NVARCHAR(MAX)

    DECLARE @Results TABLE (Value NVARCHAR(MAX))

    -- We need to know which facility this is for. The best candidate for

    -- getting it seems to be the Authors table. That's right: Dictations

    -- doesn't have it.

    SELECT @SubAccountID = SubAccountID

    FROM CorrectedDocuments CD WITH (NOLOCK)

    INNER JOIN Dictations D WITH (NOLOCK)

    ON D.DictationID = CD.DictationID

    INNER JOIN Authors A WITH (NOLOCK)

    ON A.AuthorID = D.AuthorID

    WHERE CD.CorrectedDocumentID = @CorrectedDocumentID

    PRINT '- @SubAccountID = ' + ISNULL(CAST(@SubAccountID AS VARCHAR(20)), 'NULL')

    PRINT '- Fields:'

    --################################ Step 1: Cache the old GSM mappings ################################--

    SET @StartTime1 = GETDATE()

    INSERT INTO @GsmTagValues

    EXEC MajesticTagFileMapping_OldGenericStringMapperMappings @CorrectedDocumentID, @OrderID

    PRINT '- Old GSM stuff took ' + CAST(DATEDIFF(s, @StartTime1, GETDATE()) AS VARCHAR(20)) + ' seconds.'

    --################################ Step 2: Load the Mappings ################################--

    DECLARE @TagFileMappingID INT

    DECLARE @ToFieldName VARCHAR(100)

    DECLARE @SuppressIfUdfs VARCHAR(1000)

    DECLARE @FromType CHAR(2)

    DECLARE @FromText VARCHAR(MAX)

    DECLARE @FromViewName VARCHAR(50)

    DECLARE @FromViewSelector VARCHAR(50)

    DECLARE @FormatUdfs VARCHAR(1000)

    DECLARE @FormatMethods VARCHAR(1000)

    DECLARE @CdaDataType VARCHAR(50)

    DECLARE MappingCursor CURSOR FOR

    -- We want to go through a list of mappings we'll use. This query returns

    -- one row for each tag file output field name, giving preference to

    -- facility-centric overrides and filtering out any where the facility

    -- suppresses output.

    SELECT

    TagFileMappingID,

    ToFieldName,

    SuppressIfUdfs,

    FromType,

    FromText,

    FromViewName,

    FromViewSelector,

    FormatUdfs,

    FormatMethods,

    CdaDataType

    FROM MajesticTagFileMapping M1

    WHERE (

    SubAccountID = @SubAccountID -- Facility-specific mapping.

    OR ( -- Or shared default mapping that is not overridden.

    SubAccountID IS NULL

    AND ToFieldName NOT IN (

    SELECT ToFieldName

    FROM MajesticTagFileMapping

    WHERE SubAccountID = @SubAccountID

    )

    )

    )

    AND Suppress = 0 -- Ignore any where facility suppresses output.

    ORDER BY ToFieldName

    --################################ Step 3: Process each mapping ################################--

    SET @StartTime1 = GETDATE()

    OPEN MappingCursor

    FETCH NEXT FROM MappingCursor

    INTO

    @TagFileMappingID,

    @ToFieldName,

    @SuppressIfUdfs,

    @FromType,

    @FromText,

    @FromViewName,

    @FromViewSelector,

    @FormatUdfs,

    @FormatMethods,

    @CdaDataType

    -- Loop through each mapping.

    WHILE @@FETCH_STATUS = 0 BEGIN

    SET @Value = NULL

    SET @Suppress = 0

    PRINT ''

    PRINT ' - ' + @ToFieldName + ':'

    --################################ Suppress Output? ################################--

    SET @StartTime2 = GETDATE()

    -- Process the output suppression UDFs.

    SET @SuppressIfUdfs = ISNULL(@SuppressIfUdfs, '')

    WHILE @Suppress = 0 AND @SuppressIfUdfs <> '' BEGIN

    -- Parse the next UDF name out of the list.

    SET @Pos = PATINDEX('%|%', @SuppressIfUdfs)

    IF @Pos = 0 BEGIN

    SET @UdfName = @SuppressIfUdfs

    SET @SuppressIfUdfs = ''

    END ELSE BEGIN

    SET @UdfName = SUBSTRING(@SuppressIfUdfs, 1, @Pos - 1)

    SET @SuppressIfUdfs = SUBSTRING(@SuppressIfUdfs, @Pos + 1, LEN(@SuppressIfUdfs))

    END

    SET @SuppressIfUdfs = LTRIM(@SuppressIfUdfs)

    SET @UdfName = LTRIM(RTRIM(@UdfName))

    SET @UdfName = 'MajesticTagFileMapping_Suppress_' + @UdfName

    PRINT ' - Suppress: dbo.' + @UdfName + '(' + CAST(@CorrectedDocumentID AS VARCHAR(20)) + ', ' + CAST(@TagFileMappingID AS VARCHAR(20)) + ')'

    -- We need a valid SQL query. In this case, calling our UDF.

    SET @UdfCall = 'SELECT dbo.' + @UdfName + '(@TagFileMappingID, @CorrectedDocumentID, @OrderID)'

    -- Call the UDF, passing in our arguments.

    DELETE FROM @UdfResults

    BEGIN TRY

    INSERT INTO @UdfResults

    EXEC sp_executesql

    @UdfCall,

    N'@CorrectedDocumentID INT, @TagFileMappingID INT',

    @TagFileMappingID = @TagFileMappingID,

    @CorrectedDocumentID = @CorrectedDocumentID,

    @OrderID = @OrderID

    END TRY

    BEGIN CATCH

    CLOSE MappingCursor

    DEALLOCATE MappingCursor

    SET @ErrorMessage = 'While executing ''' + @ToFieldName + ''' mapping''s call to dbo.' + @UdfName + '(), got this: ' + ERROR_MESSAGE()

    RAISERROR(@ErrorMessage, 11, 1) WITH SETERROR

    RETURN @@ERROR

    END CATCH

    SELECT @Suppress = CAST(Value AS BIT) FROM @UdfResults

    END -- WHILE @SuppressIfUdfs <> '' BEGIN (loop through all output suppression UDFs)

    SET @TotalTime_SuppressUdfs = @TotalTime_SuppressUdfs + DATEDIFF(ms, @StartTime2, GETDATE())

    --################################ Get the Value ################################--

    ----------------------------------------------------------------

    IF @Suppress = 1 BEGIN

    PRINT ' ** Returned true; suppressing output'

    ----------------------------------------------------------------

    END ELSE IF @FromType = 'CV' BEGIN -- Use a predefined constant value.

    PRINT ' - Value: Constant (' + @FromText + ')'

    SET @Value = @FromText

    ----------------------------------------------------------------

    END ELSE IF @FromType = 'VC' BEGIN -- Get the value from the DB view row.

    SET @Sql = 'SELECT TOP 1 [' + ISNULL(@FromText, '?') + '] FROM MajesticTagFileMapping_View_' + ISNULL(@FromViewName, '?') + ' WHERE FILTER_CorrectedDocumentID = ' + CAST(@CorrectedDocumentID AS NVARCHAR(20))

    IF ISNULL(@FromViewSelector, '') <> ''

    SET @Sql = @Sql + ' AND FILTER_Selector = ''' + REPLACE(ISNULL(@FromViewSelector, ''), '''', '''''') + ''''

    IF @OrderID IS NOT NULL

    SET @Sql = @Sql + ' AND FILTER_OrderId = ' + CAST(@OrderID AS NVARCHAR(20))

    PRINT ' - Value: ' + @Sql

    -- Clear the temporary table, first.

    DELETE FROM @Results

    -- Execute the query and pull the results into a single-column temp table.

    INSERT INTO @Results

    EXEC sp_executesql @Sql

    -- Pluck the resulting value out of that temp table.

    SELECT @Value = Value FROM @Results

    ----------------------------------------------------------------

    END ELSE IF @FromType = 'GS' BEGIN -- Get the value from the old generic string mapper.

    PRINT ' - Value: SELECT Value FROM @GsmTagValues WHERE Name = ''' + @FromText + ''''

    -- Execute the query and pull the results into a single-column temp table.

    SELECT @Value = Value FROM @GsmTagValues WHERE Name = @FromText

    ----------------------------------------------------------------

    END ELSE IF @FromType = 'UP' BEGIN -- Get the value from a user-defined field row.

    PRINT ' - Value: MajesticPatientUserDefFields: ' + @FromText

    SELECT TOP 1 @Value = FieldValue

    FROM MajesticADT.dbo.MajesticPatientUserDefFields F

    INNER JOIN ADT_Data_Dictation DD

    ON DD.PatientID = F.PatientID

    WHERE DD.CorrectedDocumentID = @CorrectedDocumentID

    AND F.FieldName = @FromText

    ----------------------------------------------------------------

    END ELSE IF @FromType = 'UE' BEGIN -- Get the value from a user-defined field row.

    PRINT ' - Value: MajesticEncounterUserDefFields: ' + @FromText

    SELECT TOP 1 @Value = FieldValue

    FROM MajesticADT.dbo.MajesticEncounterUserDefFields F

    INNER JOIN ADT_Data_Dictation DD

    ON DD.EncounterID = F.EncounterID

    WHERE DD.CorrectedDocumentID = @CorrectedDocumentID

    AND F.FieldName = @FromText

    ----------------------------------------------------------------

    END ELSE IF @FromType = 'UO' BEGIN -- Get the value from a user-defined field row.

    PRINT ' - Value: MajesticOrderUserDefFields: ' + @FromText

    SELECT TOP 1 @Value = FieldValue

    FROM MajesticADT.dbo.MajesticOrderUserDefFields F

    INNER JOIN ADT_Data_Dictation DD

    ON DD.OrderID = F.OrderID

    WHERE DD.CorrectedDocumentID = @CorrectedDocumentID

    AND F.FieldName = @FromText

    ----------------------------------------------------------------

    END ELSE BEGIN -- What?

    CLOSE MappingCursor

    DEALLOCATE MappingCursor

    SET @ErrorMessage = 'I don''t know how to process FromType = ''' + @FromType + ''' mappings in MajesticTagFileMapping. (See the ''' + @ToFieldName + ''' mapping).'

    RAISERROR(@ErrorMessage, 11, 1) WITH SETERROR

    RETURN @@ERROR

    END

    ----------------------------------------------------------------

    IF @Suppress = 0 BEGIN

    --################################ Format the Value ################################--

    SET @StartTime2 = GETDATE()

    -- We never want leading or trailing spaces, so let's whack them now.

    SET @Value = LTRIM(RTRIM(ISNULL(@Value, '')))

    -- Process the formatting UDFs.

    SET @FormatUdfs = ISNULL(@FormatUdfs, '')

    WHILE @FormatUdfs <> '' BEGIN

    -- Parse the next UDF name out of the list.

    SET @Pos = PATINDEX('%|%', @FormatUdfs)

    IF @Pos = 0 BEGIN

    SET @UdfName = @FormatUdfs

    SET @FormatUdfs = ''

    END ELSE BEGIN

    SET @UdfName = SUBSTRING(@FormatUdfs, 1, @Pos - 1)

    SET @FormatUdfs = SUBSTRING(@FormatUdfs, @Pos + 1, LEN(@FormatUdfs))

    END

    SET @FormatUdfs = LTRIM(@FormatUdfs)

    SET @UdfName = LTRIM(RTRIM(@UdfName))

    SET @UdfName = 'MajesticTagFileMapping_Format_' + @UdfName

    PRINT ' - Format: dbo.' + @UdfName + '(@Value, ' + CAST(@TagFileMappingID AS VARCHAR(20)) + ', ' + CAST(@CorrectedDocumentID AS VARCHAR(20)) + ', ' + ISNULL(CAST(@OrderID AS VARCHAR(20)), 'NULL') + ')'

    -- We need a valid SQL query. In this case, calling our UDF.

    SET @UdfCall = 'SELECT dbo.' + @UdfName + '(@Value, @TagFileMappingID, @CorrectedDocumentID, @OrderID)'

    -- Call the UDF, passing in our arguments.

    DELETE FROM @UdfResults

    BEGIN TRY

    INSERT INTO @UdfResults

    EXEC sp_executesql

    @UdfCall,

    N'@Value NVARCHAR(MAX), @TagFileMappingID INT, @CorrectedDocumentID INT, @OrderID INT',

    @Value = @Value,

    @TagFileMappingID = @TagFileMappingID,

    @CorrectedDocumentID = @CorrectedDocumentID,

    @OrderID = @OrderID

    END TRY

    BEGIN CATCH

    CLOSE MappingCursor

    DEALLOCATE MappingCursor

    SET @ErrorMessage = 'While executing ''' + @ToFieldName + ''' mapping''s call to dbo.' + @UdfName + '(), got this: ' + ERROR_MESSAGE() + ' (@Value = ' + ISNULL(@Value, 'NULL') + ')'

    RAISERROR(@ErrorMessage, 11, 1) WITH SETERROR

    RETURN @@ERROR

    END CATCH

    SELECT @Value = Value FROM @UdfResults

    END -- WHILE @FormatUdfs <> '' BEGIN (loop through all formatting UDFs)

    --################################ Output the Value ################################--

    -- Pack the final value into a list of all property values.

    INSERT INTO @TagValues VALUES (

    @ToFieldName,

    @Value,

    @TagFileMappingID, -- The consumer might care about this.

    @FormatMethods, -- The consumer will be responsible for handling this part.

    @CdaDataType

    )

    SET @TotalTime_FormatUdfs = @TotalTime_FormatUdfs + DATEDIFF(ms, @StartTime2, GETDATE())

    END -- IF @Suppress = 0

    FETCH NEXT FROM MappingCursor

    INTO

    @TagFileMappingID,

    @ToFieldName,

    @SuppressIfUdfs,

    @FromType,

    @FromText,

    @FromViewName,

    @FromViewSelector,

    @FormatUdfs,

    @FormatMethods,

    @CdaDataType

    END -- WHILE @@FETCH_STATUS = 0 (all mappings)

    CLOSE MappingCursor

    DEALLOCATE MappingCursor

    PRINT ''

    PRINT '- Gathering field values took ' + CAST(DATEDIFF(s, @StartTime1, GETDATE()) AS VARCHAR(20)) + ' seconds:'

    --PRINT ' - Fetching HL7 values took ' + CAST(CAST(@TotalTime_HL7 / 1000 AS NUMERIC(18,0)) AS VARCHAR(20)) + ' seconds.'

    PRINT ' - Suppression UDFs took ' + CAST(CAST(@TotalTime_SuppressUdfs / 1000 AS NUMERIC(18,0)) AS VARCHAR(20)) + ' seconds.'

    PRINT ' - Formatting UDFs took ' + CAST(CAST(@TotalTime_FormatUdfs / 1000 AS NUMERIC(18,0)) AS VARCHAR(20)) + ' seconds.'

    -- Return the accumulated list of tag file properties.

    IF @FillTempTable = 1 BEGIN

    -- Dump the values into a caller-provided temporary table.

    INSERT INTO #TEMP_TagOutput

    SELECT * FROM @TagValues

    ORDER BY FieldName

    END ELSE BEGIN

    -- Dump the values out to the caller as a result set.

    SELECT * FROM @TagValues

    ORDER BY FieldName

    END

    PRINT '- Whole SP took ' + CAST(DATEDIFF(s, @StartTime0, GETDATE()) AS VARCHAR(20)) + ' seconds.'

    END

  • you really need to look at writing the procedure in a set based manner , you seem to have a very procedural approach to the code which is fine in some languages, but by using cursors etc.. you are limiting the ability of SQL Server to run the procedure correctly.

  • In the glance through, nothing leaped to my eye except the cursors as well. But I'm in agreement with Steve.

    "The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
    - Theodore Roosevelt

    Author of:
    SQL Server Execution Plans
    SQL Server Query Performance Tuning

  • You might want to look at the two articles by R Barry Young[/url] on "15 Ways to Lose Your Cursor".

    Wayne
    Microsoft Certified Master: SQL Server 2008
    Author - SQL Server T-SQL Recipes


    If you can't explain to another person how the code that you're copying from the internet works, then DON'T USE IT on a production system! After all, you will be the one supporting it!
    Links:
    For better assistance in answering your questions
    Performance Problems
    Common date/time routines
    Understanding and Using APPLY Part 1 & Part 2

  • From an IO view, limit LOBs whenever possible, the number of varchar(max) declarations are going to kill your IO. Another item is to consider using Temp tables vs table variables

  • Hmmm

    There is a LOT going on in there, there seems to be a fundemental flaw.

    The calling of any sql in a dynamic fashion , as you have done, would point to an 'incorrect' database design IMO.

    The reason for the cursor is for the dynamic inserts to the @UdfResults table and they will have to be worked around.

    Start at the inside and work outwards merging multiple statements into inline table valued functions.

    It's not going to be a simple or quick process, if this is mission critical then i would advise you to get in a consultant for a few days.



    Clear Sky SQL
    My Blog[/url]

  • Forums are for SIMPLE, straight-forward problems. This is NOT THAT!! People who answer questions here do so as unpaid volunteers and it isn't fair to ask them to give you what will likely be MANY HOURS of effort to fix this very complicated process.

    I'm with Dave - hire a consultant to work with you on this one.

    Best,
    Kevin G. Boles
    SQL Server Consultant
    SQL MVP 2007-2012
    TheSQLGuru on googles mail service

Viewing 7 posts - 1 through 6 (of 6 total)

You must be logged in to reply to this topic. Login to reply