Re: Please help me optimise this stored procedure.
From: Hugo Kornelis (hugo_at_pe_NO_rFact.in_SPAM_fo)
Date: 04/23/04
- Next message: Hugo Kornelis: "Re: Found a bug - is it known? can others reproduce it?"
- Previous message: Tibor Karaszi: "Re: in vs. exists"
- In reply to: Tom Spence: "Please help me optimise this stored procedure."
- Messages sorted by: [ date ] [ thread ]
Date: Fri, 23 Apr 2004 10:52:10 +0200
On Fri, 23 Apr 2004 12:21:49 +1000, Tom Spence wrote:
>Hello fellow SQL users.
>
>I am developing an online store, and as part of the code each time an order
>changes status (ie, pending -> complete -> cancel) it updates the totals for
>each item in the order to reflect this, so that when I am looking at
>individual items it is easy to tell how many of said item is pending,
>complete, ect. Here is my stored procedure:
(snip)
>Basically, the procedure runs fine, but I recently had a big order (over 200
>individual items), and the prodecure took about 10 minutes to execute. Not
>cool for a customer who has to wait for it when placing the order.
Hi Tom,
Steve and Andrew already gave you suggestions how to improve your
query. But do you really need this query? You have designed it to
count all orders for any title in the affected order. For this big
order, SQL Server had to loop through for each of the 200 items in
that order and execute the subquery (that involves checking ALL orders
for that item) eight times (for each status). The suggestions of Steve
and Andrew reduce the subquery execution to once for each item in the
order and give the optimizer more options to consider alternative
plans (at the price of using non-portable proprietary syntax), but it
is still a lot of work!
I assume your query is run in a trigger. Why not use the data in the
inserted and deleted pseudo tables to add or subtract the appropriate
amount to the counters?
Something like this might work. I suggest you try the ANSI-standard
update syntax first (as below); if it still doesn't perform quick
enough, use the techniques suggested by Steve and Andrew to speed it
up even more. Note: trigger code is untested!
CREATE TRIGGER UpdateTally
ON Orders
AFTER UPDATE
AS
IF @@rowcount = 0 RETURN
IF UPDATE(OrderID)
BEGIN
-- include error handling here
RETURN
END
IF NOT UPDATE(OrderStatusID) RETURN
-- reduce counters for "old" status
-- and increase counters for "new" status
UPDATE Titles
SET Pending = Pending
- COALESCE(
(SELECT SUM(quantity)
FROM deleted AS d
INNER JOIN OrderTitles AS OT
ON OT.OrderID = d.OrderID
AND OT.TitleID = Titles.TitleID
WHERE d.OrderStatusID = 1), 0)
+ COALESCE(
(SELECT SUM(quantity)
FROM inserted AS i
INNER JOIN OrderTitles AS OT
ON OT.OrderID = i.OrderID
AND OT.TitleID = Titles.TitleID
WHERE i.OrderStatusID = 1), 0),
UnderProcess = UnderProcess
- COALESCE(
(SELECT SUM(quantity)
FROM deleted AS d
INNER JOIN OrderTitles AS OT
ON OT.OrderID = d.OrderID
AND OT.TitleID = Titles.TitleID
WHERE d.OrderStatusID = 2), 0)
+ COALESCE(
(SELECT SUM(quantity)
FROM inserted AS i
INNER JOIN OrderTitles AS OT
ON OT.OrderID = i.OrderID
AND OT.TitleID = Titles.TitleID
WHERE i.OrderStatusID = 2), 0),
Sent = Sent
- COALESCE(
(SELECT SUM(quantity)
FROM deleted AS d
INNER JOIN OrderTitles AS OT
ON OT.OrderID = d.OrderID
AND OT.TitleID = Titles.TitleID
WHERE d.OrderStatusID = 3), 0)
+ COALESCE(
(SELECT SUM(quantity)
FROM inserted AS i
INNER JOIN OrderTitles AS OT
ON OT.OrderID = i.OrderID
AND OT.TitleID = Titles.TitleID
WHERE i.OrderStatusID = 3), 0)
WHERE EXISTS
(SELECT *
FROM inserted AS i
INNER JOIN OrderTitles AS OT
ON OT.OrderID = i.OrderID
AND OT.TitleID = Titles.TitleID
INNER JOIN deleted AS d
ON d.OrderID = i.OrderID
WHERE i.OrderStatusID <> d.OrderStatusID)
go
Some notes:
* The trigger assumes that OrderID will never be changed in an update.
I included a check for this, but proper error handling needs to be
added .
* The final where is very important. It restricts the update to only
those titles that are part of an order that has actually changed
status.
* I was already done with the trigger code query when I realised that
you've got your table names prefixed with tb and in singular instead
of plural. Also, I forgot that the counters are in another table, not
in the titles table itself. I hope you don't mind I left the table
names as I typed them.
* I left out the SentDay, SentWeek, etc columns. I suggest you remove
them from your tables and calculate them when you collect data for
your reports. This was actually a flaw in your design: on change of
orderstatus, you recalculate these columns for only the titles that
are part of that order. If nobody orders title 11754 for the next
three weeks, than these counters for title 11754 will be unchanged.
They will still reflect the number sent in the week preceding the last
order change for an order with title 11754, not the number sent in the
week preceding the report date.
Best, Hugo
-- (Remove _NO_ and _SPAM_ to get my e-mail address)
- Next message: Hugo Kornelis: "Re: Found a bug - is it known? can others reproduce it?"
- Previous message: Tibor Karaszi: "Re: in vs. exists"
- In reply to: Tom Spence: "Please help me optimise this stored procedure."
- Messages sorted by: [ date ] [ thread ]
Relevant Pages
|
