A painfully slow stored procedure.

Please start any new threads on our new site at https://forums.sqlteam.com. We've got lots of great SQL Server experts to answer whatever question you can come up with.

All Forums

SQL Server 2000 Forums

Transact-SQL (2000)

A painfully slow stored procedure.

Author

Topic

hartmanm
Starting Member

7 Posts

Posted - 2002-07-05 : 11:28:09

Hey all. I have a stored procedure that's ran fairly often during an application. It's used to populate a treeview for a patient list in a hospital. There's a few speed issues that I simply can't get past (my link to the Pharmacy server is the big one), but I'm certain that there's big room for improvement on my code. Here's a paste. I'll bold some of my comments to explain pieces.

---- PATIENTLIST TABLE ----

DECLARE @PATIENTLIST TABLE (
EVENTID INT NOT NULL,
CRNUM NVARCHAR(7) NOT NULL,
LOCATION NVARCHAR(15) NOT NULL,
PATIENTNAME NVARCHAR(50) NOT NULL,
NEW BIT NOT NULL
)
INSERT INTO @PATIENTLIST (EVENTID, CRNUM, LOCATION, PATIENTNAME, NEW)
SELECT H.EVENTID, H.CRNUM, CASE(RTRIM(P.CROOM)) WHEN '' THEN '(BLANK)' ELSE RTRIM(P.CROOM) END, RTRIM(P.LNAME) + ', ' + RTRIM(P.FNAME), H.NEW
FROM (HOSPITALEVENT AS H INNER JOIN PCSRECORDS AS P ON P.CRNUM = H.CRNUM AND H.DISCANESID = -1)
WHERE H.DISCANESID = -1

---- PHARMACY TABLE ----
This calls another procedure which creates a dynamic view for certain patient identifier numbers in Pharmacy. It creates the view Pharmacy_Alerts_1 that is referenced later.
exec sp_refresh_pharmacy_view

---- MODALITY TABLE ----
This section finds all the patient modalities and appends them into one row like "Epidural, Other".
DECLARE @MYCOUNTER INT, @MODALITY NVARCHAR(20), @ROWID INT, @EVENTID INT
DECLARE @TEMPMODALITY TABLE (
ROWID INT IDENTITY(1, 1) NOT NULL,
EVENTID INT NOT NULL,
MODALITY NVARCHAR(100) DEFAULT ('') NOT NULL)

DECLARE @MODALITYLIST TABLE(
ROWID INT IDENTITY(1,1) NOT NULL,
EVENTID INT NOT NULL,
MODALITY NVARCHAR(100) DEFAULT('') NOT NULL
)
INSERT INTO @MODALITYLIST (EVENTID) SELECT EVENTID FROM @PATIENTLIST
INSERT INTO @TEMPMODALITY (EVENTID, MODALITY) SELECT PL.EVENTID, LMT.MODALITY FROM ((@PATIENTLIST AS PL INNER JOIN MODALITY AS M ON M.EVENTID = PL.EVENTID) INNER JOIN LUMODALITYTYPE AS LMT ON LMT.MODALITYTYPEID = M.MODALITYTYPEID) WHERE M.STATE='C'

SELECT @MYCOUNTER = COUNT(ROWID) FROM @TEMPMODALITY
WHILE (@MYCOUNTER > 0)
BEGIN
SELECT TOP 1 @ROWID = ROWID, @EVENTID = EVENTID, @MODALITY = MODALITY FROM @TEMPMODALITY
UPDATE @MODALITYLIST SET MODALITY = MODALITY + (SELECT MODALITY FROM @TEMPMODALITY WHERE ROWID = @ROWID) + ', ' WHERE EVENTID = @EVENTID
DELETE FROM @TEMPMODALITY WHERE ROWID = @ROWID
SET @MYCOUNTER = @MYCOUNTER - 1
END
UPDATE @MODALITYLIST SET MODALITY = LEFT(MODALITY, case(LEN(MODALITY)) when 0 then 0 else len(modality) - 1 end)
UPDATE @MODALITYLIST SET MODALITY = 'None' WHERE MODALITY = ''

