-1

I am going through cleaning up some stored procedures left by prior employees and came across one where I am trying to determine if there is a way it can be handled without the use of a cursor. Currently the cursor looks at a temp table that has a start and end date for each record. It then takes the ID of the record and creates a row with the ID and date for each date that falls in the range of start & end date.

This data is then used create another temp table that stores the ID of the record and the count of distinct dates for each ID.

Could I not just do a datediff for the date part days between the start and end dates? I feel like there must be a much better way to derive this information but I'm worried that I may be oversimplifying it and missing something.

Here is the code as it stands now:

declare @StartDate datetime, @EndDate datetime, @ID bigint, @WSIdAndDates cursor 

set @WSIdAndDates = cursor fast_forward for
select servicestartdate, serviceenddate, WorksheetID from #que_rp_claims_dates 

open @WSIdAndDates
fetch next from @WSIdAndDates into @StartDate, @EndDate, @ID 
while @@FETCH_STATUS = 0
begin

        with ctedaterange 
            as (select [Dates]=@StartDate, 
                @ID[workSheetid]

            union ALL

            select [dates] + 1,
                @ID[workSheetid]
            from ctedaterange 
            where [dates] + 1 <= @EndDate) 

            insert into #tempworksheetidanddates
            select [workSheetid],
                [dates] 
            from ctedaterange 
        option(maxrecursion 0)

    fetch next from @WSIdAndDates into @StartDate, @EndDate, @ID 
end

close @WSIdAndDates
deallocate @WSIdAndDates

select worksheetid, count(distinct(dates)) as LOS into ##que_rp_claims_LOS 
from #tempworksheetidanddates
group by worksheetid

The table ##que_rp_claims_LOS is the only one that gets used outside of this snippet. The other temp table is dropped.

Any insight or advice would be greatly appreciated.

Jana
  • 61
  • 1
  • 3
  • 9
  • 1
    Did you try using `DATEDIFF` and test it? – Tom H May 12 '16 at 20:30
  • Which RDBMS is this for? Please add a tag to specify whether you're using `mysql`, `postgresql`, `sql-server`, `oracle` or `db2` - or something else entirely. – marc_s May 12 '16 at 20:47
  • Tom: I did with one data set and the results appear the same, I should have mentioned in my original post, but this is in a stored procedure and I'm trying to determine if there may not be situations where exceptions could cause different results. I was hoping for basically a second (or more) set of eyes to see if I'm not overlooking anything. And yes KMC, I understand the problem the code block is solving. Thanks for asking. – Jana May 12 '16 at 20:51

2 Answers2

3

The process is taking the ID and a start and end date from #que_rp_claims_dates and opens a cursor for this.

With each row the same is done: A recursive CTE is collecting/calculating all days from the start date up to the end date and fill this in a table.

In the final step you deliver nothing more than the ID and the count of days.

So I think you are right... If your intermediate results (the temp tables) are not needed anywhere else it should suffice to calculate the DATEDIFF for each ID.

Attention

If there are DATETIME values in use the result might depend on the actual time (if the times are not full day). Look at this especially carefully!

And btw: I appreciate your efforts to get rid of bad stuctures!

Community
  • 1
  • 1
Shnugo
  • 66,100
  • 9
  • 53
  • 114
  • Thank you, this is very helpful feedback. It's reassuring that you are seeing the same thing I am. I will continue to test with further data sets but I think a simple `DATEDIFF` will work. There are no `DATETIME` values in use, thankfully. – Jana May 12 '16 at 20:59
2

A better alternative to temporary tables is using table variables. You can declare a table variable like this:

DECLARE @tempVariable table(
    WorksheetID int,
    servicestartdate DATE,
    serviceenddate DATE
);

And as far as the cursor is concerned, you can always insert all you data in a table variable, use a while to iterate through it and delete the entries just after you have handled them. Something like this:

while exists (select top 1 * from tempVariable)
begin
    select top 1 @StartDate = servicestartdate, @EndDate = serviceenddate, @ID = WorksheetID from @tempVariable

    //your code

    DELETE TOP(1) FROM @tempVariable;
end
Patricia
  • 71
  • 5
  • This statement "A better alternative to temporary tables is using table variables." is a generalization that is absolutely not true. There are times when a table variable is a viable option but there are also times when they absolutely a not a good option. http://stackoverflow.com/questions/11857789/when-should-i-use-a-table-variable-vs-temporary-table-in-sql-server And you are using TOP with no order by, this is a very bad practice as you need to certain which row will be returned. https://blogs.msdn.microsoft.com/conor_cunningham_msft/2008/08/27/no-seatbelt-expecting-order-without-order-by/ – Sean Lange May 12 '16 at 21:26
  • Actually I don't need to certain which row will be returned, as I want to go through all rows, retrieving one row at a time. And if you go through all the cases when to use table variables from the post you suggested, I would incline to say table variables are indeed a "better alternative" in this case. :) – Patricia May 12 '16 at 22:03
  • Ahh but it does matter the order in your code. Since you have no order by you cannot be 100% certain that the row you delete is the same one you selected in the select statement. And yes I agree in this situation a table variable is probably the best choice. Your statement suggested they are always better which simply is not true. – Sean Lange May 13 '16 at 13:05
  • And of course replacing a cursor with a while loop is an exercise in futility. Most of the time a cursor with the right options will out perform a while loop. The best option would be to use set based logic and forget about looping all together. :D – Sean Lange May 13 '16 at 13:06