15

I have this function for a computed column :

CREATE FUNCTION [dbo].[GetAllocatedStartTime](@Year INT, @Week INT)
RETURNS DATETIME

WITH schemabinding
AS BEGIN
    RETURN dateadd(week,@Week-(1),dateadd(day,(-1),dateadd(week,datediff(week,(0),CONVERT([varchar](4),@Year,(0))+'-01-01'),(1))))
END

GO

I added the WITH schemabinding in the hope it would make it deterministic so I can persist it. It should be as the two inputs [Week] and [Year] will always yield the same results.

The exact error is :

Computed column 'AllocatedTimeStart' in table 'Tmp_Bookings' cannot be persisted because the column is non-deterministic.

I am using this formula in the column :

([dbo].[GetAllocatedStartTime]([Year],[Week]))

And the column defs :

[Week] [int] NOT NULL,
[Year] [int] NOT NULL,
[AllocatedTimeStart]  AS ([dbo].[GetAllocatedStartTime]([Year],[Week])),

Any ideas?

EDIT:

Changed line to :

RETURN dateadd(week,@Week-(1),dateadd(day,(-1),dateadd(week,datediff(week,(0),CONVERT(datetime,CONVERT([varchar](4),@Year,(0))+'0101',112)),(1))))

But now I get an error saying the formula for the column is invalid. Even though the function saves fine.

EDIT 2:

I've shown exactly what I am doing (or atleast I've tried). There is nothing extra really. As it says the previous function (original one) coupled with the formula ref [dbo].AllocatedStartDate(...) to it in the column worked, but was not persisting, it said it was non deterministic. So according to the suggestion I changed the FUNCTION, replacing the conversion part with the new code, so the function now looks like :

FUNCTION [dbo].[GetSTime](@Year INT, @Week INT)

RETURNS DATETIME
WITH schemabinding
AS BEGIN
    RETURN dateadd(week,@Week-(1),dateadd(day,(-1),dateadd(week,datediff(week,(0),CONVERT(datetime,CONVERT([varchar](4),@Year,(0))+'0101',112)),(1))))
END

Then I tried the same formula as before in the computed field (([dbo].[GetAllocatedStartTime]([Year],[Week]))) ... and it rejects the formula, says its not valid... which is strange as the formula is the same, so it must be doing some sort of check of the changed function and finding that to be invalid, which is also strange because I did a plain SELECT dbo.GetAllocatedStartTime(2012,13) and it worked...

So yes I am confused, and I've never seen SqlFiddle never mind use it. But really there is nothing more than what I have just said.

sprocket12
  • 5,368
  • 18
  • 64
  • 133
  • 1
    The problem is not with your function, its with the computed column on your table. Post that definition on your question so we can help you – Lamak Jan 02 '13 at 15:30
  • What version of SQL-Server is this for? – ypercubeᵀᴹ Jan 02 '13 at 15:45
  • Which line did you change? Do you mean you changed the computed column definition to use `RETURN dateadd(...)` directly? – Andriy M Jan 02 '13 at 16:59
  • @AndriyM yes as damien suggested, I took out the original convert and replaced it with a more explicit version. – sprocket12 Jan 02 '13 at 17:05
  • Then the issue must be the `RETURN` keyword. The syntax for a computed column definition is `ColumnName AS expression`. `RETURN` cannot be part of the expression, just drop it and leave `dateadd(...)`. Having said that, I can't really see where Damien is suggesting the replacement of your UDF with the actual expression. He is merely telling you how to make your expression (and, as a result, your function) deterministic. – Andriy M Jan 02 '13 at 17:08
  • You've obviously not shown us *exactly* what you're doing, since the computed column definition you've shown us doesn't specify `PERSISTED`. Could you try and create a small but complete example, as a single script, that demonstrates the issue please? (Possibly using [sqlfiddle](http://sqlfiddle.com/) to host it) – Damien_The_Unbeliever Jan 02 '13 at 17:12
  • @Damien_The_Unbeliever please see edit 2. – sprocket12 Jan 02 '13 at 17:33
  • Here is a [SQL Fiddle](http://sqlfiddle.com/#!3/433ba/1/0) with the last version of your function and a persisted column. Looks like it is working or ...? – Mikael Eriksson Jan 02 '13 at 17:51
  • @MikaelEriksson The link loads the page, which says "Loading" forever, also says "Build Schema" which is un-clickable because of the mask over it. Also I should mention here, I am doing all this through SSMS 2008 R2 – sprocket12 Jan 03 '13 at 08:15
  • 1
    Looks like there is an issue with SQL Fiddle. Does [this](http://sqlfiddle.com/#!6/8fd88/1/0) work better? http://sqlfiddle.com/#!6/8fd88/1/0 – Mikael Eriksson Jan 03 '13 at 08:23
  • @MikaelEriksson Thanks for that, your latest one works. It must be then that there is an issue with the way the GUI works in SSMS. I will try and amend the table to use "persisted" from SQL code. Then mark the answer. – sprocket12 Jan 03 '13 at 09:59

1 Answers1

22

CONVERT([varchar](4),@Year,(0))+'-01-01' is being passed to a DATEDIFF call, in a position where a date is expected, forcing an implicit conversion to occur.

From the rules for deterministic functions:

CAST

Deterministic unless used with datetime, smalldatetime, or sql_variant.

CONVERT

Deterministic unless one of these conditions exists:

...

Source or target type is datetime or smalldatetime, the other source or target type is a character string, and a nondeterministic style is specified. To be deterministic, the style parameter must be a constant. Additionally, styles less than or equal to 100 are nondeterministic, except for styles 20 and 21. Styles greater than 100 are deterministic, except for styles 106, 107, 109 and 113.

Well, you're calling neither, but you're relying on an implicit conversion, which I'd expect to act like CAST. Rather than rely on this, I'd switch to using CONVERT and give a deterministic style parameter.

So, I'd do: CONVERT(datetime,CONVERT([varchar](4),@Year,(0))+'0101',112) in its place. Having done so, the function itself becomes deterministic

Community
  • 1
  • 1
Damien_The_Unbeliever
  • 234,701
  • 27
  • 340
  • 448
  • 1
    If on SQL-Server 2012, they could also use - instead of the `CONVERT([varchar](4),@Year,(0))+'-01-01'` - the `DATEFROMPARTS([Year],1,1)` function: [SQL-Fiddle test](http://sqlfiddle.com/#!6/7a872) – ypercubeᵀᴹ Jan 02 '13 at 15:47
  • Hi there I tried to replace the part, but I hit another issue :/ please see edit above. – sprocket12 Jan 02 '13 at 16:03