---- LABRESULTS TABLE ----
Lab information is stored in two tables that use a one to many relationship, LabRecords and LabOBXRecords. LabOBX is around 600k records and grows daily.
DECLARE @LABRESULTS TABLE (
EVENTID INT NOT NULL,
OBXID INT NOT NULL,
IDENTIFIER NVARCHAR(10) NOT NULL,
LABVALUE NVARCHAR(10) NOT NULL,
UNITS NVARCHAR(10) NOT NULL,
LESSTHAN REAL DEFAULT(0) NOT NULL,
GREATERTHAN REAL DEFAULT(0) NOT NULL
)

INSERT INTO @LABRESULTS (EVENTID, OBXID, IDENTIFIER, LABVALUE, UNITS, LESSTHAN, GREATERTHAN)
SELECT P.EVENTID, LO.OBXID, RTRIM(LO.IDENTIFIER), RTRIM(LO.OBSERVATIONVALUE), RTRIM(LO.UNITS), LFI.LESSTHAN, LFI.GREATERTHAN
FROM (@PatientList as P
INNER JOIN LABRECORDS AS L ON P.CRNUM = L.CRNUM
INNER JOIN LABOBXRECORDS AS LO ON LO.LABID = L.LABID
INNER JOIN LUFLAGGEDIDENTIFIER AS LFI ON LFI.IDENTIFIER = LO.IDENTIFIER)
DELETE FROM @LABRESULTS WHERE OBXID NOT IN (SELECT MAX(OBXID) FROM @LABRESULTS GROUP BY EVENTID, IDENTIFIER)

---- SLIMPATIENTSBYCRNUM RESULTS ----
Here comes the actual results.
SELECT 'NODE' AS TREE, P.EVENTID, P.CRNUM, P.LOCATION, P.PATIENTNAME, P.NEW, M.MODALITY, ISNULL(VN.VoiceDATE, '0') AS NOTEDATE, VN.VoiceTIME, ISNULL(HV.VISITDATE, '0') AS VISITDATE, HV.VISITTIME AS VISITTIME, '' AS IDENTIFIER, '' AS LABVALUE, '' as UNITS, '' AS POD, '' AS SURGERY
FROM (@PATIENTLIST AS P
INNER JOIN @MODALITYLIST AS M ON M.EVENTID = P.EVENTID
LEFT JOIN VoiceNote AS VN ON VN.EVENTID = P.EVENTID AND VN.STATE='C'
LEFT JOIN HOSPITALEVENTVISITS AS HV ON HV.EVENTID = P.EVENTID AND HV.STATE = 'C')
-- Pharmacy --
UNION ALL
SELECT 'subALERT' AS TREE, P.EVENTID, P.CRNum, '', '', '0', '' AS MODALITY, '0' AS NOTEDATE, '' as NOTETIME, '0' as VisitDate, '' as VisitTime, Phar.Medication, Phar.TheDate, Phar.TheTime, '', ''
FROM (@PATIENTLIST AS P
INNER JOIN @MODALITYLIST AS M ON M.EVENTID = P.EVENTID
INNER JOIN PharmacyAlerts_1 AS Phar on P.CRNum = Phar.CRNum)
WHERE M.MODALITY <> 'PCAIV'
-- Numerics --
UNION ALL
SELECT 'subALERT' AS TREE, P.EVENTID, P.CRNum, '', '', '0', '' AS MODALITY, '0' AS NOTEDATE, '' as NOTETIME, '0' as VisitDate, '' as VisitTime, L.IDENTIFIER, L.LABVALUE, L.UNITS, '', ''
FROM (@PATIENTLIST AS P
INNER JOIN @LABRESULTS AS L on L.EVENTID = P.EVENTID and ISNUMERIC(L.LABVALUE) = 1)
WHERE (L.LessThan <> 0 AND L.LabValue <= L.LessThan) OR (L.GreaterThan <> 0 and L.LabValue >= L.GreaterThan)
-- Non-Numerics --
UNION ALL
SELECT 'subALERT' AS TREE, P.EVENTID, P.CRNum, '', '', '0', '' AS MODALITY, '0' AS NOTEDATE, '' as NOTETIME, '0' as VisitDate, '' as VisitTime, L.IDENTIFIER, L.LABVALUE, '', '', ''
FROM (@PATIENTLIST AS P
INNER JOIN @LABRESULTS AS L on L.EVENTID = P.EVENTID AND ISNUMERIC(L.LABVALUE) <> 1)
-- Ordering --
ORDER BY P.CRNum, P.EventID, TREE, IDENTIFIER

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

