Tek-Tips is the largest IT community on the Internet today!

Members share and learn making Tek-Tips Forums the best source of peer-reviewed technical information on the Internet!

  • Congratulations strongm on being selected by the Tek-Tips community for having the most helpful posts in the forums last week. Way to Go!

How can I improve this Stored Procedure ? 2

Status
Not open for further replies.

royc75

Programmer
Jun 1, 2006
127
GB
Hello,

I have a certain Stored Procedure which process large amount of records. The SP is doing what it suppose to do OK but it is taking it long time to do it. Of course, part of the "blame" is the large amount of data but I think my SP implementation can be improved. Can someone here help me to improve this SP yet keep it's original logic of course untouched?

Here it is:

Code:
CREATE PROCEDURE [dbo].[ProcessTankNumber]
@tankNumber varchar(10), @caseYear varchar(10), @office smallint, @method smallint, @type smallint

AS

BEGIN
DECLARE @maxNumber integer
DECLARE  @minNumber integer 
DECLARE  @counter  integer
DECLARE  @missingItems varchar (2000) 
DECLARE  @hall varchar (50) 

SET @missingItems = ''
SET @minNumber = (SELECT MIN(convert(integer,case_number)) FROM ROTOPLAS_ITEMS WHERE tank_number=@tankNumber AND case_year=@caseYear AND office=@office AND method=@method AND type=@type)
SET @maxNumber = (SELECT MAX(convert(integer,case_number)) FROM ROTOPLAS_ITEMS WHERE tank_number=@tankNumber AND case_year=@caseYear AND office=@office AND method=@method AND type=@type)
SET @hall = (SELECT DISTINCT hall FROM ROTOPLAS_ITEMS WHERE tank_number=@tankNumber AND case_year=@caseYear AND office=@office AND method=@method AND type=@type)
SET @counter = @minNumber

WHILE (@counter<=@maxNumber)
BEGIN
	IF NOT EXISTS ( SELECT record_number FROM ROTOPLAS_ITEMS WHERE case_number=@counter AND tank_number=@tankNumber AND case_year=@caseYear AND office=@office AND method=@method AND type=@type)
		SET @missingItems = @missingItems + convert(varchar,@counter) + ', ' 
set @counter = @counter + 1
END

IF @missingItems = ''
BEGIN
	SET @missingItems = 'No missing files'
END

IF RIGHT(@missingItems,2) = ',' 
BEGIN
	SET @missingItems = LEFT(@missingItems,LEN(@missingItems)-1)
END

INSERT INTO ROTOPLAS_TEMP_REPORT
(tankNumber, hall, minNumber, maxNumber, missingNumbers)
VALUES
(@tankNumber, @hall, @minNumber, @maxNumber,@missingItems)
END
GO
 
You could change this part...

SET @minNumber = (SELECT MIN(convert(integer,case_number)) FROM ROTOPLAS_ITEMS WHERE tank_number=@tankNumber AND case_year=@caseYear AND office=@office AND method=@method AND type=@type)
SET @maxNumber = (SELECT MAX(convert(integer,case_number)) FROM ROTOPLAS_ITEMS WHERE tank_number=@tankNumber AND case_year=@caseYear AND office=@office AND method=@method AND type=@type)

To...

Code:
SELECT @minNumber = MIN(convert(integer,case_number)),
       @maxNumber = MAX(convert(integer,case_number))
FROM   ROTOPLAS_ITEMS 
WHERE  tank_number=@tankNumber 
       AND case_year=@caseYear 
       AND office=@office 
       AND method=@method 
       AND type=@type)

It won't help much though. The biggest problem is with the while loop. It looks like you are trying to build a comma delimited list of case numbers that are missing from a list of values.

Ex.

In the table...

ROTOPLAS_ITEMS
4
5
7
12

You want @MissingItems to be: '6, 8, 9, 10, 11'

Is this correct?


-George

Strong and bitter words indicate a weak cause. - Fortune cookie wisdom
 
How to find missing numbers:

First, you'll want to have a numbers table in the database. It won't take much room in the DB, but will allow this query to be MANY times faster. To create the numbers table...

Code:
Create Table Numbers(N Integer Primary Key Clustered)

Declare @i Integer
Set @i = 1

While @i <=100000
  Begin
    Insert Into Numbers Values(@i)
    Set @i = @i + 1
  End

Then, you can use it like this...

Code:
declare @Temp Table(CaseNumber Integer)

