March 15, 2011 at 8:26 am
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
March 15, 2011 at 8:33 am
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.
March 15, 2011 at 9:54 am
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
March 15, 2011 at 10:34 am
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
March 16, 2011 at 8:05 am
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
March 17, 2011 at 1:06 am
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.
March 17, 2011 at 7:38 am
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