Here's two records of a sample output. I use the Tree column in a select case to determine if this is a child node or a parent node. The NEW column is used to determine which icon to use on that treeview node. POD and Surgery are unused at this point.

Some more info.. HospitalEvent stores the active patient list, PCSRecords contains demographics for everyone.

NODE 545 0111111 D404-1 Roach, Buddy 1 PCAIV 0 NULL 0 NULL
subALERT 545 0111111 0 0 0 HB 79 g/L

Thanks for any tips guys. Please reply here, but also send me your replies at hartmanm@kgh.kari.net in case I happen to miss them.

Matt.

Page47
Master Smack Fu Yak Hacker

2878 Posts

Posted - 2002-07-05 : 11:38:02

Why not select directly from your base tables rather than populating all these table variables? Your variables cannot take advantage of the the indexes on the based tables when you finally return the rowset. Also, rather than populating a table variable with a large set and then issueing delete statements to narrow the set, you should just put the required conditions in the WHERE clause so that you only deal with one DML. Granted, you are going to end up with one mighty select statement, but I think in end, it will out perform your current meathod. Give it a try...

<O>

M.E.
Aged Yak Warrior

539 Posts

Posted - 2002-07-05 : 11:44:54

Youve also got a while loop there..

quote:

SELECT @MYCOUNTER = COUNT(ROWID) FROM @TEMPMODALITY
WHILE (@MYCOUNTER > 0)
BEGIN
SELECT TOP 1 @ROWID = ROWID, @EVENTID = EVENTID, @MODALITY = MODALITY FROM @TEMPMODALITY
UPDATE @MODALITYLIST SET MODALITY = MODALITY + (SELECT MODALITY FROM @TEMPMODALITY WHERE ROWID = @ROWID) + ', ' WHERE EVENTID = @EVENTID
DELETE FROM @TEMPMODALITY WHERE ROWID = @ROWID
SET @MYCOUNTER = @MYCOUNTER - 1
END

That should be change to a setbased method. Page47 is right though, I would first look into using the base tables and not so many table variables.

-----------------------
Take my advice, I dare ya

hartmanm
Starting Member

7 Posts

Posted - 2002-07-05 : 11:48:13

Just some stats to throw in..

The INSERT INTO @LABRESULTS query takes 22% of the total batch cost.

The final massive SELECT at the end takes 29% of the total batch cost, but 84% of that 29% (so about 24% overall batch cost) is from the call to the remote Pharmacy table.

Regarding the table variables, I thought this would be faster due to the fact that the PCSRecords table is around 21k records and I have to perform the HospitalEvent/PCSRecords join in many more queries. Isn't it faster to calculate it once, store the results in memory, and just reuse them? Especially considering that the primary key on PCSRecords is a nvarchar(10) (not my choice).

The logic behind the INSERT/DELETE is close to the same. I'm not sure how I'd make the MAX a condition of the LabOBXRecords join, and I don't want to perform the MAX function on the 600k or more records in the original select. That's why I resorted to the secondary DELETE statement. How would I incorporate the MAX into the INSERT?

