SQL Function times out with specific input (while loop)

Question

I want to get the first business day of a given month. I generate a holiday table and use it in combination with regular weekday check, to find the first business day of a month. It seems to work. However, when I use the first month as input e.g.(2020, 1) the function times out. This is for every year like this, every other month works.

CREATE FUNCTION FirstBusinessDay(@year INT, @month INT) RETURNS DATETIME
AS BEGIN
    DECLARE @calendar TABLE (Name varchar(80), Date DATETIME)
    INSERT INTO @calendar SELECT * FROM dbo.HolidayTable(@year)
    DECLARE @d DATETIME = DATEFROMPARTS(@year, @month, 1)
    DECLARE @isHoliday bit
    SELECT @isHoliday = CASE WHEN Name IS NULL THEN 0 ELSE 1 END
    FROM @calendar WHERE Date = @d
    WHILE DATEPART(DW, @d+@@DATEFIRST-1) > 5 or @isHoliday = 'true'
    BEGIN
        SET @d = DATEADD(DAY, 1, @d)
        SELECT @isHoliday = CASE WHEN Name IS NULL THEN 0 ELSE 1 END
        FROM @calendar WHERE Date = @d
    END
    RETURN @d
END;
GO

Example usage

select dbo.FirstBusinessDay(2020, 1) as 'First Workday';  -- timeout
select dbo.FirstBusinessDay(2020, 2) as 'First Workday';  -- works 
select dbo.FirstBusinessDay(2020, 3) as 'First Workday';  -- works
select dbo.FirstBusinessDay(2021, 7) as 'First Workday';  -- works
select dbo.FirstBusinessDay(2020, 12) as 'First Workday'; -- works
select dbo.FirstBusinessDay(2021, 1) as 'First Workday';  -- timeout
select dbo.FirstBusinessDay(2022, 1) as 'First Workday';  -- timeout

As you can see, there is a pattern, every time the first month is used, it times out. Can anyone spot the issue?

Using a WHILE loop is a really bad idea in the first place, as is a multi-line scalar value function. You should really be using an inline table-value function with a set based solution here. Posting the DDL of your calendar table, which according to your above function is above TVF (I hope it isn't multi-line TVF...) so that we can show you a far better approach. — Thom A
– Thom A ♦, Commented Dec 28, 2019 at 20:48
@Larnu there is the full script, I have also one version where this is all one temporary stored procedure, with a cursor and stuff, because this solution there would require permission on the database. I don't work normally with SQL, so I dont know much why a while loop would be bad. There is probably a more SQL like way, but as I say, Im not that good with sql. — The Fool
– The Fool, Commented Dec 28, 2019 at 20:55
Cursors and Whiles, in SQL, should be avoided at all costs, unless they are really needed because there is no set based operation (like sending emails to different recipients using sp_send_dbmail). What you have here is a performance nightmare (sorry, that's probably the lightest way I can put it), with Whiles, and both multi-line table-value and scalar-value functions. Honestly, everything above needs changing. You're thinking like you're writing a Programming language; you're not. SQL is exactly what is says it is, a Structured Query Language. Don't treat it like a PL. — Thom A
– Thom A ♦, Commented Dec 28, 2019 at 21:01
Ok, thx. But saying NO. Is not really useful. I have a problem to solve and this is the way I can think if, for my taste 10-20ms max once per day is fine performance-wise. If anything, show me how to do it better. — The Fool
– The Fool, Commented Dec 28, 2019 at 21:05
I didn't say "NO" at all. I just explained that you have a lot of fundamental issues there. I certainly can rework everything there; but surely you didn't expect me to rewrite all that, for free, in 3 minutes, did you? :) — Thom A
– Thom A ♦, Commented Dec 28, 2019 at 21:07

Clockwork-Muse · Accepted Answer · 2019-12-28 21:04:43Z

2

The remarks section about setting variables this way says:

If the SELECT statement returns no rows, the variable retains its present value.

... so what's happening is pretty clear: New Year's Day is a holiday, and sets @isHoliday to true. Since there are no rows in @calendar that are not holidays, it can never be set to false. The other months only work because the default of @isHoliday is 0 - false.

That said, what you really want is a true calendar table, one listing every date, with a bunch of other indexable columns. It turns your function into a simple query:

SELECT MIN(calendar_date)
FROM Calendar
WHERE calendar_date >= DATEFROMPARTS(@year, @month, 1)
      AND is_holiday = 'false'
      AND day_of_week_iso < 6

Calendar tables are probably the most useful tables for dimensional analysis, especially for various aggregates (eg, "every sale grouped by month"), because it turns them into ranged bounds queries, instead of needing to use something like DATEPART.

answered Dec 28, 2019 at 21:04

Clockwork-Muse

13.2k6 gold badges33 silver badges50 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

The Fool Over a year ago

thank you! I get the point that a full calendar is better, but how do you create the full calendar table in the first place? That is where you would need to use a function similar to my HolidayTable function as some dates are dynamic, e.g. third Thursday in May, or even last Monday of November where it could be the fourth or fifth occurrence. Based on my holiday function I could insert holidays in such a calendar table for any arbitrary year, and the rest are just regular dates. Then I thought further and felt like this table could become stale, so why not generate it on demand?

Clockwork-Muse Over a year ago

"become stale"? No more than your holiday function. It's not like holidays move themselves - generally dates will be inserted to some number of years in the future, which takes care of the problem. There's any number of sample calendar table scripts out there. Note that some holidays defy easy calculation, like Easter.

The Fool Over a year ago

wow, that article is pretty cool. I will try to implement something like that. With stale I mean, let's say the company decides to add another holiday or remove one. I feel like editing the base holiday table is easier than adjusting the full calendar. At the end, it makes sense to use a full calendar though. I still learned a ton with my anti-SQL functions ;P

Collectives™ on Stack Overflow

SQL Function times out with specific input (while loop)

1 Answer 1

3 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related