Optimizing data calculation inside SP

Question

Post reply

Optimizing data calculation inside SP

a_mail.address

SSC Enthusiast

Points: 104
More actions
October 2, 2007 at 8:46 am

#121839

Hi,
I created a SP which is supposed to calculate some values for me and return them as a resultset. I have a RequestTime field, and a ResponseTime field; This sp should calculate how much time does it take for us to respond to a customer's request. if it is more than a specific time, this sp should calculate the extra time and its fine base on a constant fine-per-extra-minute value.
It should also calculate total fine for all records.
To do so, I wrote it as this:
CREATE PROCEDURE up_responsetime
@sdate smalldatetime,
@edate smalldatetime,
@TotalFine int OUTPUT
AS
DECLARE @Fine_Per_Min int
DECLARE @Max_ResponseTime int
SET @Fine_Per_Min = 3
SET @Max_ResponseTime = 120
SELECT ID,RequestDate,RequestTime,ResponseDate,ResponseTime,
-- Calculate exceeded amount of time for each record
DATEDIFF(minute,RequestDate+RequestTime,ResponseDate+ResponseTime) - @Max_ResponseTime as ExtraTime,
-- Calculate fine for each record
(DATEDIFF(minute,RequestDate+RequestTime,ResponseDate+ResponseTime) - @Max_ResponseTime) * @Fine_Per_Min as Fine
FROM CusRequests
WHERE RequestDate BETWEEN @sdate AND @edate
--Calculate sum of all fines and return it in TotalFine variable.
SELECT @TotalFine = SUM((DATEDIFF(minute,RequestDate+RequestTime,ResponseDate+ResponseTime) - @Max_ResponseTime) * @Fine_Per_Min) FROM CusRequests
GO
but I have two concerns about this:
1- Calculated Fine field returns a negative figure if the ResponseTime is in the desired period of time. But I want it to return zero for such cases, and return only a positive figure when extra time was spent on responding to the request.
2- As you can see, there are many redundant calculation in this code, and this will affect its performance. I wanna know if there is any more optimized way to write such a code?
I appreciate any help,
Thanks alot
p.s. sorry if the post was lengthy.

Viewing 12 posts - 1 through 11 (of 11 total)

You must be logged in to reply to this topic. Login to reply

mrpolecat SSCertifiable Points: 6964 More actions · Answer 1

Here is the return 0 part

case when

(DATEDIFF(minute,RequestDate+RequestTime,ResponseDate+ResponseTime) - @Max_ResponseTime) > 0 then

(DATEDIFF(minute,RequestDate+RequestTime,ResponseDate+ResponseTime) - @Max_ResponseTime) * @Fine_Per_Min

else 0 end as Fine

a_mail.address SSC Enthusiast Points: 104 More actions · Answer 2

Here is the return 0 part

Thanks alot.

Is there any way to optimize its performance too?

Jeff Moden SSC Guru Points: 1003863 More actions · Answer 3

This is going to sound mean and I don't mean it that way...

The table is improperly designed... why do you have separate date and time columns? And, I'll bet they're probably VARCHAR to boot so lot's of intrinsic conversions are slowing things down, as well. Things like the RequestDate and RequestTime should be in a single datetime column.

--Jeff Moden

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)

a_mail.address SSC Enthusiast Points: 104 More actions · Answer 4

Thank you Jeff,

No, RequestDate and RequestTime are not VARCHAR, just SMALLDATTIME.

They are seperated because RequestTime and RequestDate were just some symbolic names to make my query's structure clear. In my project such fields do not exists. The real name for time field is WreckageTime, and for the date field, AnnounceDateTime. The first refers to the time a particular system is down, the latter refers to the date & time in which the wreckage was reported. Actually WreckageTime is added to date part of AnnounceDateTime to generate a smalldatetime value to be used particularly in this query. I removed such stuff and used symbolic names for the sake of simplicity.

Jeff Moden SSC Guru Points: 1003863 More actions · Answer 5

Actually WreckageTime is added to date part of AnnounceDateTime to generate a smalldatetime value to be used particularly in this query

But, that's what I'm talking about... I wasn't talking about the column names... I'm talking about the fact that you have to add a time to a date to get something else... not the way to do it... WreckageTime should be WreckageDateTime...

--Jeff Moden

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)

a_mail.address SSC Enthusiast Points: 104 More actions · Answer 6

WreckageTime should be WreckageDateTime...

Such a tthing doesn't exists. In this project we don't have a WreckageDate. Such a thing is never saved, because it is not entered in any of the input forms. They do not care about wreckage date, only it's time is important. The only query we need to have a combination of date/time is this current query. In all the other queries WreckageTime is used seperately. Take note that AnnounceDateTime is a different concept and is used in a different way.

