0

Helo everyone,

I have this method written in C#, and it basically search for a register on the database, and if it's repeatable (isRepeatable = true), it will increment and insert itself again in a list with a different date, but with the same Id and Name properties. I do specify too the repeating type (daily, weekly, monthly or yearly), and it will go until it reaches the value specified in the RepeatingEndDate or the date that i specify on the method, so it won't loop infinitely through repeatable registers that don't have a RepeatingEndDate specified.

In summary, if i have a register like this in my Database:

Id: 1
Name: Foo
Date: 03/10/2014
IsRepeatable: true
RepetitionType: 3 (Monthly)
RepeatingEndDate 05/10/2014

This will be the list of registers that my C#'s method will output from that single register:

Id: 1
Name: Foo
Date: 03/10/2014
IsRepeatable: true
RepetitionType: 3 (Monthly)
RepeatingEndDate 05/10/2014


Id: 1
Name: Foo
Date: 04/10/2014
IsRepeatable: true
RepetitionType: 3 (Monthly)
RepeatingEndDate 05/10/2014


Id: 1
Name: Foo
Date: 05/10/2014
IsRepeatable: true
RepetitionType: 3 (Monthly)
RepeatingEndDate 05/10/2014

Note that all the properties except the Date are the same, because i'm creating new Objects with all the same properties, but taking in consideration the repetition that i want and setting only the date. My goal here is to process repeatable data without saving multiple data and, in my actual case, letting the application handle the repetition.

But i found this is really CPU intense at some point, and i had the idea of converting this all to Stored Procedure in SQL SERVER.

My question is: Is it possible to convert all this C# logic into SQL SERVER stored procedure and just consume this proc in my application as a List of Registers? If so, my result will basically be getting a list of registers, some may have the same Id, Name, Etc, but different dates, according to it's repetition.

EDIT

Plain code is here: https://codereview.stackexchange.com/questions/83777/refactoring-a-loop-through-repetitive-registers

Community
  • 1
  • 1