Insert Into @Temp Values(4)
Insert Into @Temp Values(5)
Insert Into @Temp Values(7)
Insert Into @Temp Values(12)

Declare @Min Integer
Declare @Max Integer
Declare @Missing VarChar(8000)
Set @Missing = ''

Select @Min = Min(CaseNumber),
       @Max = Max(CaseNumber)
From   @Temp

Select @Missing = @Missing + Convert(VarChar(20), Numbers.N) + ','
From   Numbers	
       Left Join @Temp T
         On Numbers.N = T.CaseNumber
Where  T.CaseNumber Is NULL
       And Numbers.N Between @Min And @Max

If Right(@Missing, 1) = ','
    Set @Missing = left(@Missing, Len(@Missing)-1)

Select @Missing


-George

Strong and bitter words indicate a weak cause. - Fortune cookie wisdom
 
Hello George,

I think I am missing the Numbers table part.
Consider that this SP is being activated by another SP many times. Do you mean I need to create such Numbers tables for each invocation of these SP? What is the range of these numbers and how do I set it? I will be happy if you could elaborate the previous code sample...
 
OK.

First of all. The Numbers table should be a permanent table in your database. Once you create this table, you don't have to recreate it ever again. This is where much of the speed improvement comes from.

Code:
Create Table Numbers(N Integer Primary Key Clustered)

Declare @i Integer
Set @i = 1

While @i <= [!]100000[/!]
  Begin
    Insert Into Numbers Values(@i)
    Set @i = @i + 1
  End

The first line creates the table. Then, we set up a loop so that we can add records to the table. Notice the part in red. This value is 100,000 So, after the code is executed, the numbers table will have a record with every value between 1 and 100,000. You can increase the number if need be to accomodate your data. Since this table has only 1 field (an integer field), the size will be relatively small, but will allow you to join to this table to find missing numbers.

Now, for the rest. First, let me explain that I presented the code the way I did to simplify the example.

Code:
declare @Temp Table(CaseNumber Integer)

Insert Into @Temp Values(4)
Insert Into @Temp Values(5)
Insert Into @Temp Values(7)
Insert Into @Temp Values(12)

Declare @Min Integer
Declare @Max Integer
Declare @Missing VarChar(8000)
Set @Missing = ''

Select @Min = Min(CaseNumber),
       @Max = Max(CaseNumber)
From   @Temp

Select @Missing = @Missing + Convert(VarChar(20), Numbers.N) + ','
From   Numbers    
       Left Join @Temp T
         On Numbers.N = T.CaseNumber
Where  T.CaseNumber Is NULL
       And Numbers.N Between @Min And @Max

If Right(@Missing, 1) = ','
    Set @Missing = left(@Missing, Len(@Missing)-1)

Select @Missing

The @Temp stuff is actually a table variable. It's only purpose here is to demonstrate the concept. Ultimately, your final query would NOT have an @Temp table.

Your query does retrieve the min and max case numbers, so you'll want to keep that part. If I understand correctly, you only want the missing numbers that are between the min and max numbers.

Next, let me explain the left join. The Numbers table is guaranteed to have every number between 1 and 100,000. So, we can use this table as the LEFT table in the join. When the RIGHT table has a match, the value will be shown in that column. If there is no match, then the Numbers.N value will be shown, the value from the right table will be NULL. We filter on the NULLs so that only records that don't exist will be returned.

The last part to explain is the actual concatenation.
Select @Missing = @Missing + Convert(VarChar(20), Numbers.N) + ','

It seems weird, but for each row returned by this query, the value from the numbers table is converted to a string (varchar(20)) and added to the @Missing variable.

I hope this helps you to understand the query better.

-George

Strong and bitter words indicate a weak cause. - Fortune cookie wisdom
 
It does George thank you very much for taking the time to explain so nicely!
 
Hello again George,

I have a problem with the query:
Select @Missing = @Missing + Convert(VarChar(20), Numbers.N) + ','
From Numbers
Left Join @Temp T
On Numbers.N = T.CaseNumber
Where T.CaseNumber Is NULL
And Numbers.N Between @Min And @Max

It's very wierd but it behaves like INNER JOIN not LEFT JOIN. When I run it I receive only numbers that exist on both tables instead of all the numbers at the Numbers table with a corresponding NULL value from the other table as it should. Any idea why...?
 
Status
Not open for further replies.

Part and Inventory Search

Sponsor

Back
Top