Thanks again.

Matt.

hartmanm
Starting Member

7 Posts

Posted - 2002-07-05 : 11:51:51

What's a faster method of appending rows like:
ID Data
1 One
1 Two
1 Three
to this:
ID Data
1 One, Two, Three
The while loop was all I could figure out heh.

quote:

Youve also got a while loop there..
quote:

SELECT @MYCOUNTER = COUNT(ROWID) FROM @TEMPMODALITY
WHILE (@MYCOUNTER > 0)
BEGIN
SELECT TOP 1 @ROWID = ROWID, @EVENTID = EVENTID, @MODALITY = MODALITY FROM @TEMPMODALITY
UPDATE @MODALITYLIST SET MODALITY = MODALITY + (SELECT MODALITY FROM @TEMPMODALITY WHERE ROWID = @ROWID) + ', ' WHERE EVENTID = @EVENTID
DELETE FROM @TEMPMODALITY WHERE ROWID = @ROWID
SET @MYCOUNTER = @MYCOUNTER - 1
END

That should be change to a setbased method. Page47 is right though, I would first look into using the base tables and not so many table variables.

-----------------------
Take my advice, I dare ya

hartmanm
Starting Member

7 Posts

Posted - 2002-07-05 : 13:13:33

I've just modified the MODALITY section for a touch of more performance. Thanks to the one article about COALESCE. It's not all that much faster, but the while loop has less iterations and there's fewer work done on the temporary tables.

---- MODALITY TABLE ----

DECLARE @MYCOUNTER INT, @MODALITY NVARCHAR(100), @ROWID INT, @EVENTID INT
DECLARE @TEMPMODALITY TABLE (
ROWID INT IDENTITY(1, 1) NOT NULL,
EVENTID INT NOT NULL,
MODALITY NVARCHAR(100) DEFAULT ('') NOT NULL)

DECLARE @MODALITYLIST TABLE(
ROWID INT IDENTITY(1,1) NOT NULL,
EVENTID INT NOT NULL,
MODALITY NVARCHAR(100) DEFAULT('') NOT NULL
)
INSERT INTO @MODALITYLIST (EVENTID) SELECT EVENTID FROM @PATIENTLIST
INSERT INTO @TEMPMODALITY (EVENTID, MODALITY) SELECT PL.EVENTID, LMT.MODALITY FROM ((@PATIENTLIST AS PL INNER JOIN MODALITY AS M ON M.EVENTID = PL.EVENTID) INNER JOIN LUMODALITYTYPE AS LMT ON LMT.MODALITYTYPEID = M.MODALITYTYPEID) WHERE M.STATE='C'

SELECT @MYCOUNTER = count(eventid) from @MODALITYLIST
while (@MYCOUNTER > 0)
begin
set @MODALITY = null
SELECT @EVENTID = EVENTID FROM @MODALITYLIST WHERE ROWID = @MYCOUNTER
SELECT @MODALITY = COALESCE(@MODALITY + ', ', '') + MODALITY FROM @TEMPMODALITY WHERE EVENTID = @EVENTID
update @MODALITYLIST SET MODALITY = ISNULL(@MODALITY, '') WHERE EVENTID = @EVENTID
SET @Mycounter = @Mycounter - 1
end

Page47
Master Smack Fu Yak Hacker

2878 Posts

Posted - 2002-07-05 : 13:24:26

You can do that without the while loop....

http://www.sqlteam.com/forums/topic.asp?TOPIC_ID=14095

<O>

hartmanm
Starting Member

7 Posts

Posted - 2002-07-05 : 14:41:34

Alright, the while loop is out and that topic's new code is in, thanks :)

Subscribe to SQLTeam.com

SQLTeam.com Articles via RSS

SQLTeam.com Weblog via RSS

- Advertisement -

Resources