Edgar Froes
  • 768
  • 8
  • 20
  • 2
    I don't see any c# code to translate... but to answer your question it is possible (most likely) – Florian Schmidinger Mar 10 '15 at 17:49
  • 1
    Yes it is possible. Is it advisable, Will it make it faster, Is it a good design idea? These question can't be answered given the information you provided. – Hogan Mar 10 '15 at 17:50
  • @FlorianSchmidinger My code is big and complex, and it's in Brazilian Portuguese. I just gave in the logic to translate to SP. – Edgar Froes Mar 10 '15 at 17:52
  • @Hogan I'm pretty sure it will make my application faster. – Edgar Froes Mar 10 '15 at 17:54
  • This very problem can actually be realized as a trigger in sql... – Florian Schmidinger Mar 10 '15 at 17:56
  • 1
    @EdgarSalazar - I doubt it. If so I expect your application server is under-powered or your C# code is badly implemented. The business rules you describe are much more suited to C# code than SQL server. But as I said I can't REALLY tell without more detail. – Hogan Mar 10 '15 at 17:56
  • @Hogan Here is my C# code: http://csharppad.com/gist/13fd79379a4f1340e88b It uses NHibernate to get all the Registers, then loop through them and check if they are repeatable. If not, it will just insert the register into the result list. If it is repeatable, then it will check the repetition type, repetition end date, and all that, and insert the multiple results into the result list. – Edgar Froes Mar 10 '15 at 18:02
  • use while loop in SP. – A_Sk Mar 10 '15 at 18:04
  • Yeah that code could be better. I don't speak this language so I might be missing something in the comments but this is what I see with a quick review: `var Registros = query.List().Where(r => r.Data <= Data).OrderByDescending(r => r.Data);` Remove order by here. It has no effect on the results, you do an order by at the end of the function and it will materialize the linq statement. Don't use for loops, use while loops -- each while loop should look the same -- loop till you are greater than or equal to a target date. Right now you calculate the loop and check the date – Hogan Mar 10 '15 at 18:11
  • so you are checking twice. Once to figure out how maby times the for loop should run (with all sorts of needlessly complicated date math) and then again on each iteration of the loop to see if you should add to the list. You should only do this check once and the check should just be to compare two dates. – Hogan Mar 10 '15 at 18:14
  • I'm also interested to understand why you think this would be simpler in SQL -- I'm OK at SQL and I don't think there is some magic way to do this "better" in SQL – Hogan Mar 10 '15 at 18:17
  • I just thought SP would be faster. – Edgar Froes Mar 10 '15 at 18:19
  • Don't trust me, you could post your code on http://codereview.stackexchange.com/ and ask how to improve / speed it up. – Hogan Mar 10 '15 at 18:21
  • So go for it -- write the SP code, see that it is slower and then buy me a beer next time you see me. – Hogan Mar 10 '15 at 18:22
  • @Hogan, i followed your advice and here is my plain code, translated and fully commented: http://codereview.stackexchange.com/questions/83777/refactoring-a-loop-through-repetitive-registers – Edgar Froes Mar 10 '15 at 19:10
  • @EdgarSalazar Is speed really an issue? A stored procedure might be faster, but it's hard to tell without being familiar with your app. Using a tally table (cross joins + row number + CTE), a proc could generate all the repeating dates at once. This assumes all the register data is stored in the database to begin with. – Paul Williams Mar 10 '15 at 19:20
  • @PaulWilliams - a tally table or cross join -- don't need both. Not sure what use row number would be here. – Hogan Mar 10 '15 at 19:36
  • @Hogan I just meant a CTE number table created by cross joins like [this example](http://stackoverflow.com/questions/10819/sql-auxiliary-table-of-numbers). – Paul Williams Mar 10 '15 at 20:49
  • @PaulWilliams - did you look at the relative speed? That is the worst way to do it -- thus I don't think of it, long since been dismissed as a reasonable option. – Hogan Mar 10 '15 at 20:52
  • @Hogan We may be arguing the same thing, but I meant using what that post calls "Itzik's CROSS JOINED CTE method". That is crazy fast. – Paul Williams Mar 10 '15 at 22:04
  • @PaulWilliams - For small sets (less than 100k) – Hogan Mar 11 '15 at 13:59

3 Answers3

2

I doubt SQL Server could do more efficiently and easily than c#.

Given the following class:

public class Record
{
    public int Id { get; set;} 
    public string Name {get; set;}

    public DateTime Date {get; set;}

    public bool IsRepeatable {get; set;}

    public int RepetitionType {get; set;}
    public DateTime? RepeatingEndDate { get; set; }
}

You can expand repetitions using ExpandRepetitions extension method:

public static class RecordExtensions
{

    private static Func<DateTime, DateTime>[] PeriodIncrementers = new Func<DateTime, DateTime>[]
    {
        (date) => date, // RepetitionType = 0
        (date) => date.AddDays(1), // RepetitionType = 1 (daily)
        (date) => date.AddDays(7), // RepetitionType = 2 (weekly)
        (date) => date.AddMonths(1), // RepetitionType = 3 (monthy)
        (date) => date.AddMonths(3), // RepetitionType = 4 (quarterly)
        (date) => date.AddMonths(6), // RepetitionType = 5 (semiannually)
        (date) => date.AddYears(1), // RepetitionType = 6 (annually)
        (date) => date.AddYears(2), // RepetitionType = 7 (biannually)
    };

    private static Func<DateTime, DateTime>[] DefaultDateLimiters = new Func<DateTime, DateTime>[]
    {
        (date) => date, // RepetitionType = 0
        (date) => (new DateTime(date.Year, date.Month, 1)).AddMonths(1).AddDays(-1), // RepetitionType = 1 (daily). Limit: last day of month
        (date) => date.AddDays(7 * 10 ), // RepetitionType = 2 (weekly). Limit: 10 weeks
        (date) => date.AddYears(1), // RepetitionType = 3 (monthy). Limit: 1 year
        (date) => date.AddYears(2), // RepetitionType = 4 (quarterly). Limit:  2 year
        (date) => date.AddYears(4), // RepetitionType = 5 (semiannually). Limit: 4 years 
        (date) => date.AddYears(8), // RepetitionType = 6 (annually). Limit: 8 years
        (date) => date.AddYears(16), // RepetitionType = 7 (biannually). Limit: 16 years

    };

    public static IEnumerable<Record> ExpandRepetitions(this IEnumerable<Record> records, DateTime? fromDate, DateTime? toDate)
    {
        var concatenation = Enumerable.Empty<Record>();
        foreach (var record in records)
        {
            concatenation = concatenation.Concat(ExpandRepetition(record, fromDate, toDate));
        }
        return concatenation;
    }

    private static IEnumerable<Record> ExpandRepetition(Record record, DateTime? fromDate, DateTime? toDate)
    {
        if ((fromDate == null || fromDate.Value <= record.Date) && (toDate == null || toDate.Value >= record.Date))
        {
            yield return record;
        }
        var previousRecord = record;
        DateTime endDate = record.RepeatingEndDate == null ? DefaultDateLimiters[record.RepetitionType](record.Date) : record.RepeatingEndDate.Value;
        if (toDate.HasValue && toDate.Value < endDate) endDate = toDate.Value;

        var incrementer = PeriodIncrementers[record.RepetitionType];
        if (record.IsRepeatable)
        {
            DateTime date = incrementer(previousRecord.Date);
            while (date <= endDate )
            {
                if (fromDate == null || fromDate.Value <= date)
                {
                    var newRecord = new Record
                    {
                        Date = date,
                        IsRepeatable = previousRecord.IsRepeatable,
                        Name = previousRecord.Name,
                        RepeatingEndDate = previousRecord.RepeatingEndDate,
                        RepetitionType = previousRecord.RepetitionType
                    };
                    previousRecord = newRecord;
                    yield return newRecord;
                }
                date = incrementer(date);
            }
        }
    }
}

Use like this:

var records = new Record[] {
    new Record 
    {
        Id = 1,
        Date = DateTime.Today,
        IsRepeatable = false,
        Name = "Unique",
        RepetitionType = 0
    },
    new Record 
    {
        Id = 2,
        Date = DateTime.Today,
        IsRepeatable = true,
        Name = "Daily",
        RepetitionType = 1
    },
    new Record
    {
        Id = 3,
        Date = DateTime.Today,
        IsRepeatable = true,
        Name = "Weekly",
        RepetitionType = 2,
        RepeatingEndDate = DateTime.Today.AddDays(7*2)
    }
};

var allRecords = records.ExpandRepetitions(DateTime.Today.AddDays(7), new DateTime(2015, 3, 25)).ToList();

Clean and easy!

Jesús López
  • 8,338
  • 7
  • 40
  • 66
  • Look at my code here: http://codereview.stackexchange.com/questions/83777/refactoring-a-loop-through-repetitive-registers – Edgar Froes Mar 10 '15 at 19:09
  • I think you want a yield here `return concatenation;` – Hogan Mar 10 '15 at 19:44
  • Other than that very nice answer! Lazy and optimized -- the OP should see a big improvement. The lazy evaluation should smooth out his high cpu use. I don't see the sort... maybe that does not matter. – Hogan Mar 10 '15 at 19:45
  • concatenation is IEnumerable therefore I can return it directly. Once you have expandedRecords you can order it by the property you want. Can't you? – Jesús López Mar 10 '15 at 21:24
  • I want to lick ya! This solution is damn great and so tiny! Thanks Jesus! – Edgar Froes Mar 11 '15 at 14:32
  • Jesús, i'm inserting your code in my solution, but there are a few things that needs to be solved too. RepetitionEndDate is nullable, because some Registers can go on and on infinitely. I'm setting a limit date to these cases, let's say, if i want to show all the repetitions of a register that is daily, inside a Month interval, and RepetitionEndDate is null, the limit date would be the last date of the month, otherwise i would get an infinite loop. Can i just add an DateTime argue to the ExpandRepetitions()? – Edgar Froes Mar 11 '15 at 14:58
  • 1
    @Edgar, I looked at your code and I realized the logic is more complex. However I think this code is a good starting point. Incidentally, I see you have something like ExcludedDates, you should not use the Any extension method to see if a date is there, but a HashSet or simmilar to speed things up. – Jesús López Mar 11 '15 at 15:01
  • Jesus, how can i do that? Would you mind give me some assistance? – Edgar Froes Mar 11 '15 at 15:28
  • I saw your changes to the code, but the monthly view is just an example. My application wants to display the Registers inside a day, inside a month and even inside a year. So i need a logic to get all the Register in a date interval. – Edgar Froes Mar 11 '15 at 15:30
  • I edited my answer, I added default date limiters for when RepeatingEndDate is null. each RepetitionType has its own date limiter. – Jesús López Mar 11 '15 at 15:30
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/72771/discussion-between-jesus-lopez-and-edgar-salazar). – Jesús López Mar 11 '15 at 15:51
  • I used this code as my base, and today i achieve the purpose with glory and code-style. Thanks. – Edgar Froes Jun 29 '15 at 18:57
