HiveBrain v1.2.0
Get Started
← Back to all entries
patternsqlMinor

Return the top 5 of each kind of record

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thetopeachreturnkindrecord

Problem

I have this currently working code, but it's extremely slow. I'm talking almost an hour to execute, if it executes at all (our server isn't all that great to start with). Is there a better way to write this?

Basically, I am wanting to return the Top 5 parts, that match a distinct pattern code within the same table. So, if there were 100 distinct pattern codes in my [PartNumber] table, I would end up with 500 records.

ORIGINAL at time of question posting (Executes in 32 minutes):

DECLARE @PartInfo TABLE(
    [PartNumber] [varchar](64) NOT NULL,
    [PatternCode] [varchar](64) NOT NULL,
    [LanguageCode] [varchar](24) NOT NULL);

DECLARE @PatternCode nvarchar(64);
DECLARE pattern_Cursor CURSOR LOCAL SCROLL STATIC
    FOR
    SELECT TOP 1000 [PartNumber].[PatternCode]
    FROM [Web_Service].[dbo].[PartNumber] [PartNumber]
    WHERE PatternCode NOT LIKE 'NOT FOUND'
    GROUP BY [PartNumber].[PatternCode]

    OPEN pattern_Cursor;

    FETCH NEXT FROM pattern_Cursor
    INTO @PatternCode;

    WHILE @@FETCH_STATUS = 0
        BEGIN
            INSERT INTO @PartInfo
            SELECT TOP 5 [PartNumberInfo].*
            FROM [Web_Service].[dbo].[PartNumber] [PartNumber]
            WHERE [PartNumber].[PatternCode] = @PatternCode;

            FETCH NEXT FROM pattern_Cursor
            INTO @PatternCode;
        END
CLOSE pattern_Cursor;
DEALLOCATE pattern_Cursor;

SELECT *
FROM @PartInfo
ORDER BY [@PartInfo].[PatternCode]


First revision (Executes in 24 minutes):

```
CREATE TABLE #PartInfo ([PartNumber] varchar NOT NULL,
[PatternCode] varchar NOT NULL,
[LanguageCode] varchar);

DECLARE @PatternCode nvarchar(64);

DECLARE pattern_Cursor CURSOR FAST_FORWARD
FOR
SELECT [PartNumber].[PatternCode]
FROM [Web_Service].[dbo].[PartNumber] [PartNumber]
WHERE PatternCode <> 'NOT FOUND'
GROUP BY [PartNumber].[PatternCode]

OPEN pattern_Cursor;

FETCH NEXT FROM pattern_Cursor

Solution

You have three problems that I can see. Two performance, and one logic. In least-to-most significant order:

  • Performance: You should no use the NOT LIKE 'NOT FOUND' statement, it should be <> 'NOT FOUND'



  • Potential logic problem: You should have an order-by clause on the inner select for the TOP 5 ... otherwise, what TOP 5 are you getting?



  • Performance: Using declare @table... syntax has different performacne to create table #table ... For example, this blog entry shows some real differences. I recommend trying the same query with a #temp table instead.



Also, read up on this SO Answer: SQL Server tables: what is the difference between @, # and ##?

Edit:

It was almost too obvious to ask... but, you have done the two most basic items... right?

  • an index on the [PartNumber].[PatternCode]



  • updated all table statistics



... right?

Context

StackExchange Code Review Q#44256, answer score: 6

Revisions (0)

No revisions yet.