May 30, 2011 at 6:50 am
I have the following set of T-SQL -- can anyone see a method of changing it to a set based operation instead of being cursor based?
If it cannot be converted are there any optimization suggestions?
Tony
DECLARE @col1 VARCHAR(1000)
DECLARE @TotalInvoiceAmount decimal(18,6)
DECLARE @termsdiscountamount decimal(18,6)
DECLARE @InvoiceSubTotal decimal(18,6)
DECLARE @supplierkey VARCHAR(1000)
DECLARE @UnitsShipped decimal(18,6)
DECLARE @GrossAmount decimal(18,6)
DECLARE @NetAmount decimal(18,6)
DECLARE @ChargeType varchar(20)
DECLARE @ShipToKey varchar(20)
DECLARE @RebateUnits decimal(18,6)
DECLARE @TotalGross decimal(18,6)
DECLARE @TotalNet decimal(18,6)
DECLARE @TotalUnits decimal(18,6)
DECLARE @RuleId decimal(18,6)
DECLARE @RuleUnitType integer
DECLARE @TotalChargeUnits decimal(18,6)
DECLARE @TotalChargeGross decimal(18,6)
DECLARE @TotalChargeNet decimal(18,6)
DECLARE @UnitsToDeductFromTerms decimal(18,6)
DECLARE @TermsRuleType as integer
DECLARE @TermsPercent as decimal(18,6)
set @RuleUnitType = 1
set @TermsRuleType = 1
truncate table rb_ruleinvoice
DECLARE cur CURSOR STATIC LOCAL FOR select invoicekey,rb_ruleid from rb_invoicestoprocess
OPEN cur
WHILE 1 = 1
BEGIN
FETCH cur INTO @col1,@RuleId
IF @@fetch_status <> 0
BREAK
DECLARE curMain CURSOR STATIC LOCAL FOR
select TotalInvoiceAmount,InvoiceSubTotal,termsdiscountamount,supplierkey,shiptokey from invoicetotal a,invoice b where a.invoicekey = b.invoicekey and a.invoicekey = @col1
open curMain
while 1=1
BEGIN
FETCH curMain INTO @TotalInvoiceAmount, @InvoiceSubTotal, @termsdiscountamount, @supplierkey,@ShipToKey
IF @@fetch_status <> 0
break
end
Deallocate curMain
set @UnitsToDeductFromTerms = 0
set @RebateUnits = 0
set @TotalGross = 0
set @TotalNet = 0
set @TotalUnits = 0
set @TotalChargeUnits = 0
set @TotalChargeGross = 0
set @TotalChargeNet =0
--print cast(@col1 as nvarchar(30))
declare curLineItems CURSOR STATIC LOCAL FOR
select sum(unitsshipped),SUM(grossamount),SUM(netamount) from
(
select UnitsShipped,GrossAmount,NetAmount from InvoiceLineItem a,Invoice b where a.invoicekey = @col1 and a.invoicekey = b.invoicekey and a.ProductRebateCategory in (select c.categorynumber from RB_RuleCategory c where c.rb_ruleid = 8251 and c.SupplierId = B.supplierkey)
union
select 1,a.TaxAmount,a.TaxAmount from invoicetotaltax a,Invoice b where a.invoicekey = @col1 and a.invoicekey = b.invoicekey and a.ProductRebateCategory in (select c.categorynumber from RB_RuleCategory c where c.rb_ruleid = 8251 and c.SupplierId = B.supplierkey)
) as tftest
open curLineItems
while 1=1
BEGIN
FETCH curLineItems INTO @UnitsShipped,@GrossAmount,@NetAmount
IF @@fetch_status <> 0
break
set @TotalGross = @TotalGross + @GrossAmount
set @TotalNet = @TotalNet + @NetAmount
set @TotalUnits = @TotalUnits + @UnitsShipped
--print cast(@col1 as nvarchar(30)) + ' ' + cast(@UnitsShipped as nvarchar(30))
end
Deallocate curLineItems
--declare curTax CURSOR STATIC LOCAL FOR
--select 1,a.TaxAmount,a.TaxAmount as AmountType from invoicetotaltax a,Invoice b where a.invoicekey = @col1 and a.invoicekey = b.invoicekey and a.ProductRebateCategory in (select c.categorynumber from RB_RuleCategory c where c.rb_ruleid = 8251 and c.SupplierId = B.supplierkey)
--open curTax
--while 1=1
--BEGIN
--FETCH curTax INTO @UnitsShipped,@GrossAmount,@NetAmount
--IF @@fetch_status <> 0
--break
--set @TotalGross = @TotalGross + @GrossAmount
--set @TotalNet = @TotalNet + @NetAmount
--set @TotalUnits = @TotalUnits + @UnitsShipped
--end
--Deallocate curTax
declare curCharges CURSOR STATIC LOCAL FOR
select 1,Amount,Amount,Type as AmountType from InvoiceTotalAdditionalCosts a,Invoice b where
a.invoicekey = @col1
and a.invoicekey = b.invoicekey
and a.ProductRebateCategory in (
select c.categorynumber from RB_RuleCategory c where c.rb_ruleid = 8251
and c.SupplierId = B.supplierkey
)
open curCharges
while 1=1
BEGIN
FETCH curCharges INTO @UnitsShipped,@GrossAmount,@NetAmount,@ChargeType
IF @@fetch_status <> 0
break
if upper(@ChargeType) = 'C'
begin
if @GrossAmount < 0
begin
set @GrossAmount = @GrossAmount * -1
end
if @NetAmount < 0
begin
set @NetAmount = @NetAmount * -1
end
end
set @TotalGross = @TotalGross + @GrossAmount
set @TotalNet = @TotalNet + @NetAmount
set @TotalUnits = @TotalUnits + @UnitsShipped
-- set the charge units
set @TotalChargeUnits = @TotalChargeUnits + @UnitsShipped
set @TotalChargeGross = @TotalChargeGross + @GrossAmount
set @TotalChargeNet = @TotalChargeNet + @NetAmount
end
Deallocate curCharges
-- now calculate the final totals
select @RebateUnits = case (@RuleUnitType)
WHEN 1 THEN @TotalGross
WHEN 2 THEN @TotalNet
WHEN 3 THEN @TotalUnits
END
select @UnitsToDeductFromTerms= case (@RuleUnitType)
WHEN 1 THEN @TotalChargeGross
WHEN 2 THEN @TotalChargeNet
WHEN 3 THEN @TotalChargeUnits
END
if @termsdiscountamount <> 0 and @TermsRuleType <> 0
begin
if @TermsRuleType = 1
begin
set @TermsPercent = @termsdiscountamount / @TotalInvoiceAmount
end
else
begin
set @TermsPercent = @termsdiscountamount / @InvoiceSubTotal
end
if @TermsPercent <> 0
begin
set @RebateUnits = @RebateUnits - @UnitsToDeductFromTerms
set @RebateUnits = @RebateUnits - ((@UnitsToDeductFromTerms) * @TermsPercent)
set @RebateUnits = @RebateUnits + @UnitsToDeductFromTerms
end
end
INSERT INTO [RB_RuleInvoice]
([RB_RuleId]
,[RB_RulePeriodId]
,[InvoiceKey]
,[IsAssociated]
,[ForcastData]
,[RebateUnits]
,[GrossAmount]
,[NetAmount]
,[Units]
,[DealerId]
,[RebateAmount]
,[RebateAmountPaid]
,[RebateAmountActual]
,[ReallocationAmount]
,[ReallocationAmountPaid]
,[ReallocationPaidFlag]
,[PostPeriodAdjustAmount]
,[TotalInvoiceNotPaid]
,[TotalReallocation]
,[PercentPayable])
values
(@RuleId,-1,@col1,1,0,@RebateUnits,@TotalGross,@TotalNet,@TotalUnits,@ShipToKey,0,0,0,0,0,0,0,0,0,0)
END
select * from rb_ruleinvoice
DEALLOCATE cur
May 30, 2011 at 7:38 am
It most certainly can be converted into a set based query (maybe using the divide'n'conquer approach).
We'd need table def and sample data for all tables involved in a ready to use format as decribed in the first link referenced in my signature.
I wonder if this code actually runs succesfully:
while 1=1
BEGIN
FETCH curMain INTO @TotalInvoiceAmount, @InvoiceSubTotal, @termsdiscountamount, @supplierkey,@ShipToKey
IF @@fetch_status <> 0
break
end
What exactly is this code snippet supposed to be doing? It seems like it's there to get the last values from the query that defined the *cough* c.u.r.s.o.r., but this query does not have an ORDER BY. So it's a random SELECT.
May 31, 2011 at 10:11 am
No kidding. The scary level of this is off the chart. There are 4 nested while 1=1 loops. Which in turn declares 4 nested cursors. :w00t: This is a textbook example of how bad cursors can get. As Lutz suggested post some consumable ddl and sample data and there will most likely be multiple versions for you to try and see how the performance changes.
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
May 31, 2011 at 10:24 am
All,
maybe if I provide some background information as to what I am trying to accomplish you can provide some hints --- if that does not pan out I will post the DDL and some sample data. I am just hesitant to do this as there are many tables involved.
The outer most loop "select invoicekey,rb_ruleid from rb_invoicestoprocess" is simply getting a list of the documents that need to be processed. The order does not need to matter as they all must be processed.
For each invoice that I have to deal with I need to do the following:
1. Calculate the total rebate units on the invoice at the line and tax level
select UnitsShipped,GrossAmount,NetAmount from InvoiceLineItem a,Invoice b where a.invoicekey = @col1 and a.invoicekey = b.invoicekey and a.ProductRebateCategory in (select c.categorynumber from RB_RuleCategory c where c.rb_ruleid = 8251 and c.SupplierId = B.supplierkey)
union
select 1,a.TaxAmount,a.TaxAmount from invoicetotaltax a,Invoice b where a.invoicekey = @col1 and a.invoicekey = b.invoicekey and a.ProductRebateCategory in (select c.categorynumber from RB_RuleCategory c where c.rb_ruleid = 8251 and c.SupplierId = B.supplierkey
2. Calculate the total rebate units in the additional charges table (you will note that there is code to manipulate the values in the loop)
select 1,Amount,Amount,Type as AmountType from InvoiceTotalAdditionalCosts a,Invoice b where
a.invoicekey = @col1
and a.invoicekey = b.invoicekey
and a.ProductRebateCategory in (
select c.categorynumber from RB_RuleCategory c where c.rb_ruleid = 8251
and c.SupplierId = B.supplierkey
)
3.Then I do some final total calculations
4. Finally I write a new record into a different table.
5. There is another step (not shown here) that removes the entry from the table to prevent the outer loop from processing it again.
I am just not sure with the different "if" conditions that determine how to calculate the totals that it can be collapsed in order to eliminate the need to use cursors.
The only way I could think to do it is to create a temporary table with the values and keep doing updates to the table to get my final totals -- this would allow me to do it "set based" vs cursor based..... not sure.
I have done some testing and I have proven that I can calculate the values on 100,000 invoices in approx. 2 minutes......
May 31, 2011 at 10:34 am
Well we are not familiar with your requirements and we can't see over you shoulder so there really isn't a lot we can do. One comment I would make about your select statement in #2 is that I find it easier to read queries using joins instead of listing the tables and then moving the logical join info to the where clause. I also find it a LOT easier to alias your tables with a super short version of the table name. This will help you across the whole system because you start thinking of long table names in terms of 3-4 character abbreviations. Otherwise you have to read through the whole query each time (and usually multiple times) before you can get it straight about which table is a, which table is b etc.
Yes I know it is a lot of work but if you want some real help with this we have to have something to work with. Given the complexity there is little chance that somebody is going to dedicate the couple hours it will take to create your tables with some possible sample data for you.
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
May 31, 2011 at 10:40 am
Here is what I mean about more legible. This is a rewrite of your query in #2
select 1, Amount, Amount, Type as AmountType
from InvoiceTotalAdditionalCosts itac
join Invoice i on itac.invoicekey = i.invoicekey
join RB_RuleCategory rc on itac.ProductRebateCategory = rc.categorynumber
and rc.SupplierId = i.supplierkey and rc.rb_ruleid = 8251
where a.invoicekey = @col1
Much easier to read. 🙂
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
May 31, 2011 at 10:46 am
It looks to me, from what I can see in the code, that the primary issue you are running into is a running total calculation that has to be performed with multiple partitions.
Does that accurately describe it?
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
May 31, 2011 at 10:49 am
I think you are correct -- although the word 'partition' is confusing me.
Basically I have a list of invoices that I need to calculate total values on -- these calculations cause me to run 3 subqueries and total the results of these queries to get my final answer for each invoice.
The trick is that it is not just a straight sum --- I need to do math on the results before I can calculate the total.
May 31, 2011 at 10:59 am
Sorry, "partition", with reference to a running total or other aggregate, simply means to break the data down into subsets. If, for example, you had multiple orders on a report, and you needed a running total of the line-items inside each order, you would partition it by the order number (or some other identifier for each order). A non-partitioned running total would just continue right on down the page, while a partitioned one would start over for each order. Clearer?
And, to simplify/confuse this more, I was wrong. That's not what you're doing.
It looks like a lot of your more complex, line-by-line math could be handled by inline queries or some relatively simple Case statements determining what math to run where.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
May 31, 2011 at 11:00 am
At a first glance I thought it's a running total problem, too. But I'm not sure anymore. It might be possible to do it all set based. But I'd need to see the table and data to be sure. There are so many loops it makes me feel being in the gift packing industry... 😛
May 31, 2011 at 11:01 am
Thanks....
Trying to generate test data -- this might take awhile...
May 31, 2011 at 11:17 am
Here's an initial take on it. It doesn't quite make sense, because the sub-queries don't seem to tie directly to the outer queries.
This version is only one step better than nested cursors, but it's a starting place. View it more as a place to start from than as a destination.
SELECT *
FROM rb_invoicestoprocess AS ToProcess
CROSS APPLY (SELECT TotalInvoiceAmount,
InvoiceSubTotal,
termsdiscountamount,
supplierkey,
shiptokey
FROM invoicetotal a,
invoice b
WHERE a.invoicekey = b.invoicekey
AND a.invoicekey = ToProcess.invoicekey) AS curMain
CROSS APPLY (SELECT SUM(unitsshipped) AS SumUnitsShipped,
SUM(grossamount) AS SumGrossAmount,
SUM(netamount) AS SumNetAmount
FROM (SELECT UnitsShipped,
GrossAmount,
NetAmount
FROM InvoiceLineItem a,
Invoice b
WHERE a.invoicekey = ToProcess.invoicekey
AND a.invoicekey = b.invoicekey
AND a.ProductRebateCategory IN (
SELECT c.categorynumber
FROM RB_RuleCategory c
WHERE c.rb_ruleid = 8251
AND c.SupplierId = B.supplierkey)
UNION
SELECT 1,
a.TaxAmount,
a.TaxAmount
FROM invoicetotaltax a,
Invoice b
WHERE a.invoicekey = ToProcess.invoicekey
AND a.invoicekey = b.invoicekey
AND a.ProductRebateCategory IN (
SELECT c.categorynumber
FROM RB_RuleCategory c
WHERE c.rb_ruleid = 8251
AND c.SupplierId = B.supplierkey))
AS SubTfTest) AS tftest
CROSS APPLY (SELECT 1,
Amount,
Amount,
Type AS AmountType
FROM InvoiceTotalAdditionalCosts a,
Invoice b
WHERE a.invoicekey = ToProcess.invoicekey
AND a.invoicekey = b.invoicekey
AND a.ProductRebateCategory IN (
SELECT c.categorynumber
FROM RB_RuleCategory c
WHERE c.rb_ruleid = 8251
AND c.SupplierId = B.supplierkey)) AS curCharges
It won't run as written, because the last Cross Apply needs column aliases. I couldn't quite tell from the code what those should be. In the other places, I tried to suggest them.
If you aren't familiar with Cross/Outer Apply, MSDN has a good article on it. It can often make cursor-based operations much more efficient.
Does that look like it'll get you started?
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
May 31, 2011 at 11:21 am
G Squared --
Thanks for the note -- that was what I was looking for -- not a solution, just something to look into.
I have never used a "cross apply" so I will go about doing the research....
Just out of curiosity --- if you had 100K rows that you wish to preform operations on -- how long would you expect it to take before you gave up tuning?
Currently this procedure runs in 2 minutes on 100,000 lines -- is it your opinion that I can significantly improve that?
Should I be expecting that it runs in a mtter of seconds\milliseconds?
May 31, 2011 at 11:25 am
I'm pretty accustomed to 100k row operations taking a few seconds. It depends on how complex it all is, but I would expect some simple aggregation and summing to be very fast.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
May 31, 2011 at 11:38 am
tfeuz (5/31/2011)
G Squared --Thanks for the note -- that was what I was looking for -- not a solution, just something to look into.
I have never used a "cross apply" so I will go about doing the research....
Just out of curiosity --- if you had 100K rows that you wish to preform operations on -- how long would you expect it to take before you gave up tuning?
Currently this procedure runs in 2 minutes on 100,000 lines -- is it your opinion that I can significantly improve that?
Should I be expecting that it runs in a mtter of seconds\milliseconds?
On far more complex reports selecting over 17 GB of data and doing a heck of a lot more calculations than this I get the whole thing done and rendered in a report on ssrs in that amount of time.
So 2 minutes for that "little" work seems excessive to me.
The server we have is very modest... single xeon processor, dual core. 4 GB ram, decent SAN. Nothing special.
Viewing 15 posts - 1 through 15 (of 117 total)
You must be logged in to reply to this topic. Login to reply