There Must Be 15 Ways To Lose Your Cursors… Part 2: Just Put It in a S

  • davec-640463 (6/30/2010)


    Hi MutlyP,

    It also sounds like table variables could help you out here. You could possibly subset your data into table variables perfoming some of the logic you describe. Data can then be returned from those table variables instead of the physical tables. I also like this approach as it can provide huge speed improvements. For example, if you are going to be querying a table with a million rows and performing some kind of complex calculation using the data from only 50 of the rows. It is far quicker (in my opinion) to extract those 50 rows into a table variable and work with them there. Your table variable can have whatever columns you want so you can insert your data and then update additional columns to have calculated values.

    You can also use Temp Tables (which also start out in memory and move to TempDB when they no longer fit) for such a thing. The advantage of Temp Tables over table variables is mostly in the form of statistics and maybe being a little easier to troubleshoot because they persist in SSMS where table variables don't in either case.

    Either way is better than a cursor, though. Just don't fall into the trap of creating a temporary structure and then running a While Loop on that... that's almost identical to what a Cursor does.

    --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)

  • Jeff Moden (6/30/2010)


    You can also use Temp Tables (which also start out in memory and move to TempDB when they no longer fit) for such a thing. The advantage of Temp Tables over table variables is mostly in the form of statistics and maybe being a little easier to troubleshoot because they persist in SSMS where table variables don't in either case.

    Hi Jeff,

    I did not know that temp tables started in memory. I thought that they were always created physically to disk in the TempDB. This would make a lot of sense for standard Temp Tables. Do global Temp Tables (denoted by ## rather than #) work in the same way? I would guess they do too. I usually use a table variable unless one of the following conditions is met:

    1. I need to refer to the table outside of the current scope

    2. I need to create an index on the table

    3. I need to use the table in an Exec statement (although I NEVER EVER do this unless absolutely necessary)

  • davec-640463 (6/30/2010)


    Jeff Moden (6/30/2010)


    You can also use Temp Tables (which also start out in memory and move to TempDB when they no longer fit) for such a thing. The advantage of Temp Tables over table variables is mostly in the form of statistics and maybe being a little easier to troubleshoot because they persist in SSMS where table variables don't in either case.

    Hi Jeff,

    I did not know that temp tables started in memory. I thought that they were always created physically to disk in the TempDB. This would make a lot of sense for standard Temp Tables. Do global Temp Tables (denoted by ## rather than #) work in the same way? I would guess they do too. I usually use a table variable unless one of the following conditions is met:

    1. I need to refer to the table outside of the current scope

    2. I need to create an index on the table

    3. I need to use the table in an Exec statement (although I NEVER EVER do this unless absolutely necessary)

    Yes... Table Variables and Temp Tables (global or not) all start out in memory and move to TempDB disk space only when there's not enough memory for the object. That also means that, contrary to popular belief, that Table Variables are NOT "memory only".

    Here's an older link which is still appropriate...

    http://support.microsoft.com/default.aspx?scid=kb;en-us;305977&Product=sql2k

    --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)

  • mutlyp (6/29/2010)But I do see your logic for this case but like I said there are a lot more data that I must modify, compare and then take actions all from that one employee record. In your logic you were able to do it for one piece of data. What from one row(record from the database) of the employee you must manipulate 8 different types of data.

    Something to keep in mind is it might be faster to deal with each of your 8 manipulations separately if they aren't directly related, rather than doing them row-by-row in a cursor. (And by better I mean faster and more efficient I/O and CPU wise.) If you need them to all either to complete or not you can wrap the whole section of code in a BEGIN TRANSACTION - COMMIT TRANSACTION block.

    It is fairly easy to look up a discount and apply the logic in the SELECT statement. As Mike C said if you can post the whole section of code there are a number of us that will help show you how to eliminate the cursor.

  • The last I heard on the topic of temp tables v table variables was that the server decides when it can use memory v serializing to disk. I only use table variables when I need temporary storage in a udf. They don't have very good statistics and can't be changed (now new indexes, etc.) after the fact. Plus the scope is so narrow. OTOH I've yet to find a good use for a global temp table. Whenever I've thought I could use a global temp table I've found a permanent table more usable. A global temp table is essentially a perm table in tempdb though. A lot of devs got in the habit of creating them using create table tempdb..temptable back in the day (I wouldn't recommend this though).

    Mike C

  • Hi

    Thank you again for all the responses. Been very helpfull.

    Here is the code you asked for:

    CREATE PROCEDURE dbo.usp_GetMonthlyFare

    AS

    DECLARE @Month int

    DECLARE @Week int

    DECLARE @Year int

    /*Which ever day the last Wednesday is determines which month this will be recorded in. This will only be run on Friday*/

    if datepart(m,dateadd(d,-2,getdate())) = datepart(m,getdate())

    select @Month = datepart(m,getdate())

    else

    select @Month = (datepart(m,getdate()) - 1)

    select @Week = datepart(ww,getdate())

    select @Year = datepart(yyyy,getdate())

    DECLARE CursorMonth CURSOR

    SCROLL

    KEYSET

    FOR

    SELECT SUM(dbo.RouteFareHistoryWeek.CostWSeatSubWFuel) AS MonthDeductions, SUM(dbo.RouteFareHistoryWeek.CostWSeatSub) AS CostWSeatSub,

    SUM(dbo.RouteFareHistoryWeek.FuelIncrease) AS FuelDeductions, SUM(dbo.RouteFareHistoryWeek.CostWSeatSub)

    + SUM(dbo.RouteFareHistoryWeek.FuelIncrease) - SUM(dbo.RouteFareHistoryWeek.CostWSeatSubWFuel) AS DifferenceMonthly,

    SUM(dbo.RouteFareHistoryWeek.Discount) AS Discount, dbo.RouteFareHistoryWeek.RouteID_FK

    FROM dbo.RouteFareHistoryWeek INNER JOIN

    dbo.Route ON dbo.RouteFareHistoryWeek.RouteID_FK = dbo.Route.RouteID_PK

    GROUP BY dbo.RouteFareHistoryWeek.WhichMonth, dbo.RouteFareHistoryWeek.RouteID_FK, dbo.Route.RouteStartDate, dbo.Route.RouteEndDate

    HAVING (dbo.RouteFareHistoryWeek.WhichMonth = @Month) AND (dbo.Route.RouteStartDate <= GETDATE()) AND (dbo.Route.RouteEndDate IS NULL OR

    dbo.Route.RouteEndDate > GETDATE())

    DECLARE @MonthDeductions float

    DECLARE @CostWSeatSub float

    DECLARE @FuelDeductions float

    DECLARE @DifferenceMonthly float

    DECLARE @discount float

    DECLARE @RouteID varchar(50)

    OPEN CursorMonth

    FETCH NEXT FROM CursorMonth INTO @MonthDeductions , @CostWSeatSub, @FuelDeductions, @DifferenceMonthly, @discount, @RouteID

    WHILE @@FETCH_STATUS=0

    BEGIN

    INSERT INTO [RouteFareHistoryMonth]

    (

    [RouteID_FK],

    [FriDate],

    [MonthDeductions],

    [CostWSeatSub],

    [FuelDeductions],

    [DifferenceMonthly],

    [WhichMonth],

    [WhichYear],

    [WhichWeek],

    [Discount]

    )

    VALUES

    (

    @RouteID,

    GetDate(),

    @MonthDeductions,

    @CostWSeatSub,

    @FuelDeductions,

    @DifferenceMonthly,

    @Month,

    @Year,

    @Week,

    @discount

    )

    FETCH NEXT FROM CursorMonth INTO @MonthDeductions , @CostWSeatSub, @FuelDeductions, @DifferenceMonthly, @discount, @RouteID

    end

    CLOSE CursorMonth

    DEALLOCATE CursorMonth

    GO

    And here is another more complex one:

    ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

    CREATE PROCEDURE [DBO].[usp_GetWeeklyFare]

    AS

    DECLARE @WFS real

    DECLARE @SeatExp int

    DECLARE @Month int

    DECLARE @Week int

    DECLARE @Year int

    DECLARE @Dis float

    DECLARE @howmany int

    DECLARE @CostWSeatSub real

    DECLARE @CostWOSeatSub real

    DECLARE @FuelIncrease real

    DECLARE @CostWSeatSubWFuel real

    DECLARE @BaseCostPerRiderWSubWFuel real

    DECLARE @TodaysDate smalldatetime

    DECLARE @NumberPassengers int

    /* SETS @TodaysDate WITH TODAYS DATE*/

    set @TodaysDate = getdate()

    --set @TodaysDate = '3/4/2008'

    /* CALLS usp_WhichMonth TO FIGURE OUT WHICH MONTH THE WEEK IS IN*/

    exec usp_WhichMonth @TodaysDate, @Month OutPut

    /* SETS @Week WITH TODAYS WEEK PART*/

    set @Week = datepart(ww,getdate())

    /* SETS @Year WITH TODAYS YEAR PART*/

    set @Year = datepart(yyyy,getdate())

    if datepart(dw, getdate()) = 3

    set @TodaysDate = CONVERT(varchar, dateadd(d, 3, getdate()), 101)

    if datepart(dw, getdate()) = 4

    set @TodaysDate = CONVERT(varchar, dateadd(d, 2, getdate()), 101)

    if datepart(dw, getdate()) = 5

    set @TodaysDate = CONVERT(varchar, dateadd(d, 1, getdate()), 101)

    if datepart(dw, getdate()) = 6

    set @TodaysDate = CONVERT(varchar, getdate(), 101)

    if datepart(dw, getdate()) = 7

    set @TodaysDate = CONVERT(varchar, dateadd(d, -1, getdate()), 101)

    set @TodaysDate = cast(@TodaysDate as datetime)

    /*declare @Num as int

    exec @Num = usp_RouteFareHistoryWeek_FriDate_Check1 @TodaysDate*/

    /* DECLARE CursorTee AS THE CURSOR THAT IS GOING TO LOOP THROUGH THE FOLLOWING SELECT STATEMENT*/

    DECLARE CursorTee CURSOR

    SCROLL

    KEYSET

    FOR

    /* Gets Fare info Plus gets Fuel Increase Per Week as GasIncrease */

    SELECT dbo.Route.RouteID_PK, dbo.Route.Discount, dbo.Route.RoundTripMileage, dbo.Van.MPG, dbo.Van.NumSeats, dbo.GasPrice.BasePrice,

    dbo.Route.NumPassengers, dbo.Route.FareStructID_FK, dbo.Route.Seat2ExpireDate,

    (((((dbo.Route.RoundTripMileage * 5) / CAST(dbo.Van.MPG AS real)) * dbo.GasPrice.AveragePrice) / CAST(dbo.Van.NumSeats AS real))

    * (dbo.Van.NumSeats - 1)) AS GasIncrease,

    dbo.GasPrice.AveragePrice, dbo.Route.FourSeatSub

    FROM dbo.Route INNER JOIN

    dbo.Van ON dbo.Route.RouteID_PK = dbo.Van.RouteID_FK CROSS JOIN

    dbo.GasPrice

    WHERE (dbo.Route.RouteEndDate IS NULL OR

    dbo.Route.RouteEndDate > GETDATE()) AND (dbo.GasPrice.DateChanged IS NULL) AND (dbo.GasPrice.WhichMonth = @Month) AND

    (dbo.GasPrice.WhichYear = @Year) AND (dbo.Van.StartDate <= GETDATE()) AND (dbo.Van.ReturnDate IS NULL OR

    dbo.Van.ReturnDate > GETDATE()) AND (dbo.Route.RouteStartDate <= GETDATE())

    DECLARE @Route varchar(50)

    DECLARE @discount real

    DECLARE @RTM int

    DECLARE @MPG int

    DECLARE @NumSeats int

    DECLARE @BasePrice real

    DECLARE @NumPass int

    DECLARE @FareStruc int

    DECLARE @2SeatExpire smalldatetime

    DECLARE @GasIncrease real

    DECLARE @AvgGasPrice real

    DECLARE @FourSeatSub varchar(10)

    OPEN CursorTee

    /* FETCHES THE FIRST RECORD OF THE SELECT STATMENT*/

    FETCH NEXT FROM CursorTee INTO @Route, @discount, @RTM, @MPG, @NumSeats, @BasePrice, @NumPass, @FareStruc, @2SeatExpire, @GasIncrease, @AvgGasPrice, @FourSeatSub

    WHILE @@FETCH_STATUS=0

    BEGIN

    set @Dis = 0

    /* Determine what this months Gas Increse is with this formula

    /*set @GasIncrease = Round((((((@RTM * 5) / @MPG)* @AvgGasPrice) / @NumSeats) * (@NumSeats - 1)),2) */

    set @GasIncrease = Round(@GasIncrease, 2)

    /* CHECKS THE Discout FIELD FROM THE RECORD SET TO SEE IF THIS PARTICULAR ROUTE GETS A ROUTE DISCOUNT*/

    if @discount IS NOT NULL

    begin

    /* CALLS usp_HowManyWeeksInMonth TO GET THE NUMBER OF WEEKS IN THE MONTH IN QUESTION */

    exec @howmany = usp_HowManyWeeksInMonth

    /* DEVIDE THE NUMBER OF WEEKS WITH THE ROUTE DISCOUNT TO FIGURE OUT HOW MUCH OF A DISCOUNT THIS ROUTE GETS THIS WEEK */

    if @howmany = 5

    begin

    set @Dis = @discount / 5

    end

    else begin

    set @Dis = @discount / 4

    end

    if @Month = 12

    begin

    set @Dis = @discount / 4

    end

    end

    /*select @GasIncrease = (((((@RTM * 5)/(@MPG))* @BasePrice)/@NumSeats)*(@NumSeats - 1))*/

    /* TAKES THE NuSeats AND FareSructID_FK FROM THE RECORD SET TO GET THE FARE STRUCTURE FOR THIS PARTICULAR ROUTE */

    if @NumSeats = '9'

    begin

    set @WFS = (SELECT NinePassVan FROM FareStructure WHERE (RoundTripMileageLow <= @RTM) AND (RoundTripMileageHigh >= @RTM))

    end

    if @NumSeats = '10'

    begin

    set @WFS = (SELECT TenPassVan FROM FareStructure WHERE (RoundTripMileageLow <= @RTM) AND (RoundTripMileageHigh >= @RTM))

    /*set @WFS = (SELECT TenPassVan FROM FareStructure WHERE FareStructID_PK = @FareStruc)*/

    end

    if @NumSeats = '12'

    begin

    set @WFS = (SELECT TwelvePassVan FROM FareStructure WHERE (RoundTripMileageLow <= @RTM) AND (RoundTripMileageHigh >= @RTM))

    end

    if @NumSeats = '14'

    begin

    set @WFS = (SELECT FourteenPassVan FROM FareStructure WHERE (RoundTripMileageLow <= @RTM) AND (RoundTripMileageHigh >= @RTM))

    /*set @WFS = (SELECT FourteenPassVan FROM FareStructure WHERE FareStructID_PK = @FareStruc)*/

    end

    set @NumberPassengers = (SELECT COUNT(RiderID_PK) AS NumRiders FROM dbo.Rider WHERE (StartDate <= @TodaysDate) GROUP BY RouteID_FK, Active HAVING (RouteID_FK = @Route) AND (Active = 1))

    /* CHECKS TO SEE IF THE 2 SEAT SUBSIDY HAS EXPIRED */

    if getdate() <= @2SeatExpire

    begin /* IF SUBSIDY DATE HAS NOT EXPIRED SET THE SEAT SUBSIDY TO 2 ELSE IT IS THE NORMAL 1 */

    if @FourSeatSub = 1

    begin

    set @SeatExp = 4

    end

    else

    begin

    if @NumSeats = @NumberPassengers

    begin

    set @SeatExp = 1

    end

    else

    begin

    set @SeatExp = 2

    end

    end

    end

    else

    begin

    if @FourSeatSub = 1

    begin

    set @SeatExp = 2

    end

    else

    begin

    set @SeatExp = 1

    end

    end

    /* COST W/1 SEAT SUBSIDY PER WEEK*/

    /* WEEKLY FARE STRUCTURE MULTIPLIED BY NUMBER OF SEATS IN VAN MINUS THE SEAT SUBSIDY*/

    set @CostWSeatSub = ROUND(CAST((@WFS * (@NumSeats - @SeatExp)) AS real),2)

    /*Cost Without Subsudy*/

    /* WEEKLY FARE STRUCTURE MULTIPLIED BY NUMBER OF SEATS IN VAN */

    set @CostWOSeatSub = ROUND(CAST((@WFS * @NumSeats) AS real),2)

    /*Fuel Increase*//* GasIncrease FROM THE RECORD SET ROUNDED TO 2 DECIMAL PLACES*/

    set @FuelIncrease = ROUND(CAST(@GasIncrease AS real),2)

    /* COST W/1 SEAT SUBSIDY PER WEEK PLUS THE FUEL INCREASE PER WEEK MINUS THE ROUTE DISCOUNT IF ANY*/

    set @CostWSeatSubWFuel = (@CostWSeatSub + @FuelIncrease) - @Dis

    /*SELECT @BaseCostPerRiderWSubWFuel = ROUND(((((@WFS * (@NumSeats - 1))) + @GasIncrease)/(CAST((@NumPass - 1) AS FLOAT(50)))),2)*/

    /*Base Cost Per Rider W/1 Seat Subsidy Including Fuel*/

    /* COST W/1 SEAT SUBSIDY PER WEEK INCLUDING FUEL DEVIDED BY THE NUMBER OF SEATS ON THE VAN MINUS THE SEAT SUBSIDY */

    if @NumberPassengers < 2

    begin

    set @BaseCostPerRiderWSubWFuel = @CostWSeatSubWFuel

    end

    else

    begin

    set @BaseCostPerRiderWSubWFuel = @CostWSeatSubWFuel /(@NumberPassengers -1)

    end

    IF EXISTS (

    SELECT

    RouteID_FK

    FROM

    RouteFareHistoryWeek

    WHERE

    FriDate = @TodaysDate

    AND

    RouteID_FK = @Route

    )

    UPDATE [RouteFareHistoryWeek]

    SET

    [CostWSeatSub] = @CostWSeatSub,

    [CostWOSeatSub] = @CostWOSeatSub,

    [FuelIncrease] = @FuelIncrease,

    [CostWSeatSubWFuel] = @CostWSeatSubWFuel,

    [BaseCostPerRiderWSubWFuel] = @BaseCostPerRiderWSubWFuel,

    [RouteID_FK] = @Route,

    [WhichMonth] = @Month,

    [WhichWeek] = @Week,

    [WhichYear] = @Year,

    [WFS] = @WFS,

    [NumSeat] = @NumSeats,

    [SeatExp] = @SeatExp,

    [Seat2ExpireDate] = @2SeatExpire,

    [Discount] = @Dis

    WHERE

    [FriDate] = @TodaysDate and [RouteID_FK] = @Route

    else

    INSERT INTO [RouteFareHistoryWeek]

    (

    [FriDate],

    [CostWSeatSub],

    [CostWOSeatSub],

    [FuelIncrease],

    [CostWSeatSubWFuel],

    [BaseCostPerRiderWSubWFuel],

    [RouteID_FK],

    [WhichMonth],

    [WhichWeek],

    [WhichYear],

    [WFS],

    [NumSeat],

    [SeatExp],

    [Seat2ExpireDate],

    [Discount]

    )

    VALUES

    (

    @TodaysDate,

    @CostWSeatSub,

    @CostWOSeatSub,

    @FuelIncrease,

    @CostWSeatSubWFuel,

    @BaseCostPerRiderWSubWFuel,

    @Route,

    @Month,

    @Week,

    @Year,

    @WFS,

    @NumSeats,

    @SeatExp,

    @2SeatExpire,

    @Dis

    )

    select @Dis = ' '

    FETCH NEXT FROM CursorTee INTO @Route, @discount,@RTM, @MPG, @NumSeats, @BasePrice, @NumPass, @FareStruc, @2SeatExpire, @GasIncrease, @AvgGasPrice, @FourSeatSub

    end

    CLOSE CursorTee

    DEALLOCATE CursorTee

    GO

    I think the code is understandable. Let me know if you need any clarification. Understand if it's too much but you did ask for it :-).

    I really do appreciate the advice.

    MutlyP

  • mutlyp (6/30/2010)


    Hi

    Thank you again for all the responses. Been very helpful.

    Here is the code you asked for:

    I'll take the first one as it is simple. 🙂

    In this case it looks like you are just creating a cursor to loop through and insert the data. This is best done using the INSERT INTO <table> SELECT statement like:

    CREATE PROCEDURE dbo.usp_GetMonthlyFare

    AS

    DECLARE @Month INT;

    IF DATEPART(m, DATEADD(d, -2, GETDATE())) = DATEPART(m, GETDATE())

    SET @Month = DATEPART(m, GETDATE())

    ELSE

    SET @Month = (DATEPART(m, GETDATE()) - 1);

    INSERT INTO [RouteFareHistoryMonth]

    ([RouteID_FK],

    [FriDate],

    [MonthDeductions],

    [CostWSeatSub],

    [FuelDeductions],

    [DifferenceMonthly],

    [WhichMonth],

    [WhichYear],

    [WhichWeek],

    [Discount]

    )

    SELECT

    dbo.RouteFareHistoryWeek.RouteID_FK,

    GETDATE() AS FriDate,

    SUM(dbo.RouteFareHistoryWeek.CostWSeatSubWFuel) AS MonthDeductions,

    SUM(dbo.RouteFareHistoryWeek.CostWSeatSub) AS CostWSeatSub,

    SUM(dbo.RouteFareHistoryWeek.FuelIncrease) AS FuelDeductions,

    SUM(dbo.RouteFareHistoryWeek.CostWSeatSub)

    + SUM(dbo.RouteFareHistoryWeek.FuelIncrease)

    - SUM(dbo.RouteFareHistoryWeek.CostWSeatSubWFuel) AS DifferenceMonthly,

    @Month as WhichMonth,

    DATEPART(yyyy, GETDATE()) AS WhichYear,

    DATEPART(ww, GETDATE()) AS WhichWeek,

    SUM(dbo.RouteFareHistoryWeek.Discount) AS Discount

    FROM dbo.RouteFareHistoryWeek

    INNER JOIN dbo.Route

    ON dbo.RouteFareHistoryWeek.RouteID_FK = dbo.Route.RouteID_PK

    GROUP BY

    dbo.RouteFareHistoryWeek.WhichMonth,

    dbo.RouteFareHistoryWeek.RouteID_FK,

    dbo.Route.RouteStartDate,

    dbo.Route.RouteEndDate

    HAVING

    (dbo.RouteFareHistoryWeek.WhichMonth = @Month)

    AND (dbo.Route.RouteStartDate <= GETDATE())

    AND (dbo.Route.RouteEndDate IS NULL

    OR dbo.Route.RouteEndDate > GETDATE()

    );

    GO

    As with anything you get from the internet you should check the results before putting it in production.

    As for your second example, it might help to explain what you want done in it. It appears that there is missing comment close token "*/" at about the usp_HowManyWeeksInMonth call, but I can't be sure. Also, does the usp_HowManyWeeksInMonth SP take parameters, or where is it getting the month/date from to return the value? (Does it just look at the current month?)

  • MutlyP,

    What version of SQL Server are you using? (If you are using 2008 the second procedure can be written a little different.)

    In your second example looking at the first portion of your code I have a question:

    SET @TodaysDate = GETDATE()

    --set @TodaysDate = '3/4/2008'

    /* CALLS usp_WhichMonth TO FIGURE OUT WHICH MONTH THE WEEK IS IN*/

    EXEC usp_WhichMonth @TodaysDate, @Month OUTPUT

    /* SETS @Week WITH TODAYS WEEK PART*/

    SET @Week = DATEPART(ww, GETDATE())

    /* SETS @Year WITH TODAYS YEAR PART*/

    SET @Year = DATEPART(yyyy, GETDATE())

    IF DATEPART(dw, GETDATE()) = 3

    SET @TodaysDate = CONVERT(VARCHAR, DATEADD(d, 3, GETDATE()), 101)

    IF DATEPART(dw, GETDATE()) = 4

    SET @TodaysDate = CONVERT(VARCHAR, DATEADD(d, 2, GETDATE()), 101)

    IF DATEPART(dw, GETDATE()) = 5

    SET @TodaysDate = CONVERT(VARCHAR, DATEADD(d, 1, GETDATE()), 101)

    IF DATEPART(dw, GETDATE()) = 6

    SET @TodaysDate = CONVERT(VARCHAR, GETDATE(), 101)

    IF DATEPART(dw, GETDATE()) = 7

    SET @TodaysDate = CONVERT(VARCHAR, DATEADD(d, -1, GETDATE()), 101)

    SET @TodaysDate = CAST(@TodaysDate AS DATETIME)

    It looks like you are trying to force the date to Friday, but you don't account for Sunday and Monday is that on purpose? If not this can simplify that section:

    -- Return just the date portion of the current date/time

    SET @TodaysDate = DATEADD(Day, 0, DATEDIFF(Day, 0, GETDATE()));

    --set @TodaysDate = '3/4/2008'

    /* CALLS usp_WhichMonth TO FIGURE OUT WHICH MONTH THE WEEK IS IN*/

    EXEC usp_WhichMonth @TodaysDate, @Month OUTPUT

    /* SETS @Week WITH TODAYS WEEK PART*/

    SET @Week = DATEPART(ww, GETDATE())

    /* SETS @Year WITH TODAYS YEAR PART*/

    SET @Year = DATEPART(yyyy, GETDATE())

    -- Make sure the date is the Friday of the week

    SET @TodaysDate = DATEADD(DAY, 6 - DATEPART(DW, @TodaysDate), @TodaysDate);

    If you want it to allow the Sunday and Monday dates to go through unchanged that can be accomplished with a simple IF.

  • Hi mutlyp,

    I took a shot at rewriting this in cursor-less form. There are some things to keep in mind:

    * You'll probably need to review some of the logic (I've added comments). Some of it wasn't completely clear, and some mismatched block comments appeared to comment out parts that maybe shouldn't have been in your original code. Also there were references to stored procedures with some logic that I didn't have access to. You'll need to test.

    * I didn't run the code locally since I don't have all your tables and sample data to run with. You will probably need to fix a few things up (syntax, etc.) here and there.

    * You'll notice I used a lot of CTEs (common table expressions). I think they make the code more readable, but they should also help with following the logic flow.

    * I created an auxiliary calendar table for you. Take a look, it comes in handy for date-based calculations (like determining the last Wednesday).

    * I slightly modified some logic (like determining the last Wednesday). The theory is how do you know the code is being run on Friday with 100% certainty? It might have not run on Friday because of some error, and have to be run on the following Monday. In either case, your code is based off the last Wednesday. If you prefer it should be easy to change that logic back.

    * You may be able to get some additional performance by playing with indexes, etc., as well. I didn't bother since I can't run the code over here right now.

    CREATE TABLE dbo.Calendar

    (

    Date DATETIME NOT NULL,

    IsWeekday BIT NOT NULL,

    IsHoliday BIT NOT NULL,

    Year INT,

    FiscalYear INT,

    Quarter INT,

    Month INT,

    Day INT,

    DayOfWeek INT,

    MonthName VARCHAR(9),

    DayName VARCHAR(9),

    Week INT,

    ISOWeek INT,

    CONSTRAINT PK_Calendar PRIMARY KEY

    (

    Date

    )

    )

    GO

    WITH CTE

    AS

    (

    SELECT CAST('20000101' AS DATETIME) AS Date

    UNION ALL

    SELECT DATEADD(DAY, 1, Date)

    FROM CTE

    WHERE Date < '20500101'

    )

    INSERT INTO dbo.Calendar

    (

    Date,

    IsWeekday,

    IsHoliday,

    Year,

    FiscalYear,

    Quarter,

    Month,

    Day,

    DayOfWeek,

    MonthName,

    DayName,

    Week,

    ISOWeek

    )

    SELECT Date,

    CASE DATENAME(DW, Date)

    WHEN 'Saturday' THEN 0

    WHEN 'Sunday' THEN 0

    ELSE 1

    END,

    0, -- Default to 0 for now; set later if necessary

    DATEPART(YYYY, Date),

    CASE WHEN DATEPART(MM, Date) < 10-- This assumes Fiscal Year is October to September. Change if necessary

    THEN DATEPART(YYYY, Date) - 1

    ELSE DATEPART(YYYY, Date)

    END,

    DATEPART(QQ, Date),

    DATEPART(MM, Date),

    DATEPART(DD, Date),

    DATEPART(DW, Date),

    DATENAME(MM, Date),

    DATENAME(DW, Date),

    DATEPART(WW, Date),

    DATEPART(ISOWW, Date)

    FROM CTE

    OPTION (MAXRECURSION 0);

    GO

    CREATE PROCEDURE dbo.usp_GetMonthlyFare

    AS

    BEGIN

    DECLARE @Month INT;

    DECLARE @Week INT;

    DECLARE @Year INT;

    DECLARE @WedDate DATETIME;

    /*Which ever day the last Wednesday is determines which month this will be recorded in. This will only be run on Friday*/

    -- What if it's not run on Friday for some reason?

    -- What about Year and Week? Should those be based on the last Wednesday also, or not?

    SELECT @Month = Month,

    @Week = Week,

    @Year = Year,

    @WedDate = Date

    FROM dbo.Calendar

    WHERE Date =

    (

    SELECT MAX(Date)

    FROM dbo.Calendar

    WHERE DayName = 'Wednesday'

    AND Date < GETDATE()

    );

    -- Do it all in one INSERT, no cursor

    INSERT INTO dbo.RouteFareHistoryMonth

    (

    RouteID_FK,

    FriDate,

    MonthDeductions,

    CostWSeatSub,

    FuelDeductions,

    DifferenceMonthly,

    WhichMonth,

    WhichYear,

    WhichWeek,

    Discount

    )

    SELECT rfhw.RouteID_FK,

    DATEADD(DAY, 2, @WedDate) AS FriDate,

    SUM(rfhw.CostWSeatSubWFuel) AS MonthDeductions,

    SUM(rfhw.CostWSeatSub) AS CostWSeatSub,

    SUM(rfhw.FuelIncrease) AS FuelDeductions,

    SUM(rfhw.CostWSeatSub) + SUM(rfhw.FuelIncrease) - SUM(rfhw.CostWSeatSubWFuel) AS DifferenceMonthly,

    SUM(rfhw.Discount) AS Discount

    FROM dbo.RouteFareHistoryWeek rfhw

    INNER JOIN dbo.Route r

    ON rfhw.RouteID_FK = r.RouteID_PK

    GROUP BY rfhw.WhichMonth,

    rfhw.RouteID_FK,

    r.RouteStartDate,

    r.RouteEndDate

    HAVING (rfhw.WhichMonth = @Month)

    AND (r.RouteStartDate <= GETDATE())

    AND (r.RouteEndDate IS NULL

    OR r.RouteEndDate > GETDATE());

    END;

    GO

    CREATE PROCEDURE dbo.usp_GetWeeklyFare

    AS

    BEGIN

    DECLARE @TodaysDate DATETIME,

    @Month INT,

    @Year INT,

    @Week INT,

    @Offset INT;

    -- Are you sure you don't want to back into it from the previous Wednesday? What if today is not Friday?

    -- Start with today's date

    SET @Week = Week,

    @Year = Year,

    @Offset = 6 - DayOfWeek,

    @TodaysDate = DATEADD(DAY, @Offset, Date)

    FROM dbo.Calendar

    WHERE Date = DATEDIFF(DAY, 0, GETDATE());

    -- Use usp_WhichMonth to figure out which month the week is in? What kind of logic is in this thing I wonder?

    EXEC dbo.usp_WhichMonth @TodaysDate, @Month OUTPUT;

    WITH CTE

    AS

    (

    SELECT r.RouteID_PK AS Route,

    r.Discount,

    r.RoundTripMileage AS RTM,

    v.MPG,

    v.NumSeats,

    gp.BasePrice,

    r.NumPassengers AS NumPass,

    r.FareStructID_FK AS FareStruc,

    r.Seat2ExpireDate AS TwoSeatExpire,

    ROUND((((((r.RoundTripMileage * 5) / CAST(r.MPG AS REAL)) * gp.AveragePrice)

    / CAST(v.NumSeats AS REAL)) * (v.NumSeats - 1)), 2) AS GasIncrease,

    gp.AveragePrice AS AvgGasPrice,

    r.FourSeatSub,

    (

    -- Don't know how you calculated the "# of weeks in a month"???

    SELECT MAX(c.Week)

    FROM dbo.Calendar c

    WHERE c.Year = @Year

    AND c.Month = @Month

    ) AS HowMany

    FROM dbo.Route r

    INNER JOIN dbo.Van v

    ON r.RouteID_PK = v.RouteID_FK

    CROSS JOIN dbo.GasPrice gp

    WHERE (r.RouteEndDate IS NULL

    OR r.RouteEndDate > GETDATE())

    AND (gp.DateChanged IS NULL)

    AND (gp.WhichMonth = @Month)

    AND (gp.WhichYear = @Year)

    AND (v.StartDate <= GETDATE())

    AND (v.ReturnDate IS NULL

    OR v.ReturnDate > GETDATE())

    AND (r.RouteStartDate <= GETDATE())

    ),

    CTE2

    AS

    (

    SELECT c.[Route],

    c.Discount,

    c.RTM,

    c.MPG,

    c.NumSeats,

    c.BasePrice,

    c.NumPass,

    c.FareStruc,

    c.TwoSeatExpire,

    c.GasIncrease,

    c.AvgGasPrice,

    c.FourSeatSub,

    COALESCE

    (

    c.Discount / (

    CASE WHEN c.HowMany = 5 THEN 5.0

    ELSE 4.0

    END

    ) /(

    CASE WHEN @Month = 12 THEN 4.0

    ELSE 1.0

    END

    ), 0.0

    ) AS Dis,

    CASE c.NumSeats

    WHEN '9' THEN

    (-- Assumption here is that there are no overlapping ranges!

    SELECT fs.NinePassVan

    FROM dbo.FareStructure fs

    WHERE fs.RoundTripMileageLow <= c.RTM

    AND fs.RoundTripMileageHigh >= c.RTM

    )

    WHEN '10' THEN

    (

    SELECT fs.TenPassVan

    FROM dbo.FareStructure fs

    WHERE fs.RoundTripMileageLow <= c.RTM

    AND fs.RoundTripMileageHigh >= c.RTM

    )

    WHEN '12' THEN

    (

    SELECT fs.TwelvePassVan

    FROM dbo.FareStructure fs

    WHERE fs.RoundTripMileageLow <= c.RTM

    AND fs.RoundTripMileageHigh >= c.RTM

    )

    WHEN '14' THEN

    (

    SELECT fs.FourteenPassVan

    FROM dbo.FareStructure fs

    WHERE fs.RoundTripMileageLow <= c.RTM

    AND fs.RoundTripMileageHigh >= c.RTM

    )

    END AS WFS,

    (

    SELECT COUNT(r.RiderID_PK)

    FROM dbo.Rider r

    WHERE r.StartDate <= @TodaysDate

    GROUP BY r.RouteID_FK,

    r.Active

    HAVING r.RouteID_FK = c.[Route]

    AND r.Active = 1

    ) AS NumberPassengers

    FROM CTE c

    ),

    CTE3

    AS

    (

    SELECT c.[Route],

    c.Discount,

    c.RTM,

    c.MPG,

    c.NumSeats,

    c.BasePrice,

    c.NumPass,

    c.FareStruc,

    c.TwoSeatExpire,

    c.GasIncrease,

    c.AvgGasPrice,

    c.FourSeatSub,

    c.Dis,

    c.WFS,

    c.NumberPassengers,

    CASE WHEN GETDATE() <= c.TwoSeatExpire THEN

    CASE WHEN c.FourSeatSub = 1 THEN 4

    ELSE CASE WHEN c.NumSeats = c.NumberPassengers THEN 1

    ELSE 2

    END

    END

    ELSE CASE WHEN c.FourSeatSub = 1 THEN 2

    ELSE 1

    END AS SeatExp,

    ROUND(c.WFS * c.NumSeats, 2) AS CostWOSeatSub,

    ROUND(c.GasIncrease, 2) AS FuelIncrease

    FROM CTE2 c

    )

    SELECT c.[Route],

    c.Discount,

    c.RTM,

    c.MPG,

    c.NumSeats,

    c.BasePrice,

    c.NumPass,

    c.FareStruc,

    c.TwoSeatExpire,

    c.GasIncrease,

    c.AvgGasPrice,

    c.FourSeatSub,

    c.Dis,

    c.WFS,

    c.NumberPassengers,

    c.SeatExp,

    c.CostWOSeatSub,

    c.FuelIncrease,

    ROUND(c.WFS * (c.NumSeats - c.SeatExp), 2) AS CostWSeatSub,

    (c.CostWSeatSub + c.FuelIncrease) - c.Dis AS CostWSeatSubWFuel,

    ((c.CostWSeatSub + c.FuelIncrease) - c.Dis) /

    CASE WHEN c.NumberPassengers < 2

    THEN 1.0

    ELSE (c.NumPassengers - 1)

    END AS BaseCostPerRiderWSubWFuel

    INTO #Temp

    FROM CTE3 c;

    UPDATE dbo.RouteFareHistoryWeek

    SETCostWSeatSub = t.CostWSeatSub,

    CostWOSeatSub = t.CostWOSeatSub,

    FuelIncrease = t.FuelIncrease,

    CostWSeatSubWFuel = t.CostWSeatSubWFuel,

    BaseCostPerRiderWSubWFuel = t.BaseCostPerRiderWSubWFuel,

    RouteID_FK = t.[Route],

    WhichMonth = t.Month,

    WhichWeek = t.Week,

    WhichYear = t.Year,

    WFS = t.WFS,

    NumSeat = t.NumSeats,

    SeatExp = t.SeatExp,

    Seat2ExpireDate = t.TwoSeatExpire,

    Discount = t.Dis

    FROM #Temp t

    WHERE dbo.RouteFareHistoryWeek.FriDate = @TodaysDate

    AND dbo.RouteFareHistoryWeek.RouteID_FK = t.[Route]

    INSERT INTO dbo.RouteFareHistoryWeek

    (

    [FriDate],

    [CostWSeatSub],

    [CostWOSeatSub],

    [FuelIncrease],

    [CostWSeatSubWFuel],

    [BaseCostPerRiderWSubWFuel],

    [RouteID_FK],

    [WhichMonth],

    [WhichWeek],

    [WhichYear],

    [WFS],

    [NumSeat],

    [SeatExp],

    [Seat2ExpireDate],

    [Discount]

    )

    SELECT t.TodaysDate,

    t.CostWSeatSub,

    t.CostWOSeatSub,

    t.FuelIncrease,

    t.CostWSeatSubWFuel,

    t.BaseCostPerRiderWSubWFuel,

    t.[Route],

    t.Month,

    t.Week,

    t.Year,

    t.WFS,

    t.NumSeats,

    t.SeatExp,

    t.TwoSeatExpire,

    t.Dis

    FROM #Temp t

    WHERE NOT EXISTS

    (

    SELECT 1

    FROM dbo.RouteFareHistoryWeek rfhw

    WHERE rfhw.FriDate = @TodaysDate

    AND rfhw.RouteID_FK = t.[Route]

    );

    END;

    GO

    Mike C

  • A few things I noticed about Mike C's code:

    * Since he is using CTEs it will only run on SQL Server 2005 and up.

    * His week number is the week of the year, not the week of the month. (If you change the "MAX(c.Week)" to a "COUNT(c.Week)" you should be good, depending on what your SP does.) I choose to continue using your SP since it only needs to be calculated once for the whole run.

    * He took some liberties in assuming that tables are in the DBO schema and added it, I left the calls the same as you had them just in case.

    Here is my version of your second SP that will work with SQL Server 2000, just in case you don't have at least SQL Server 2005.

    CREATE PROCEDURE [DBO].[usp_GetWeeklyFare]

    AS

    DECLARE @TodaysDate SMALLDATETIME;

    DECLARE @Month INT;

    DECLARE @Week INT;

    DECLARE @Year INT;

    DECLARE @howmany INT;

    DECLARE @NumWeeks INT;

    -- Return just the date portion of the current date/time

    SET @TodaysDate = DATEADD(Day, 0, DATEDIFF(Day, 0, GETDATE()));

    /* CALLS usp_WhichMonth TO FIGURE OUT WHICH MONTH THE WEEK IS IN*/

    EXEC usp_WhichMonth @TodaysDate, @Month OUTPUT

    /* SETS @Week WITH TODAYS WEEK PART*/

    SET @Week = DATEPART(ww, GETDATE());

    /* SETS @Year WITH TODAYS YEAR PART*/

    SET @Year = DATEPART(yyyy, GETDATE());

    -- Make sure the date is the Friday of the week

    SET @TodaysDate = DATEADD(DAY, 6 - DATEPART(DW, @TodaysDate), @TodaysDate);

    EXEC @howmany = usp_HowManyWeeksInMonth;

    /* Figure out how many weeks to divide the discount by */

    IF @howmany = 5 AND @Month <> 12

    SET @NumWeeks = 5

    ELSE

    SET @NumWeeks = 4;

    /* Gets Fare info Plus gets Fuel Increase Per Week as GasIncrease */

    SELECT

    dbo.Route.RouteID_PK,

    COALESCE(CAST(dbo.Route.Discount AS FLOAT) / @NumWeeks, 0.0) AS DIS,

    dbo.Route.RoundTripMileage,

    dbo.Van.MPG,

    dbo.Van.NumSeats,

    dbo.GasPrice.BasePrice,

    dbo.Route.NumPassengers,

    dbo.Route.FareStructID_FK,

    dbo.Route.Seat2ExpireDate,

    ROUND(CAST((((((dbo.Route.RoundTripMileage * 5) / CAST(dbo.Van.MPG AS REAL)) * dbo.GasPrice.AveragePrice) / CAST(dbo.Van.NumSeats AS REAL)) * (dbo.Van.NumSeats - 1)) AS REAL), 2) AS GasIncrease,

    dbo.GasPrice.AveragePrice,

    dbo.Route.FourSeatSub,

    CASE dbo.Van.NumSeats

    WHEN '9' THEN FareStruct.NinePassVan

    WHEN '10' THEN FareStruct.TenPassVan

    WHEN '12' THEN FareStruct.TwelvePassVan

    WHEN '14' THEN FareStruct.FourteenPassVan

    END AS WFS,

    (SELECT COUNT(RiderID_PK) FROM dbo.Rider WHERE (StartDate <= @TodaysDate) GROUP BY RouteID_FK, Active HAVING (RouteID_FK = dbo.Route.RouteID_PK) AND (Active = 1)) AS NumberPassengers,

    CASE WHEN GETDATE() <= dbo.Route.Seat2ExpireDate THEN

    CASE WHEN dbo.Route.FourSeatSub = 1 THEN 4 ELSE

    CASE WHEN dbo.Van.NumSeats = (SELECT COUNT(RiderID_PK) FROM dbo.Rider WHERE (StartDate <= @TodaysDate) GROUP BY RouteID_FK, Active HAVING (RouteID_FK = dbo.Route.RouteID_PK) AND (Active = 1)) THEN 1 ELSE 2 END

    END

    ELSE CASE WHEN dbo.Route.FourSeatSub = 1 THEN 2 ELSE 1 END

    END AS SeatExp

    INTO

    #Temp1

    FROM

    dbo.Route

    INNER JOIN dbo.Van

    ON dbo.Route.RouteID_PK = dbo.Van.RouteID_FK

    LEFT JOIN FareStructure FareStruct

    ON dbo.Route.RoundTripMileage BETWEEN FareStruct.RoundTripMileageLow AND FareStruct.RoundTripMileageHigh

    CROSS JOIN dbo.GasPrice

    WHERE

    (dbo.GasPrice.DateChanged IS NULL)

    AND (dbo.Route.RouteEndDate IS NULL

    OR dbo.Route.RouteEndDate > GETDATE()

    )

    AND (dbo.GasPrice.WhichMonth = @Month)

    AND (dbo.GasPrice.WhichYear = @Year)

    AND (dbo.Van.StartDate <= GETDATE())

    AND (dbo.Van.ReturnDate IS NULL

    OR dbo.Van.ReturnDate > GETDATE()

    )

    AND (dbo.Route.RouteStartDate <= GETDATE());

    SELECT

    Temp1.*,

    (CostWSeatSub + GasIncrease) - Dis AS CostWSeatSubWFuel,

    CASE WHEN NumberPassengers < 2 THEN (CostWSeatSub + GasIncrease) - @Dis

    ELSE ((CostWSeatSub + GasIncrease) - Dis) / (NumberPassengers - 1)

    END AS BaseCostPerRiderWSubWFuel

    INTO

    #Temp2

    FROM

    (SELECT

    Temp1.*,

    ROUND(CAST((WFS * (NumSeats - SeatExp)) AS REAL), 2) AS CostWSeatSub,

    ROUND(CAST((WFS * NumSeats) AS REAL), 2) AS CostWOSeatSub,

    GasIncrease AS FuelIncrease

    FROM

    #Temp1

    ) Temp1;

    -- Update the existing records

    UPDATE

    RFHW

    SET

    [CostWSeatSub] = CostWSeatSub,

    [CostWOSeatSub] = CostWOSeatSub,

    [FuelIncrease] = FuelIncrease,

    [CostWSeatSubWFuel] = CostWSeatSubWFuel,

    [BaseCostPerRiderWSubWFuel] = BaseCostPerRiderWSubWFuel,

    [RouteID_FK] = RouteID_PK,

    [WhichMonth] = @Month,

    [WhichWeek] = @Week,

    [WhichYear] = @Year,

    [WFS] = WFS,

    [NumSeat] = NumSeats,

    [SeatExp] = SeatExp,

    [Seat2ExpireDate] = Seat2ExpireDate,

    [Discount] = Dis

    FROM

    [RouteFareHistoryWeek] RFHW

    INNER JOIN #Temp2 Temp2

    ON RFHW.RouteID_FK = Temp2.RouteID_PK

    AND RFHW.FriDate = @TodaysDate;

    -- Add the new records

    INSERT INTO [RouteFareHistoryWeek]

    ([FriDate],

    [CostWSeatSub],

    [CostWOSeatSub],

    [FuelIncrease],

    [CostWSeatSubWFuel],

    [BaseCostPerRiderWSubWFuel],

    [RouteID_FK],

    [WhichMonth],

    [WhichWeek],

    [WhichYear],

    [WFS],

    [NumSeat],

    [SeatExp],

    [Seat2ExpireDate],

    [Discount]

    )

    SELECT

    @TodaysDate,

    CostWSeatSub,

    CostWOSeatSub,

    FuelIncrease,

    CostWSeatSubWFuel,

    BaseCostPerRiderWSubWFuel,

    RouteID_PK,

    @Month,

    @Week,

    @Year,

    WFS,

    NumSeats,

    SeatExp,

    Seat2ExpireDate,

    Dis

    FROM

    #Temp2 Temp2

    WHERE

    NOT EXISTS ( SELECT

    NULL

    FROM

    [RouteFareHistoryWeek] RFHW

    WHERE

    RFHW.RouteID_FK = Temp2.RouteID_PK

    AND RFHW.FriDate = @TodaysDate );

    DROP #Temp1;

    DROP #Temp2;

    GO

    I did duplicate a formula in a couple places, I could have avoided that by adding another level of sub-queries, or another temp table.

    As Mike C said I don't have your tables and data so I can't actually test it, but I am pretty sure there aren't any syntax errors.

    It would be great if you could let us know how close we got, and if there is any speed and/or IO usage difference.

  • Good catch on the week # UMG. As you mentioned, I did assume 2005 or 2008, but only because 2000 is out of support :). If using 2000, he can easily turn the CTEs into SELECT INTO #tempTable statements. I'm sure there's plenty of opportunity for optimization here as well, if we had some sample data and expected outputs.

    One note on your update--be careful when using this syntax:

    UPDATE t

    SET ...

    FROM table t

    INNER JOIN table2 t2

    ON ...

    This syntax lends itself to parallelism and can result in unpredictable "intra-query parallelism deadlock event". I just fixed 4 or 5 production queries for a bank with this very problem.

    Thanks

    Mike C

  • Mike C (7/1/2010)


    Good catch on the week # UMG. As you mentioned, I did assume 2005 or 2008, but only because 2000 is out of support :). If using 2000, he can easily turn the CTEs into SELECT INTO #tempTable statements. I'm sure there's plenty of opportunity for optimization here as well, if we had some sample data and expected outputs.

    One note on your update--be careful when using this syntax:

    UPDATE t

    SET ...

    FROM table t

    INNER JOIN table2 t2

    ON ...

    This syntax lends itself to parallelism and can result in unpredictable "intra-query parallelism deadlock event". I just fixed 4 or 5 production queries for a bank with this very problem.

    Thanks

    Mike C

    Now THAT's interesting. What was the fix, Mike? MAXDOP?

    --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)

  • Jeff Moden (7/1/2010)


    Mike C (7/1/2010)


    Good catch on the week # UMG. As you mentioned, I did assume 2005 or 2008, but only because 2000 is out of support :). If using 2000, he can easily turn the CTEs into SELECT INTO #tempTable statements. I'm sure there's plenty of opportunity for optimization here as well, if we had some sample data and expected outputs.

    One note on your update--be careful when using this syntax:

    UPDATE t

    SET ...

    FROM table t

    INNER JOIN table2 t2

    ON ...

    This syntax lends itself to parallelism and can result in unpredictable "intra-query parallelism deadlock event". I just fixed 4 or 5 production queries for a bank with this very problem.

    Thanks

    Mike C

    Now THAT's interesting. What was the fix, Mike? MAXDOP?

    That was our DBA's quick-fix but it turned a 1 hour stored proc into a 5 hour stored proc. In some cases you can tweak the indexes on the tables to prevent table scans of the input tables, which is the underlying cause of the parallelism. In this case joins were already on clustered PK. My fix was to rewrite the query completely using ANSI correlated subquery syntax. Cut the run-time down to about 12 mins and uncovered a business logic flaw when I changed it (multiple matching rows in the second table).

    Mike C

  • Mike C (7/1/2010)


    Good catch on the week # UMG. As you mentioned, I did assume 2005 or 2008, but only because 2000 is out of support :). If using 2000, he can easily turn the CTEs into SELECT INTO #tempTable statements. I'm sure there's plenty of opportunity for optimization here as well, if we had some sample data and expected outputs.

    One note on your update--be careful when using this syntax:

    UPDATE t

    SET ...

    FROM table t

    INNER JOIN table2 t2

    ON ...

    This syntax lends itself to parallelism and can result in unpredictable "intra-query parallelism deadlock event". I just fixed 4 or 5 production queries for a bank with this very problem.

    Thanks

    Mike C

    Do you have any more information on this?

    Like how the error occurs and how to prevent it?

    I use this syntax all the time, and I never heard that it could be risky.

  • Goldie Graber (7/1/2010)


    Mike C (7/1/2010)


    Good catch on the week # UMG. As you mentioned, I did assume 2005 or 2008, but only because 2000 is out of support :). If using 2000, he can easily turn the CTEs into SELECT INTO #tempTable statements. I'm sure there's plenty of opportunity for optimization here as well, if we had some sample data and expected outputs.

    One note on your update--be careful when using this syntax:

    UPDATE t

    SET ...

    FROM table t

    INNER JOIN table2 t2

    ON ...

    This syntax lends itself to parallelism and can result in unpredictable "intra-query parallelism deadlock event". I just fixed 4 or 5 production queries for a bank with this very problem.

    Thanks

    Mike C

    Do you have any more information on this?

    Like how the error occurs and how to prevent it?

    I use this syntax all the time, and I never heard that it could be risky.

    Sure. The INNER JOIN of the source and target tables can result in a higher cost from the optimizer. The optimizer introduces table scans because of the higher costing. The optimizer then tries parallelism to speed up the table scans. Essentially your process (SPID) can launch multiple child threads (ECIDs) to fulfill the request. If this happens it means you're performing a scan against a table that you're simultaneously trying to update with multiple child threads in parallel. If your source table contains multiple matches for the target table (based on the join) you can end up trying to update the same row twice in the target table at the same time. Basically all this adds up to UPDATE and DELETE statements that deadlock on themselves. (NOTE: As long as your server does not have more than one processor, this should never happen, although you could still be introducing unnecessary table scans into your query plans.)

    As an aside, even worse than a deadlock (to me at least) is if you have multiple matching rows for the target table and you end up updating the same target rows twice, in arbitrary order, with no deadlock. This means target rows were just updated in arbitrary fashion, leaving it up to the server to choose which update gets applied last. And you may have no idea that this happened since it didn't indicate failure. In the end it's like a box of chocolates -- you never know what you're gonna get.

    The bank I just fixed this issue for was experiencing intermittent issues with this, but they recently upgraded to new, faster, better hardware and it exacerbated the problem (the server has more processors and can launch more child threads simultaneously, increasing the odds of a self-deadlock in this situation).

    There are a couple of things you can do to avoid this issue:

    * Apply the MAXDOP 1 option to UPDATEs and DELETEs that use this format (Quick and easy fix, but can make code run slow).

    * Make sure your source and target tables are properly indexed to support the join (Relatively easy fix, improves performance and can fix the issue with no code changes; downside is the additional storage and additional index maintenance cost).

    * Rewrite your UPDATE and DELETE statements to use ANSI standard syntax (Less easy fix, but can also uncover business logic issues and can improve performance dramatically).

    I confirmed this issue with Microsoft and their recommendation is to use ANSI syntax instead of the FROM clause syntax. The one exception they noted was "putting an arbitrary tree" under the {DML} statement to drive the rows flowing into the {DML} operator, "which fits {SQL Server's} algebra nicely."

    This basically means if you have to write a ton of scalar subqueries that are all similar to one another to use ANSI syntax, then the optimizer can do a better job with the {DML}...FROM...INNER JOIN... syntax. If SQL Server introduces row constructor support at some point (row-valued subqueries), that will probably be a more powerful option than the current {DML}...FROM...INNER JOIN... syntax (but alas...).

    Bart Duncan wrote an excellent post on the subject of intra-query parallelism deadlocks at http://blogs.msdn.com/b/bartd/archive/2008/09/24/today-s-annoyingly-unwieldy-term-intra-query-parallel-thread-deadlocks.aspx.

    Mike C

Viewing 15 posts - 226 through 240 (of 316 total)

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