So wreckage date is meaningless all over the project; therefore, it is not saved anywhere.

Jack Corbett SSC Guru Points: 184393 More actions · Answer 7

a_mail.address (10/6/2007)
Thank you Jeff,
No, RequestDate and RequestTime are not VARCHAR, just SMALLDATTIME.
They are seperated because RequestTime and RequestDate were just some symbolic names to make my query's structure clear. In my project such fields do not exists. The real name for time field is WreckageTime, and for the date field, AnnounceDateTime. The first refers to the time a particular system is down, the latter refers to the date & time in which the wreckage was reported. Actually WreckageTime is added to date part of AnnounceDateTime to generate a smalldatetime value to be used particularly in this query. I removed such stuff and used symbolic names for the sake of simplicity.

Can you post the actual code? I am confused as to what the actual fields are and what they mean. In the quote you say there are 2 fields WreckageTime and AnnounceDateTime, but the original post includes 4 fields ResponseDate, ResponseTime, RequestDate, and RequestTime. Are there 4 or 2 fields in the database? Is WreckageTime a SmallDateTime or an integer representing the # of minutes the system has been down?

As far as the posted query, you might want to use a Temp Table (SQL 7) or a table variable (2000) like this:

CREATE PROCEDURE up_responsetime

@sdate smalldatetime,

@edate smalldatetime,

@TotalFine int OUTPUT

AS

DECLARE @Fine_Per_Min int

DECLARE @Max_ResponseTime int

SET @Fine_Per_Min = 3

SET @Max_ResponseTime = 120

Declare @table ({Fields you want from main table}, ExtraTime Int)

Insert Into @Table

Select

{Fields you want from main table},

DATEDIFF(minute,RequestDate+RequestTime,ResponseDate+ResponseTime) - @Max_ResponseTime

From

CustRequests

Where

RequestDate BETWEEN @sdate AND @edate

Select

{Select Fields},

ExtraTime,

ExtraTime * @Fine_Per_Min as Fine

From

@Table T

Where

ExtraTime > 0

Select @TotalFine = Sum(ExtraTime * @Fine_Per_Min) From @Table Where ExtraTime > 0

Jack Corbett
Consultant - Straight Path Solutions
Check out these links on how to get faster and more accurate answers:
Forum Etiquette: How to post data/code on a forum to get the best help
Need an Answer? Actually, No ... You Need a Question

a_mail.address SSC Enthusiast Points: 104 More actions · Answer 8

Hi Jack,

I didn't post the original query because the case with the original one was kinda complicated and needed alot of extra explanation which was not necessary for this particular query, so i created a simple imaginative scenario and wrote a similar query based on it, to keep the topic focused on this particular issue.

The recommendations posted so far helped me to write the code i needed and the query returns the result i wanted. Regarding to your recommendation to use a Table variable, I wanna know does it affect performance? Beside that, can i add a new field to this table variable i.e. a field which does not exist in the available tables? This later question is not so important and i didn't mention it in the original post, but when I saw your code, I thought it might be possible to define a new field for this Table variable.

Thanks alot

Jack Corbett SSC Guru Points: 184393 More actions · Answer 9

When declaring a table variable you can define it with any columns you need. In my example I added a column ExtraTime, which does not exist anwhere else.

As far as performance, I actually don't think having the calculation in the query multiple times is going to have much of an affect on performance. I would change it because I find using a table variable or a temp table makes that kind of code more readable and in most instances i have worked with any performance difference has been negligible. Also many times breaking a process down into small chunks can boost performance.

Jack Corbett
Consultant - Straight Path Solutions
Check out these links on how to get faster and more accurate answers:
Forum Etiquette: How to post data/code on a forum to get the best help
Need an Answer? Actually, No ... You Need a Question

a_mail.address SSC Enthusiast Points: 104 More actions · Answer 10

a_mail.address

SSC Enthusiast

Points: 104

October 8, 2007 at 3:22 pm

#740175

Thank you for your fast reply, Jack.

Jeff Moden SSC Guru Points: 1003863 More actions · Answer 11

a_mail.address (10/7/2007)
Such a tthing doesn't exists. In this project we don't have a WreckageDate. Such a thing is never saved, because it is not entered in any of the input forms. They do not care about wreckage date, only it's time is important. The only query we need to have a combination of date/time is this current query. In all the other queries WreckageTime is used seperately. Take note that AnnounceDateTime is a different concept and is used in a different way.
So wreckage date is meaningless all over the project; therefore, it is not saved anywhere.

Sorry abouyt that... I meant RequestDate and RequestTime should simply be RequestDateTime.

--Jeff Moden

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)