0

CTE would solve your problem. Here is an example that may help you.

DECLARE @T TABLE(
    Id INT,
    Name VARCHAR(20),
    [Date] DATE,
    IsRepeatable BIT,
    RepetitionType TINYINT, --1=daily,2=weekly,3=monthly
    RepeatingEndDate DATE
)



INSERT INTO @T
    SELECT 1,'Foo','03/10/2014',1,3,'05/10/2014'

;WITH Date_CTE (Id,Name,IsRepeatable,RepetitionType,RepeatingEndDate,[Date])
AS
(
    select Id,Name,IsRepeatable,RepetitionType,RepeatingEndDate,[Date] from @T
    UNION ALL
    SELECT Id,Name,IsRepeatable,RepetitionType,RepeatingEndDate,
        CASE
            WHEN RepetitionType=1 THEN DATEADD(DAY,1,[Date])
            WHEN RepetitionType=2 THEN DATEADD(WEEK,1,[Date])
            WHEN RepetitionType=3 THEN DATEADD(MONTH,1,[Date])
        END [Date]
    FROM Date_CTE
    WHERE
        CASE
            WHEN RepetitionType=1 THEN DATEADD(DAY,1,[Date])
            WHEN RepetitionType=2 THEN DATEADD(WEEK,1,[Date])
            WHEN RepetitionType=3 THEN DATEADD(MONTH,1,[Date])
        END <= RepeatingEndDate
    AND IsRepeatable=1
)
select *
from Date_CTE
UnhandledExcepSean
  • 12,504
  • 2
  • 35
  • 51
  • recursive CTEs are typically used for tree traversal (where they can shine for small trees). This is not a tree traversal and I would expect this to have poor performance. (Unless your DB server is significantly overpowered or underused compared to your application server.) – Hogan Mar 10 '15 at 19:42
  • @Hogan It could be used to meet the stored procedure requested in the question and I would bet cash money that it would be faster than the current implementation. – UnhandledExcepSean Mar 11 '15 at 12:53
