October 4, 2004 at 8:21 am
I recieve the following error when a stored procedure is executed against the database. The stored procedure is a select statement used in reporting. When the select statement from sp is run in query analyzer it run ok. Any thought on why it would work one way and not the other? Thanks
Error when Stored Procedure is Run:
Server: Msg 1205, Level 13, State 2, Procedure sp_Advantage_SalesRecap, Line 23
Transaction (Process ID 76) was deadlocked on thread | communication buffer resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Select Statement from SP:
select
sd.ordertype,
sd.invoicenumber,
sd.itemnumber,
im.jdedescription,
sd.parentnumber,
pab.description as parent,
sd.gldate,
sd.CustomerPONumber,
sum(sd.quantityshipped) as Shipped,
sum(sd.Grosslist) as Grosslist,
Avg(sd.listprice) as Listprice
from salesdetail sd
left outer join parentaddressbook pab
on sd.parentnumber = pab.parentnumber
left outer join itemmaster im
on sd.itemnumber = im.itemnumber
where sd.gldate >= convert(varchar,@@StartDate,101) --parameter passed from user
and sd.gldate <= convert(varchar,@@EndDate,101)--parameter passed from user
and sd.parentnumber in (88885332,88885262,88885353,88885355,88885972)
and sd.recordtype = 'S'
and sd.company = '00010'
and sd.laststatus = '600'
Group by
sd.ordertype,
sd.invoicenumber,
sd.itemnumber,
im.jdedescription,
sd.parentnumber,
pab.description,
sd.gldate,
sd.CustomerPONumber
October 4, 2004 at 8:56 am
It seems be heavy query if there is a lot of rows in those tables. If this is for the reporting, I don't think you need to run this under "read committed" mode so I would use "Dirty read" by putting this in front of the store proc
October 4, 2004 at 12:29 pm
The largest table has about 8 million records, the others are less then several thousand records. The SQL statement returns approx 3,000 records with the passed date ranges of one month.
The only thing in the stored proc besides the above select statement, is declaring the passed variable if they are null.
I'm still unclear on why the SQL statement would work in query analyzer and not when the stored procedure is executed?
Thanks
October 4, 2004 at 3:17 pm
In principle, there is no reason why a sp would deadlock but QA would not. Also, although the query processes a lot of rows, it isn’t particularly complicated so we need to look elsewhere.
What else does the SP do? Are there any transactions in the SP or maybe is it called from a client which has a transaction open. Check the locks while the query is running to see what is happening. We need more info in tis aera.
You could try dirty reads “Read uncommitted” as suggested. This would probably solve the problem but I wouldn’t recommend it as I have found that the query can crash and/or produce bizarre results as records can be read more than once or missed altogether if they get moved around the table because of key changes or block splits especially with long queries on tables that are being updates.
You could also put the main part of the query in a derived table. It is not bound to help, but this technique often makes the query much faster and will therefore reduce the chance of deadlocks.
Try the following query and compare the query plans. It assumes that pab.parentnumber and im.itemnumber are unique keys in their respective tables.
select
derived.ordertype,
derived.invoicenumber,
derived.itemnumber,
im.jdedescription,
derived.parentnumber,
pab.description as parent,
derived.gldate,
derived.CustomerPONumber,
derived.Shipped,
derived.Grosslist,
derived.Listprice
from ( --This subquery gets the qualifying rows
-- and calculates the groupings
Select
sd.ordertype,
sd.invoicenumber,
sd.itemnumber,
sd.parentnumber,
sd.gldate,
sd.CustomerPONumber,
sum(sd.quantityshipped) as Shipped,
sum(sd.Grosslist) as Grosslist,
Avg(sd.listprice) as Listprice
from salesdetail sd
where sd.gldate >= convert(varchar,@@StartDate,101)
and sd.gldate <= convert(varchar,@@EndDate,101)
and sd.parentnumber in (88885332,88885262,88885353,88885355,88885972)
and sd.recordtype = 'S'
and sd.company = '00010'
and sd.laststatus = '600'
group by
sd.ordertype,
sd.invoicenumber,
sd.itemnumber,
sd.parentnumber,
sd.gldate,
sd.CustomerPONumber
) derived
-- This bit then joins on the descriptions
-- and does the sort on just the selected rows.
left outer join parentaddressbook pab
on derived.parentnumber = pab.parentnumber
left outer join itemmaster im
on derived.itemnumber = im.itemnumber
Also, if sd.gldate is indexed, then you should pass the dates into the sp as datetime and use them directly rather than converting them. It often makes a big difference to the performance.
October 4, 2004 at 3:25 pm
Above query seems to goo and I agree w/ Douglas that often Derived query help to improve the performance.
Also, I am also curious about why proc makes dead lock not dynamic query. Have you recomplie the proc? If you haven't recomplied the proc for a long time, since store proc is precompiled query, it might uses the old stastics.
Why don't you compare by putting those proc and query in one pane and compare the plan.
October 5, 2004 at 7:35 am
Since the table joins are left outter joins you could create a table variable with the full results set definition, insert the main table into it, and update the other columns with two separate update queries (one to each table).
Using table variables is often faster than left outter joins but it is query dependent.
Since you are only accessing a single table and a table variable at the same time, there will be no deadlocks.
October 5, 2004 at 7:45 am
Thanks for all the input - I will try each and see which one works best.
The only thing the stored procedure does is declare and set the variables used in the select statement based on the parameters passed from the report Below is the complete stored Procedure:
CREATE PROCEDURE [sp_Advantage_SalesRecap] @Startdate datetime, @Enddate datetime
AS
--Created 4/5/2004
--Created By Jedi Pabst
Declare @@Startdate datetime
Declare @@Enddate datetime
if @StartDate is null
set @@Startdate = getdate() - 7
Else
set @@Startdate = @StartDate
if @Enddate is null
set @@Enddate = getdate() - 1
Else
set @@Enddate = @EndDate
select
sd.ordertype,
sd.invoicenumber,
sd.itemnumber,
im.jdedescription,
sd.parentnumber,
pab.description as parent,
sd.gldate,
sd.CustomerPONumber,
sum(sd.quantityshipped) as Shipped,
sum(sd.Grosslist) as Grosslist,
Avg(sd.listprice) as Listprice
from salesdetail sd
left outer join parentaddressbook pab
on sd.parentnumber = pab.parentnumber
left outer join itemmaster im
on sd.itemnumber = im.itemnumber
where sd.gldate >= convert(varchar,@@StartDate,101)
and sd.gldate <= convert(varchar,@@EndDate,101)
and sd.parentnumber in (88885332,88885262,88885353,88885355,88885972)
and sd.recordtype = 'S'
and sd.company = '00010'
and sd.laststatus = '600'
Group by
sd.ordertype,
sd.invoicenumber,
sd.itemnumber,
im.jdedescription,
sd.parentnumber,
pab.description,
sd.gldate,
sd.CustomerPONumber
GO
I have noticed that the stored procudure will work when the date range is less than 3 weeks but will not work when larger than 3 weeks!?! The report/stroed procedure has been working as a scheduled job with null values for @StartDate and @Enddate since it has been created. The users has just started running the job with a larger date range of a week.
Thanks
Bob
October 5, 2004 at 8:05 am
sd.ordertype,
sd.invoicenumber,
sd.itemnumber,
sd.parentnumber,
sd.gldate,
sd.CustomerPONumber,
sum(sd.quantityshipped) as Shipped,
sum(sd.Grosslist) as Grosslist,
Avg(sd.listprice) as Listprice
into #tempTable
from salesdetail sd
where sd.gldate >= convert(varchar,@@StartDate,101)
and sd.gldate <= convert(varchar,@@EndDate,101)
and sd.parentnumber in (88885332,88885262,88885353,88885355,88885972)
and sd.recordtype = 'S'
and sd.company = '00010'
and sd.laststatus = '600'
group by
sd.ordertype,
sd.invoicenumber,
sd.itemnumber,
sd.parentnumber,
sd.gldate,
sd.CustomerPONumber
select
derived.ordertype,
derived.invoicenumber,
derived.itemnumber,
im.jdedescription,
derived.parentnumber,
pab.description as parent,
derived.gldate,
derived.CustomerPONumber,
derived.Shipped,
derived.Grosslist,
derived.Listprice
from #tempTable derived
-- This bit then joins on the descriptions
-- and does the sort on just the selected rows.
left outer join parentaddressbook pab
on derived.parentnumber = pab.parentnumber
left outer join itemmaster im
on derived.itemnumber = im.itemnumber
October 5, 2004 at 8:09 am
I almost posted about this earlier, but held back. I just can't see how your date range check works. You are performing date comparisons in 101 format which is MM/DD/YYYY. As a result, all January 1sts of every year come first, then all January seconds, and so on. Therefore, the string scan for 01/01/2004 to 01/31/2004 will match against 01/02/2003, 01/31/2002, and so on. Try the following in QA:
if '01/01/2004' < '01/02/2002' print 'WRONG!'
if '01/31/2004' > '01/02/2002' print 'Sort of OK'
if '01/01/2004' < '01/31/2002' print 'WRONG!'
if '01/31/2004' > '01/31/2002' print 'Sort of OK'
Try converting the SP where clause to this:
where CONVERT(datetime, sd.gldate ) >= @@StartDate
and CONVERT(datetime, sd.gldate ) <= @@EndDate
It will produce a really ugly Index scan, but I'd be interested in if it changes your symptoms.
October 5, 2004 at 8:23 am
Try converting the SP where clause to this:
where CONVERT(datetime, sd.gldate ) >= @@StartDate
and CONVERT(datetime, sd.gldate ) <= @@EndDate
I tried the above change and it gave me that same results. I also tried sd.gldate <= @@Startdate in the statement with no luck.
On to the temp table and/or derived.
Bob
October 5, 2004 at 8:27 am
Hmm.. I think, the original range check will work. Aaron, you right, it will convert to MM/DD/YYYY format but it still keeps "Datetime" type format so when you compare the time, it recognized as 12:00am. Try to print the variable right after the convert, you will know what I mean. So if you want to get the range of certain day but starting begining of the day, you would use that convert. This is the example.
Since, the original query wasn't compare varchar to varchar but datetime to varchar, it should work.
DECLARE @Date1 Datetime,
@Date2 Datetime
SET @Date1=convert(varchar,getdate(),101)
SET @Date2=getdate()
print @Date1
print @Date2
IF @Date1 < @Date2
Print 'Good'
ELSE
Print 'Bad'
One thing that I noticed about that range is that, I usually check between MM/DD/YYYY 12:00am and MM/DD/YYYY 11:59:59.9999 in other words, I'd usually use >= and < for range check.
where sd.gldate >= convert(varchar,@@StartDate,101)
and sd.gldate < convert(varchar,@@EndDate,101)
So that I can exclude the 12:00am insert. But that is all depends on what you need to do though so there is no right or wrong about that but I just want to say, that comparison will work unless you are talking other factor that I don't know. Which I'd love to hear about it
October 5, 2004 at 8:45 am
I updated the stored procedure to load the sql statement into a temp table and everything seems to be working fine. thanks everyone for the help.
Most of my work is writing sql statements to be used in reporting and to present the sql statements via stored procedures. Should this methodology (loading into temp tables) be the standard for this type of work?
Any thoughts would be appreciated.
Thanks
Bob
October 5, 2004 at 8:52 am
I am glad that worked. I do not recommend to be the standard for this type of work though. All depends on the situation and query. Sometimes Derived query works the best, some times, temp table works the best and some times, just simple join work the best.
One of the rule of the thumb that I am using is that, if you are dealing w/ large table and need to join to get data but when you need to get a few amount of data from the large table and need to join and it is reporting, that I'd use temp table.
If this has to be running every minitues, I'd use physical staging table to load and truncate table so that it won't use "TempDB" and do the same kind of work.
I just want to say, all depends on situation.
October 5, 2004 at 2:03 pm
One tip:
Try use hint nolock , if possible.
You´ll prevent exclusive lock in table.
Other way , try rowlock hint to minimize locks.
Hildevan O Bezerra
October 5, 2004 at 2:43 pm
Arran, the dates in the query will work OK because SQL implicitly converts then back to dates to compare with the sd.gldate column which is declared as a date. In you example you are comparing 2 strings so it doesn’t work.
Bob, on the derived or temp table issue, there generally isn’t much difference as SQL generally creates a temporary table anyway. The derived table sometimes performs better if the derived table can be processed by the outer query without being stored in between. It won’t happen in this case however, because the derived query has a GROUP BY which needs a temp table to create. The temp table solution is more versatile and generally easier to code.
The fastest way I can see to improve the performance will be to
1) index the sd.gldate column
2) move the date calculations to a separate procedure as follows
CREATE PROCEDURE [sp_Advantage_SalesRecap] @Startdate datetime, @Enddate datetime
AS
select
sd.ordertype,
sd.invoicenumber,
sd.itemnumber,
im.jdedescription,
sd.parentnumber,
pab.description as parent,
sd.gldate,
sd.CustomerPONumber,
sum(sd.quantityshipped) as Shipped,
sum(sd.Grosslist) as Grosslist,
Avg(sd.listprice) as Listprice
from salesdetail sd
left outer join parentaddressbook pab
on sd.parentnumber = pab.parentnumber
left outer join itemmaster im
on sd.itemnumber = im.itemnumber
where sd.gldate >= @Startdate
and sd.gldate <= @Enddate
and sd.parentnumber in (88885332,88885262,88885353,88885355,88885972)
and sd.recordtype = 'S'
and sd.company = '00010'
and sd.laststatus = '600'
Group by
sd.ordertype,
sd.invoicenumber,
sd.itemnumber,
im.jdedescription,
sd.parentnumber,
pab.description,
sd.gldate,
sd.CustomerPONumber
GO
CREATE PROCEDURE [sp_Advantage_SalesRecap] @Startdate datetime, @Enddate datetime
WITH RECOMPILE AS
--Created 4/5/2004
--Created By Jedi Pabst
Declare @@Startdate datetime
Declare @@Enddate datetime
if @StartDate is null
set @@Startdate = getdate() - 7
Else
set @@Startdate = @StartDate
if @Enddate is null
set @@Enddate = getdate() - 1
Else
set @@Enddate = @EndDate
Exec sp_Advantage_SalesRecap_inner @@Startdate,@@Enddate
go
This might seem a bit pointless but it will in fact perform much better providing the sd.gldate is indexed.
In the original version, SQL optimizer cannot predict what dates will be used at the time it optimises the query so it will assume 20% of the table which in you case is 20% of 8,000,000 = 2,000,000. Your earlier comments show that this is wildly out which means that the generated plan is not likely to be optimal. In my example, even though it appears the same is subtly different. Because the parameters are not manipulated within the inner procedure, and because the SP is recompiled when executed, the optimiser can effectively ‘hard code’ the query using the values passed as parameters. The optimiser can then see that say 1 week selection in say 5 years history is only 1 / 5 * 52 or about 4% of the rows. This means that the (new?) index is almost bound to be used to retrieve the records rather than doing a table scan.
Makes your head spin doesn’t it - Fun though; let us know how you get on.
Cheers
Viewing 15 posts - 1 through 14 (of 14 total)
You must be logged in to reply to this topic. Login to reply