SQL Query Optimization

Question

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;

What version of SQL Server are you using? – Lamak Feb 07 '11 at 17:32 — Lamak, Feb 07 '11 at 17:32
PLEASE DO NOT USE NOLOCK - It leads to incorrect results! – Milney Mar 16 '17 at 07:53 — Milney, Mar 16 '17 at 07:53

score 4 · Answer 1 · edited May 23 '17 at 12:00

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'

I'll bet you don't have an index on the AccountStatus.StatusType field. You might try using the PK of that table instead.

HTH.

score 3 · Accepted Answer · answered Feb 07 '11 at 19:31

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.

Amy B · Answer 3 · 2011-02-07T17:51:09.520

2

SELECT @CollectionKey = [AccountNo] FROM @Accounts WHERE [AccountKey] = @index;

This query would benefit from a PRIMARY KEY declaration on your table variable.

When you say IDENTITY, you are asking the database to auto-populate the column.
When you say PRIMARY KEY, you are asking the database to organize the data into a clustered index.

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

edited Feb 07 '11 at 17:51

answered Feb 07 '11 at 17:42

Amy B

108,202
21
135
185

This report is run 4-5 times daily by company executives, and must point to the production database. It's not a one-time report, so copying the data to another DB doesn't seem like an option. – Blackcoil Feb 07 '11 at 18:14
1

If you use a temp table you can index. – HLGEM Feb 07 '11 at 19:26

SQL Query Optimization

3 Answers3