0

Try the following, at least to compare performance against the alternatives. It is an inline TVF that outputs the desired rows for a single "Register". For multiple "Registers", just CROSS APPLY it :-). Examples for both are provided below.

The Setup:

SET ANSI_NULLS ON;
SET QUOTED_IDENTIFIER ON;
SET NOCOUNT ON;
GO

IF (OBJECT_ID(N'dbo.GenerateRowsForDates') IS NOT NULL)
BEGIN
    DROP FUNCTION dbo.GenerateRowsForDates;
END;

GO
CREATE FUNCTION dbo.GenerateRowsForDates
(
  @Id INT,
  @Name NVARCHAR(50),
  @Date DATE,
  @IsRepeatable BIT,
  @RepetitionType TINYINT,
  @RepeatingEndDate DATE
)
RETURNS TABLE
WITH SCHEMABINDING
AS RETURN

  WITH num1(num) AS
  (
    SELECT tmp.col FROM (
                  VALUES (1), (1), (1), (1), (1), (1), (1), (1), (1), (1)
                        ) tmp(col)
  ), num2(TheNumber) AS
  (
    SELECT 0
    UNION ALL
    SELECT ROW_NUMBER() OVER (ORDER BY (SELECT NULL))
    FROM   num1 n1
    CROSS JOIN num1 n2
    --CROSS JOIN num1 n3 -- uncomment if you need more than 100 repetitions
  ), dates(TheDate) AS
  (
    SELECT TOP (CASE WHEN @IsRepeatable = 0 THEN 1
                     ELSE CASE @RepetitionType
                               WHEN 1 THEN DATEDIFF(DAY, @Date, @RepeatingEndDate)
                               WHEN 2 THEN DATEDIFF(WEEK, @Date, @RepeatingEndDate)
                               WHEN 3 THEN DATEDIFF(MONTH, @Date, @RepeatingEndDate)
                          END + 1
                END)
           CASE @RepetitionType
              WHEN 1 THEN DATEADD(DAY, num2.TheNumber, @Date)
              WHEN 2 THEN DATEADD(WEEK, num2.TheNumber, @Date)
              WHEN 3 THEN DATEADD(MONTH, num2.TheNumber, @Date)
           END
    FROM   num2
  )
  SELECT 
         @Id AS [Id],
         @Name AS [Name],
         dates.TheDate AS [Date],
         @IsRepeatable AS [IsRepeatable],
         @RepetitionType AS [RepetitionType],
         @RepeatingEndDate AS [RepeatingEndDate]
  FROM dates;
GO

Single Register tests:

SELECT * FROM dbo.GenerateRowsForDates(1, N'Foo', '2014-03-10', 1, 3, '2014-05-10');
-- 3 rows

SELECT * FROM dbo.GenerateRowsForDates(1, N'Foo', '2014-03-10', 0, 3, '2014-05-10');
-- 1 row (due to @IsRepeatable being set to 0)

Multiple Register test:

DECLARE @Registers TABLE
(
  Id INT,
  Name NVARCHAR(50),
  [Date] DATE,
  IsRepeatable BIT,
  RepetitionType TINYINT,
  RepeatingEndDate DATE
);

INSERT INTO @Registers VALUES (1, N'Foo', '2014-03-10', 1, 3, '2014-05-10');
INSERT INTO @Registers VALUES (2, N'Who', '2014-03-10', 1, 1, '2014-05-10');
INSERT INTO @Registers VALUES (3, N'You', '2014-03-10', 1, 2, '2014-05-10');

SELECT dates.*
FROM   @Registers reg
CROSS APPLY dbo.GenerateRowsForDates(reg.[Id], reg.[Name], reg.[Date],
                                     reg.[IsRepeatable], reg.[RepetitionType],
                                     reg.[RepeatingEndDate]) dates
ORDER BY dates.[Id] ASC, dates.[Date] ASC;
Solomon Rutzky
  • 46,688
  • 9
  • 128
  • 171