October 7, 2019 at 8:23 pm
There is a long running stored procedure. The 2nd (visit) and 3rd (billing) tables are used just for the results to be used in the where clause while retrieving records form the first table (approvals).
Below is the DDL of the tables involved, and the script of the stored procedure. Apparently, as it is just a sample, it doesn't display no records; thus, there are not bottlenecks observed with this sample data. But imagine, having many userr_ids in the cursor.
There is also additional cost as it is calling another stored procedure. Is there any way I can optimize the query?
DDL of the tables
CREATE TABLE [dbo].[Approvals](
userr_id [int] NOT NULL,
appro_id [int] NOT NULL,
app_units [decimal](9, 2) NOT NULL,
c_units [tinyint] NOT NULL,
usedunits [decimal](9, 2) NOT NULL,
deleted [bit] NOT NULL
)
INSERT INTO approvals
VALUES
(4262, 29, 36.00, 1, 0.00, 0),
(1717, 30, 24.00, 1, 0.00, 0),
(4743, 31, 76.00, 1, 0.00, 0),
(4460, 33, 36.00, 1, 0.00, 0),
(4488, 35, 36.00, 1, 0.00, 0),
(3871, 36, 24.00, 1, 0.00, 0),
(3561, 37, 12.00, 1, 3.00, 0),
(4828, 38, 36.00, 1, 0.00, 0),
(3828, 39, 24.00, 1, 0.00, 0),
(4101, 40, 24.00, 1, 0.00, 0)
CREATE TABLE [dbo].[Visit](
userr_id [int] NULL,
visit_id int null,
appro_id [int] NULL,
c_secondary [bit] NULL,
cascaded_id int NULL,
comb_units int null,
auth_exceeded [bit] NOT NULL,
tperiod [int] NOT NULL,
contratrate [decimal](9, 2) NOT NULL
)
INSERT INTO visit
VALUES
(5329, 85, NULL, 0, NULL, NULL, 0, 419, 0.00),
(4262, 389, NULL, 0, NULL, NULL, 0, 419, 0.00),
(5244, 412, NULL, 0, NULL, NULL, 0, 419, 0.00),
(4205, 501, NULL, 0, NULL, NULL, 0, 419, 0.00),
(4828, 509, NULL, 0, NULL, NULL, 0, 419, 0.00),
(5531, 560, NULL, 0, NULL, NULL, 0, 419, 0.00),
(5558, 593, NULL, 0, NULL, NULL, 0, 419, 0.00),
(4460, 655, NULL, 0, NULL, NULL, 0, 419, 0.00),
(5547, 718, NULL, 0, NULL, NULL, 0, 419, 0.00),
(5219, 836, NULL, 0, NULL, NULL, 0, 419, 0.00)
CREATE TABLE Billing (visit_id int, cur_id int)
INSERT INTO Billing
VALUES
(73, NULL),
(159, NULL),
(173, NULL),
(177, NULL),
(186, NULL),
(190, NULL),
(197, NULL),
(204, NULL),
(211, NULL),
(351, NULL)
The stored procedure:
CREATE PROCEDURE [dbo].[stored_procedure]
AS
DECLARE @userr_id Int, @cnt Int
SET @cnt = 0
DECLARE c CURSOR FOR
SELECT DISTINCT userr_id
FROM (SELECT userr_id, app_units, c_units, usedunits,
(SELECT count(*) FROM Visit WHERE c_secondary = 0 and appro_id = Approvals.appro_id and auth_exceeded = 0) AS count_notexceeded,
(SELECT count(*) FROM Visit WHERE c_secondary = 0 and appro_id = Approvals.appro_id ) AS count_all,
(SELECT SUM(CASE WHEN b.cur_id = v.cascaded_id THEN v.comb_units ELSE v.comb_units END) FROM Visit v JOIN Billing b ON b.visit_id = v.visit_id WHERE c_secondary = 0 and appro_id = Approvals.appro_id and auth_exceeded = 0) AS cntu_not_exceeded,
(SELECT SUM(CASE WHEN b.cur_id = v.cascaded_id THEN v.comb_units ELSE v.comb_units END) FROM Visit v JOIN Billing b ON b.visit_id = v.visit_id WHERE c_secondary = 0 and appro_id = Approvals.appro_id ) AS cntu_all,
(SELECT SUM(tperiod) / 60.00 FROM Visit WHERE c_secondary = 0 and appro_id = Approvals.appro_id ) AS th_all,
(SELECT SUM(tperiod) / 60.00 FROM Visit WHERE c_secondary = 0 and appro_id = Approvals.appro_id and auth_exceeded = 0) AS th_notexceeded,
(SELECT SUM(contratrate) FROM Visit WHERE c_secondary = 0 and appro_id = Approvals.appro_id and auth_exceeded = 0) AS tr_notexceeded,
(SELECT SUM(contratrate) FROM Visit WHERE c_secondary = 0 and appro_id = Approvals.appro_id ) AS tr_all
FROM Approvals where deleted = 0) t
WHERE ((c_units = 0 and (count_all <> usedunits or count_notexceeded > app_units))
OR (c_units = 2 and (th_all <> usedunits or th_notexceeded > app_units))
OR (c_units = 3 and (tr_all <> usedunits or tr_notexceeded > app_units)))
OPEN c
FETCH NEXT FROM c INTO @userr_id
WHILE @@fetch_status = 0
BEGIN
SET @cnt = @cnt + 1
EXEC [_name_of_another_stored_procedure] @userr_id
FETCH NEXT FROM c INTO @userr_id
END
CLOSE c
DEALLOCATE c
Thanks. Any guide is much appreicated.
October 7, 2019 at 9:32 pm
For starters, you can optimize your query by reducing the number of calls to your Visits table. It could be done like this:
SELECT DISTINCT userr_id
FROM Approvals
CROSS APPLY (SELECT COUNT(*) AS count_all,
COUNT(CASE WHEN V.auth_exceeded = 0 THEN 1 END) AS count_notexceeded,
SUM(V.tperiod) / 60.00 AS th_all,
SUM(CASE WHEN V.auth_exceeded = 0 THEN V.tperiod ELSE 0 END) / 60.00 AS th_notexceeded,
SUM(V.contratrate) AS tr_all,
SUM(CASE WHEN V.auth_exceeded = 0 THEN V.contratrate ELSE 0 END) AS tr_notexceeded
FROM Visit V
WHERE c_secondary = 0
and appro_id = Approvals.appro_id) VC
where deleted = 0
AND ((c_units = 0 and (count_all <> usedunits or count_notexceeded > app_units))
OR (c_units = 2 and (th_all <> usedunits or th_notexceeded > app_units))
OR (c_units = 3 and (tr_all <> usedunits or tr_notexceeded > app_units)));
If Approvals can only belong to one visit, you can use something like this:
SELECT DISTINCT A.userr_id
FROM Approvals A
JOIN Visit V ON A.appro_id = V.appro_id
where A.deleted = 0
AND V.c_secondary = 0
GROUP BY A.appro_id,
A.userr_id,
A.c_units,
A.usedunits,
A.app_units
HAVING ((c_units = 0 and (COUNT(*) <> usedunits or COUNT(CASE WHEN V.auth_exceeded = 0 THEN 1 END) > app_units))
OR (c_units = 2 and (SUM(V.tperiod) <> usedunits or SUM(CASE WHEN V.auth_exceeded = 0 THEN V.tperiod ELSE 0 END) / 60.00 > app_units))
OR (c_units = 3 and (SUM(V.contratrate) <> usedunits or SUM(CASE WHEN V.auth_exceeded = 0 THEN V.contratrate ELSE 0 END) > app_units)));
But the greater improvement would be to remove the cursor completely. But there's no way we can help you without the code for the procedure being called there.
October 7, 2019 at 9:49 pm
Hello keneangbu,
My first piece of advice would be remove the cursor. Cursors are inherently slow. In your sample piece of code, I do not see a nice way to kill off that cursor UNLESS you can extract that second stored procedure and have it inside this stored procedure.
IF rewriting everything to get a query that can run without the cursor is not an option, my next thought would be to re-investigate how you are getting the userr_id. You may get a performance boost by inserting those into a temporary table and have your cursor pull from that temp table. What I mean is to take your large cursor statement and change it into something like:
DECLARE @tmpData TABLE (userr_id INT)
INSERT INTO @tmpData
SELECT DISTINCT
[t].[userr_id]
FROM
(
SELECT
[userr_id]
, [app_units]
, [c_units]
, [usedunits]
, (
SELECT
COUNT(*)
FROM [Visit]WHERE [c_secondary] = 0
AND [appro_id] = [Approvals].[appro_id]
AND [auth_exceeded] = 0
) AS [count_notexceeded]
, (
SELECT
COUNT(*)
FROM [Visit]
WHERE [c_secondary] = 0
AND [appro_id] = [Approvals].[appro_id]
) AS [count_all]
, (
SELECT
SUM( CASE
WHEN .[cur_id] = [v].[cascaded_id]
THEN [v].[comb_units]
ELSE [v].[comb_units]
END
)
FROM [Visit] AS [v]
JOIN [Billing] AS
ON .[visit_id] = [v].[visit_id]
WHERE [v].[c_secondary] = 0
AND [v].[appro_id] = [Approvals].[appro_id]
AND [v].[auth_exceeded] = 0
) AS [cntu_not_exceeded]
, (
SELECT
SUM( CASE
WHEN .[cur_id] = [v].[cascaded_id]
THEN [v].[comb_units]
ELSE [v].[comb_units]
END
)
FROM [Visit] AS [v]
JOIN [Billing] AS
ON .[visit_id] = [v].[visit_id]
WHERE [v].[c_secondary] = 0
AND [v].[appro_id] = [Approvals].[appro_id]
) AS [cntu_all]
, (
SELECT
SUM([tperiod]) / 60.00
FROM [Visit]
WHERE [c_secondary] = 0
AND [appro_id] = [Approvals].[appro_id]
) AS [th_all]
, (
SELECT
SUM([tperiod]) / 60.00
FROM [Visit]
WHERE [c_secondary] = 0
AND [appro_id] = [Approvals].[appro_id]
AND [auth_exceeded] = 0
) AS [th_notexceeded]
, (
SELECT
SUM([contratrate])
FROM [Visit]
WHERE [c_secondary] = 0
AND [appro_id] = [Approvals].[appro_id]
AND [auth_exceeded] = 0
) AS [tr_notexceeded]
, (
SELECT
SUM([contratrate])
FROM [Visit]
WHERE [c_secondary] = 0
AND [appro_id] = [Approvals].[appro_id]
) AS [tr_all]
FROM [Approvals]
WHERE [deleted] = 0
) AS [t]
WHERE (
(
[t].[c_units] = 0
AND
(
[t].[count_all] <> [t].[usedunits]
OR [t].[count_notexceeded] > [t].[app_units]
)
)
OR
(
[t].[c_units] = 2
AND
(
[t].[th_all] <> [t].[usedunits]
OR [t].[th_notexceeded] > [t].[app_units]
)
)
OR
(
[t].[c_units] = 3
AND
(
[t].[tr_all] <> [t].[usedunits]
OR [t].[tr_notexceeded] > [t].[app_units]
)
)
);
DECLARE c CURSOR LOCAL FAST_FORWARD FOR
SELECT [userr_id] FROM @tmpData
And use the rest of your code as is. Might not get much of a performance boost mind you. If you could attach your execution plan, it would be easier to help performance tune. Be sure to strip out any confidential information in the execution plan mind you.
Plus, are you certain this is where the "slow code" is? All of the code you provided COULD be running incredibly fast and that nested stored procedure is causing performance hits.
And, adding "LOCAL FAST_FORWARD" onto your cursor, as I did in my bit of code, can offer performance benefits, presuming a local fast_forward cursor makes sense with your setup...
The above is all just my opinion on what you should do.
As with all advice you find on a random internet forum - you shouldn't blindly follow it. Always test on a test server to see if there is negative side effects before making changes to live!
I recommend you NEVER run "random code" you found online on any system you care about UNLESS you understand and can verify the code OR you don't care if the code trashes your system.
October 8, 2019 at 12:29 pm
Hello keneangbu,
My first piece of advice would be remove the cursor. Cursors are inherently slow. In your sample piece of code, I do not see a nice way to kill off that cursor UNLESS you can extract that second stored procedure and have it inside this stored procedure.
IF rewriting everything to get a query that can run without the cursor is not an option, my next thought would be to re-investigate how you are getting the userr_id. You may get a performance boost by inserting those into a temporary table and have your cursor pull from that temp table. What I mean is to take your large cursor statement and change it into something like:
[...stripped code out]
And use the rest of your code as is. Might not get much of a performance boost mind you. If you could attach your execution plan, it would be easier to help performance tune. Be sure to strip out any confidential information in the execution plan mind you.
Plus, are you certain this is where the "slow code" is? All of the code you provided COULD be running incredibly fast and that nested stored procedure is causing performance hits.
And, adding "LOCAL FAST_FORWARD" onto your cursor, as I did in my bit of code, can offer performance benefits, presuming a local fast_forward cursor makes sense with your setup...
I don't see how using a temp table would improve performance if the cursor is made LOCAL and STATIC, which basically creates a copy of the data to prevent any additional reads.
October 8, 2019 at 3:16 pm
While I do agree with you, I've seen strange behavior like that before. I cannot think of or provide a case where it happened, but I've seen where performance improves by dumping data into a temp table or table variable prior to putting it in a cursor.
Thinking about it now though, I am wondering if in the cases I saw the performance improvement, if it was that I didn't use LOCAL and FAST_FORWARD... It was quite a while ago that I noticed this (several years back) and since then I have cut back a lot on using cursors to ALMOST never.
The above is all just my opinion on what you should do.
As with all advice you find on a random internet forum - you shouldn't blindly follow it. Always test on a test server to see if there is negative side effects before making changes to live!
I recommend you NEVER run "random code" you found online on any system you care about UNLESS you understand and can verify the code OR you don't care if the code trashes your system.
October 8, 2019 at 6:09 pm
@luis Cazares thanks! There was a third table 'Billing' though. I have tried to follow the same logic to include that in your code, but it is displaying more user_idd than expected. That makes the process even slower. I wonder why it is giving me many ids.
October 8, 2019 at 7:24 pm
@Luis Cazares thanks! There was a third table 'Billing' though. I have tried to follow the same logic to include that in your code, but it is displaying more user_idd than expected. That makes the process even slower. I wonder why it is giving me many ids.
Since you weren't using the cntu columns which were the ones using Billing, I didn't include them in the query I posted. To do that, you can add a second APPLY to prevent changing the row counts when aggregating after joining the Billing table.
Viewing 8 posts - 1 through 7 (of 7 total)
You must be logged in to reply to this topic. Login to reply