Reputation: 173
This report used to take about 16 seconds when there were 8000 rows to process. Now there are 50000 rows and the report takes 2:30 minutes.
This was my first pass at this and the client needed it yesterday, so I wrote this code in the logical order of what needed to be done, but without optimization in mind.
Now with the report taking longer as the data increases, I need to take a second look at this and optimize it. I'm thinking indexed views, table functions, etc.
I think the biggest bottleneck is looping through the temp table, making 4 select statements, and updating the temp table...50,000 times.
I think I can condense ALL of this into one large SELECT with either (a) 4 joins to the same table to get the 4 statuses, but then I am not sure how to get the TOP 1 in there, or I can try (b) using nested subqueries, but both seem really messy compared to the current code.
I'm not expecting anyone to write code for me, but if some SQL experts can peruse this code and tell me about any obvious inefficiencies and alternate methods, or ways to speed this up, or techniques I should be using instead, it would be appreciated.
PS: Assume that this DB is for the most part normalized, but poorly designed, and that I am not able to add indexes. I basically have to work with it, as is.
Where the code says (less than) I had to replace a "less than" symbol because it was cropping some of my code.
Thanks!
CREATE PROCEDURE RptCollectionAccountStatusReport AS SET NOCOUNT ON; DECLARE @Accounts TABLE ( [AccountKey] INT IDENTITY(1,1) NOT NULL, [ManagementCompany] NVARCHAR(50), [Association] NVARCHAR(100), [AccountNo] INT UNIQUE, [StreetAddress] NVARCHAR(65), [State] NVARCHAR(50), [PrimaryStatus] NVARCHAR(100), [PrimaryStatusDate] SMALLDATETIME, [PrimaryDaysRemaining] INT, [SecondaryStatus] NVARCHAR(100), [SecondaryStatusDate] SMALLDATETIME, [SecondaryDaysRemaining] INT, [TertiaryStatus] NVARCHAR(100), [TertiaryStatusDate] SMALLDATETIME, [TertiaryDaysRemaining] INT, [ExternalStatus] NVARCHAR(100), [ExternalStatusDate] SMALLDATETIME, [ExternalDaysRemaining] INT ); INSERT INTO @Accounts ( [ManagementCompany], [Association], [AccountNo], [StreetAddress], [State]) SELECT mc.Name AS [ManagementCompany], a.LegalName AS [Association], c.CollectionKey AS [AccountNo], u.StreetNumber + ' ' + u.StreetName AS [StreetAddress], CASE WHEN c.InheritedAccount = 1 THEN 'ZZ' ELSE u.State END AS [State] FROM ManagementCompany mc WITH (NOLOCK) JOIN Association a WITH (NOLOCK) ON a.ManagementCompanyKey = mc.ManagementCompanyKey JOIN Unit u WITH (NOLOCK) ON u.AssociationKey = a.AssociationKey JOIN Collection c WITH (NOLOCK) ON c.UnitKey = u.UnitKey WHERE c.Closed IS NULL; DECLARE @MaxAccountKey INT; SELECT @MaxAccountKey = MAX([AccountKey]) FROM @Accounts; DECLARE @index INT; SET @index = 1; WHILE @index (less than) @MaxAccountKey BEGIN DECLARE @CollectionKey INT; SELECT @CollectionKey = [AccountNo] FROM @Accounts WHERE [AccountKey] = @index; DECLARE @PrimaryStatus NVARCHAR(100) = NULL; DECLARE @PrimaryStatusDate SMALLDATETIME = NULL; DECLARE @PrimaryDaysRemaining INT = NULL; DECLARE @SecondaryStatus NVARCHAR(100) = NULL; DECLARE @SecondaryStatusDate SMALLDATETIME = NULL; DECLARE @SecondaryDaysRemaining INT = NULL; DECLARE @TertiaryStatus NVARCHAR(100) = NULL; DECLARE @TertiaryStatusDate SMALLDATETIME = NULL; DECLARE @TertiaryDaysRemaining INT = NULL; DECLARE @ExternalStatus NVARCHAR(100) = NULL; DECLARE @ExternalStatusDate SMALLDATETIME = NULL; DECLARE @ExternalDaysRemaining INT = NULL; SELECT TOP 1 @PrimaryStatus = a.StatusName, @PrimaryStatusDate = c.StatusDate, @PrimaryDaysRemaining = c.DaysRemaining FROM CollectionAccountStatus c WITH (NOLOCK) JOIN AccountStatus a WITH (NOLOCK) ON c.AccountStatusKey = a.AccountStatusKey WHERE c.CollectionKey = @CollectionKey AND a.StatusType = 'Primary Status' AND a.StatusName 'Cleared' ORDER BY c.sysCreated DESC; SELECT TOP 1 @SecondaryStatus = a.StatusName, @SecondaryStatusDate = c.StatusDate, @SecondaryDaysRemaining = c.DaysRemaining FROM CollectionAccountStatus c WITH (NOLOCK) JOIN AccountStatus a WITH (NOLOCK) ON c.AccountStatusKey = a.AccountStatusKey WHERE c.CollectionKey = @CollectionKey AND a.StatusType = 'Secondary Status' AND a.StatusName 'Cleared' ORDER BY c.sysCreated DESC; SELECT TOP 1 @TertiaryStatus = a.StatusName, @TertiaryStatusDate = c.StatusDate, @TertiaryDaysRemaining = c.DaysRemaining FROM CollectionAccountStatus c WITH (NOLOCK) JOIN AccountStatus a WITH (NOLOCK) ON c.AccountStatusKey = a.AccountStatusKey WHERE c.CollectionKey = @CollectionKey AND a.StatusType = 'Tertiary Status' AND a.StatusName 'Cleared' ORDER BY c.sysCreated DESC; SELECT TOP 1 @ExternalStatus = a.StatusName, @ExternalStatusDate = c.StatusDate, @ExternalDaysRemaining = c.DaysRemaining FROM CollectionAccountStatus c WITH (NOLOCK) JOIN AccountStatus a WITH (NOLOCK) ON c.AccountStatusKey = a.AccountStatusKey WHERE c.CollectionKey = @CollectionKey AND a.StatusType = 'External Status' AND a.StatusName 'Cleared' ORDER BY c.sysCreated DESC; UPDATE @Accounts SET [PrimaryStatus] = @PrimaryStatus, [PrimaryStatusDate] = @PrimaryStatusDate, [PrimaryDaysRemaining] = @PrimaryDaysRemaining, [SecondaryStatus] = @SecondaryStatus, [SecondaryStatusDate] = @SecondaryStatusDate, [SecondaryDaysRemaining] = @SecondaryDaysRemaining, [TertiaryStatus] = @TertiaryStatus, [TertiaryStatusDate] = @TertiaryStatusDate, [TertiaryDaysRemaining] = @TertiaryDaysRemaining, [ExternalStatus] = @ExternalStatus, [ExternalStatusDate] = @ExternalStatusDate, [ExternalDaysRemaining] = @ExternalDaysRemaining WHERE [AccountNo] = @CollectionKey; SET @index = @index + 1; END; SELECT [ManagementCompany], [Association], [AccountNo], [StreetAddress], [State], [PrimaryStatus], CONVERT(VARCHAR, [PrimaryStatusDate], 101) AS [PrimaryStatusDate], [PrimaryDaysRemaining], [SecondaryStatus], CONVERT(VARCHAR, [SecondaryStatusDate], 101) AS [SecondaryStatusDate], [SecondaryDaysRemaining], [TertiaryStatus], CONVERT(VARCHAR, [TertiaryStatusDate], 101) AS [TertiaryStatusDate], [TertiaryDaysRemaining], [ExternalStatus], CONVERT(VARCHAR, [ExternalStatusDate], 101) AS [ExternalStatusDate], [ExternalDaysRemaining] FROM @Accounts ORDER BY [ManagementCompany], [Association], [StreetAddress] ASC;
Upvotes: 0
Views: 759
Reputation: 96640
First use a temp table instead of a table varaiable. These can be indexed.
Next, do not loop! Looping is bad for performance in virtually every case. This loop ran 50000 times rather than once for 50000 records, it will be horrible when you havea million records! Here is a link that will help you understand how to do set-based processing instead. It is written to avoid cursos but loops are similar to cursors, so it should help. http://wiki.lessthandot.com/index.php/Cursors_and_How_to_Avoid_Them
And (nolock) will give dirty data reads which can be very bad for reporting. If you are in a version of SQl Server higher than 2000, there are better choices.
Upvotes: 3
Reputation: 110221
SELECT @CollectionKey = [AccountNo] FROM @Accounts WHERE [AccountKey] = @index;
This query would benefit from a PRIMARY KEY declaration on your table variable.
These two concepts are very different. Typically, you should use both of them.
DECLARE @Accounts TABLE
(
[AccountKey] INT IDENTITY(1,1) PRIMARY KEY,
I am not able to add indexes.
In that case, copy the data to a database where you may add indexes. And use: SET STATISTICS IO ON
Upvotes: 2
Reputation: 6295
Don't try to guess where the query is going wrong - look at the execution plan. It will tell you what's chewing up your resources.
You can update directly from another table, even from a table variable: SQL update from one Table to another based on a ID match
That would allow you to combine everything in your loop into a single (massive) statement. You can join to the same tables for the secondary and tertiary statuses using different aliases, e.g.,
JOIN AccountStatus As TertiaryAccountStatus...AND a.StatusType = 'Tertiary Status'
JOIN AccountStatus AS SecondaryAccountStatus...AND a.StatusType = 'Secondary Status'
HTH.
Upvotes: 4