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, Dec 28 '19 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, Dec 28 '19 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, Dec 28 '19 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, Dec 28 '19 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, Dec 28 '19 at 21:07
You have not explained anything really. I only see a big NO, but not a single why. There is no substance to your NO. Either explain or leave me alone. At the end of the day what you are doing is not answering my original question either. — The Fool, Dec 28 '19 at 21:11
[Bad Habits to Kick : Thinking a WHILE loop isn't a CURSOR](https://sqlblog.org/2012/01/26/bad-habits-to-kick-thinking-a-while-loop-isnt-a-cursor), [Multi-statement Table Valued Function vs Inline Table Valued Function](https://stackoverflow.com/q/2554333/3484879), [Interleaved MSTVFs Vs Inline Table Valued Functions](https://www.brentozar.com/archive/2017/05/sql-server-2017-interleaved-mstvfs-vs-inline-table-valued-functions/), [Why Scalar Functions Can Be Costly](https://www.sqlservercentral.com/articles/why-scalar-functions-can-be-costly) — Thom A, Dec 28 '19 at 21:16
To give a few articles. There is a huge resource on why a `WHILE`/`CURSOR` are bad in SQL, as well as how poorly multi-line table/scalar-value functions perform. Fortunately, at least, SQL Server 2017 did bring in inlining for Scalar functions, but yours won't benefit from that. — Thom A, Dec 28 '19 at 21:17

score 2 · Accepted Answer · answered Dec 28 '19 at 21:04

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 '19 at 21:04

Clockwork-Muse

12,806
6
31
45

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? – The Fool Dec 28 '19 at 22:03
"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](https://www.mssqltips.com/sqlservertip/4054/creating-a-date-dimension-or-calendar-table-in-sql-server/). Note that some holidays defy easy calculation, like Easter. – Clockwork-Muse Dec 29 '19 at 01:32
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 – The Fool Dec 29 '19 at 14:40

SQL Function times out with specific input (while loop)

1 Answers1