April 11, 2014 at 9:39 am
I have a very simple stored procedure to be used in Dundas Dashboard:
ALTER proc [dbo].[ddGetCurrentOpenAndClosed]
AS
declare @Open int
declare @Closed int
select @Open = count(1) From Document Where DocType = 1 and DocStatus = 'Active'
Select @Closed = count(1) From Document Where dbo.fnFiscalYear(RetiredDate) = dbo.fnFiscalYear(getdate()) and DocType = 1 and DocStatus = 'Retired'
Select @Open as [Open], @Closed as Closed
fnFiscalYear:
ALTER FUNCTION [dbo].[fnFiscalYear](@AsOf DATETIME)
RETURNS INT
AS
BEGIN
DECLARE @AnswerINT
-- Oct 31 is the fiscal year end
-- 20131031 fiscal year is 2013
-- 20131101 fiscal year is 2014
IF ( MONTH(@AsOf) < 11 )
SET @Answer = YEAR(@AsOf)
ELSE
SET @Answer = YEAR(@AsOf) + 1
RETURN @Answer
END
The sp is not in production yet, and it is very time consuming to get it into production, however, I have the choice to use what Dundas refers to "Standard SQL Select" to directly generate the data I need. Here is the error message from Dundas:
The user statement could not be parsed. For security purposes, only statements that translate to standard SELECT SQL are allowed. ORDER BY statements are also disallowed for Virtual Tables.
I guess what it means is I need to remove the use of declare and function.
Can anyone help rewrite the sp?
Thank you very much.
April 11, 2014 at 9:58 am
SELECT
[Open] = SUM(CASE WHEN DocStatus = 'Active' THEN 1 ELSE 0 END),
[Closed] = SUM(CASE WHEN DocStatus = 'Retired' THEN 1 ELSE 0 END)
FROM Document
CROSS APPLY (
SELECT
CurrentDateFiscalYear = CASE WHEN MONTH(GETDATE()) < 11 THEN YEAR(GETDATE()) ELSE YEAR(GETDATE())+1,
RetiredDateFiscalYear = CASE WHEN MONTH(RetiredDate) < 11 THEN YEAR(GETDATE()) ELSE YEAR(RetiredDate)+1
) x
WHERE DocType = 1
AND DocStatus IN ('Active','Retired')
AND RetiredDateFiscalYear = CurrentDateFiscalYear
-- you could greatly improve performance by calculating the start and end of the range of RetiredDate
-- for any given value of GETDATE()
For fast, accurate and documented assistance in answering your questions, please read this article.
Understanding and using APPLY, (I) and (II) Paul White
Hidden RBAR: Triangular Joins / The "Numbers" or "Tally" Table: What it is and how it replaces a loop Jeff Moden
April 11, 2014 at 10:25 am
thank you so much for your quick reply, other than you forgot the End for the Case, it's working perfectly.
April 11, 2014 at 1:47 pm
This is driving people crazy!
There is another sp to be converted and this sp contains loop and lots of calculation. The original sp is working fine and logic is very simple, can I have advice from anyone?
Create proc [dbo].[ddGetDevDRROnly]
AS
Declare @q int --variable for quarter
Declare @Balance int
Declare @SVP varchar(20)
Declare @b-2 int --Open Deviations as of Oct 31
Declare @C int --New dev with original TargetClosureDate within current fiscal year
Declare @D int --New dev with original TargetClosureDate beyong current fiscal year
Declare @a int --YTD number of retired dev
Declare @DRR float --A/(B+C+D)
Create table #temp
(
Division varchar(20),
B int,
C int,
D int,
A int,
DRR float
)
--Loop each Division
declare cDivision cursor for
Select Division From ITSDivision
OPEN cDivision
FETCH NEXT FROM cDivision
INTO @SVP
WHILE @@FETCH_STATUS = 0
BEGIN
SELECT @b-2 = count(1) FROM Document d
left outer join ITSDivision i on d.ITSDivisionID = i.ID
Where
i.Division = @SVP and
DocStatus = 'Active' and
DocType = 1
SELECT @C = count(1) FROM Document d
left outer join ITSDivision i on d.ITSDivisionID = i.ID
Where
i.Division = @SVP and
DocStatus = 'Active' and
DocType = 1 and
CAST(YEAR(TargetClosureDate) AS int) = dbo.fnFiscalYear(getdate())
SELECT @D = count(1) FROM Document d
left outer join ITSDivision i on d.ITSDivisionID = i.ID
Where
i.Division = @SVP and
DocStatus = 'Active' and
DocType = 1 and
CAST(YEAR(TargetClosureDate) AS int) > dbo.fnFiscalYear(getdate())
SELECT @a = count(1) FROM Document d
left outer join ITSDivision i on d.ITSDivisionID = i.ID
Where
i.Division = @SVP and
DocStatus = 'Retired' and
DocType = 1 and
CAST(YEAR(RetiredDate) AS int) = (dbo.fnFiscalYear(getdate())-1)
IF ((@B + @C + @D)> 0)
BEGIN
Set @DRR = CONVERT(DECIMAL(8,1), 100.0) * @a /(@B + @C + @D)
END
ELSE
BEGIN
Set @DRR = 0
END
Insert into #temp Values(@SVP, @b-2, @C, @D, @a, @DRR)
FETCH NEXT FROM cDivision
INTO @SVP
END
CLOSE cDivision
DEALLOCATE cDivision
select * from #temp order by DRR desc
April 11, 2014 at 7:38 pm
halifaxdal (4/11/2014)
This is driving people crazy!There is another sp to be converted and this sp contains loop and lots of calculation. The original sp is working fine and logic is very simple, can I have advice from anyone?
Create proc [dbo].[ddGetDevDRROnly]
AS
Declare @q int --variable for quarter
Declare @Balance int
Declare @SVP varchar(20)
Declare @b-2 int --Open Deviations as of Oct 31
Declare @C int --New dev with original TargetClosureDate within current fiscal year
Declare @D int --New dev with original TargetClosureDate beyong current fiscal year
Declare @a int --YTD number of retired dev
Declare @DRR float --A/(B+C+D)
Create table #temp
(
Division varchar(20),
B int,
C int,
D int,
A int,
DRR float
)
--Loop each Division
declare cDivision cursor for
Select Division From ITSDivision
OPEN cDivision
FETCH NEXT FROM cDivision
INTO @SVP
WHILE @@FETCH_STATUS = 0
BEGIN
SELECT @b-2 = count(1) FROM Document d
left outer join ITSDivision i on d.ITSDivisionID = i.ID
Where
i.Division = @SVP and
DocStatus = 'Active' and
DocType = 1
SELECT @C = count(1) FROM Document d
left outer join ITSDivision i on d.ITSDivisionID = i.ID
Where
i.Division = @SVP and
DocStatus = 'Active' and
DocType = 1 and
CAST(YEAR(TargetClosureDate) AS int) = dbo.fnFiscalYear(getdate())
SELECT @D = count(1) FROM Document d
left outer join ITSDivision i on d.ITSDivisionID = i.ID
Where
i.Division = @SVP and
DocStatus = 'Active' and
DocType = 1 and
CAST(YEAR(TargetClosureDate) AS int) > dbo.fnFiscalYear(getdate())
SELECT @a = count(1) FROM Document d
left outer join ITSDivision i on d.ITSDivisionID = i.ID
Where
i.Division = @SVP and
DocStatus = 'Retired' and
DocType = 1 and
CAST(YEAR(RetiredDate) AS int) = (dbo.fnFiscalYear(getdate())-1)
IF ((@B + @C + @D)> 0)
BEGIN
Set @DRR = CONVERT(DECIMAL(8,1), 100.0) * @a /(@B + @C + @D)
END
ELSE
BEGIN
Set @DRR = 0
END
Insert into #temp Values(@SVP, @b-2, @C, @D, @a, @DRR)
FETCH NEXT FROM cDivision
INTO @SVP
END
CLOSE cDivision
DEALLOCATE cDivision
select * from #temp order by DRR desc
Yes... since the logic is "simple", I strongly recommend that you give it a shot on your own because you're likely the one that's going to have to support it. At least try. 😉 The best advice I can give to anyone trying to make such changes is in my signature line below about what to think about.
--Jeff Moden
Change is inevitable... Change for the better is not.
April 11, 2014 at 7:56 pm
Jeff Moden (4/11/2014)
halifaxdal (4/11/2014)
This is driving people crazy!There is another sp to be converted and this sp contains loop and lots of calculation. The original sp is working fine and logic is very simple, can I have advice from anyone?
Yes... since the logic is "simple", I strongly recommend that you give it a shot on your own because you're likely the one that's going to have to support it. At least try. 😉 The best advice I can give to anyone trying to make such changes is in my signature line below about what to think about.
Hi Jeff,
Your reply is NOT an advice, it's an encouragement; I like to take your encouragement but I also post the question for advice.
What I would like to know:
1. How to loop the Division in a single Select
2. How to do the calculation for @DRR which is @a /(@B + @C + @D)
It would be appreciated if you or anyone can give me any clue, thank you in advance.
April 11, 2014 at 8:21 pm
Nope... it's absolutely solid "advice" in its purest form. You just don't see it, yet. You're worried about solving the problem all at once and can't see the trees for the forest. 😉 You need to peel just one potato at a time. I'll show you what I mean. Let's see if we can walk you into solving this problem.
Look at the code you posted. Following what I said about thinking in columns in my signature line, what do you see in common in each SELECT for each variable being populated in the cursor? The first step would be to write a SINGLE select using the COMMON join criteria that's available in all 4 similar queries.
What should be in the SELECT list itself? That's simple. Remove the COUNT(1) and forget about it for right now. Move all the columns in the WHERE that are not common in all 4 queries up to the SELECT list. Once you've done that, post the code here and we'll show you what's next.
And, don't jump ahead. Remember, we're peeling one potato at a time here so that you learn the steps.
--Jeff Moden
Change is inevitable... Change for the better is not.
April 12, 2014 at 5:21 am
Thank you Jeff, I appreciate your enlightening, here is what I got so far:
SELECTB = Sum(Case When
DocStatus = 'Active'
Then 1 Else 0 End),
C = Sum(Case When
DocStatus = 'Active' and
CAST(YEAR(TargetClosureDate) AS int) = dbo.fnFiscalYear(getdate())
Then 1 Else 0 End),
D = Sum(Case When
DocStatus = 'Active' and
CAST(YEAR(TargetClosureDate) AS int) > dbo.fnFiscalYear(getdate())
Then 1 Else 0 End),
A = Sum(Case When
DocStatus = 'Retired' and
CAST(YEAR(RetiredDate) AS int) = (dbo.fnFiscalYear(getdate())-1)
Then 1 Else 0 End),
Critical = Sum(Case When
DocStatus = 'Active' and
RiskRating = 4
Then 1 Else 0 End),
High = Sum(Case When
DocStatus = 'Active' and
RiskRating = 3
Then 1 Else 0 End),
Medium = Sum(Case When
DocStatus = 'Active' and
RiskRating = 2
Then 1 Else 0 End),
Low = Sum(Case When
DocStatus = 'Active' and
RiskRating = 1
Then 1 Else 0 End),
TBD = Sum(Case When
DocStatus = 'Active' and
RiskRating = 0
Then 1 Else 0 End),
BCD =Sum(Case When
DocStatus = 'Active'
Then 1 Else 0 End) ----B
+
Sum(Case When
DocStatus = 'Active' and
CAST(YEAR(TargetClosureDate) AS int) = dbo.fnFiscalYear(getdate())
Then 1 Else 0 End)-----C
+
Sum(Case When
DocStatus = 'Active' and
CAST(YEAR(TargetClosureDate) AS int) > dbo.fnFiscalYear(getdate())
Then 1 Else 0 End)
FROM Document d
left outer join ITSDivision i on d.ITSDivisionID = i.ID
Where
i.Division = @SVP and
DocType = 1
It returns all values for a specific @SVP except the DRR which is a percentage DRR = A/(B+C+D), can you help? Thanks.
April 12, 2014 at 6:49 am
halifaxdal (4/11/2014)
I have a very simple stored procedure to be used in Dundas Dashboard:
ALTER proc [dbo].[ddGetCurrentOpenAndClosed]
AS
declare @Open int
declare @Closed int
select @Open = count(1) From Document Where DocType = 1 and DocStatus = 'Active'
Select @Closed = count(1) From Document Where dbo.fnFiscalYear(RetiredDate) = dbo.fnFiscalYear(getdate()) and DocType = 1 and DocStatus = 'Retired'
Select @Open as [Open], @Closed as Closed
fnFiscalYear:
ALTER FUNCTION [dbo].[fnFiscalYear](@AsOf DATETIME)
RETURNS INT
AS
BEGIN
DECLARE @AnswerINT
-- Oct 31 is the fiscal year end
-- 20131031 fiscal year is 2013
-- 20131101 fiscal year is 2014
IF ( MONTH(@AsOf) < 11 )
SET @Answer = YEAR(@AsOf)
ELSE
SET @Answer = YEAR(@AsOf) + 1
RETURN @Answer
END
The sp is not in production yet, and it is very time consuming to get it into production, however, I have the choice to use what Dundas refers to "Standard SQL Select" to directly generate the data I need. Here is the error message from Dundas:
The user statement could not be parsed. For security purposes, only statements that translate to standard SELECT SQL are allowed. ORDER BY statements are also disallowed for Virtual Tables.
I guess what it means is I need to remove the use of declare and function.
Can anyone help rewrite the sp?
Thank you very much.
You do not need this function, this can be achieved in a select as this:
😎
;WITH SAMPLE_DATE(XDATE) AS
(
SELECT CONVERT(DATETIME2(0),XDATE,120) AS XDATE
FROM (VALUES
('2007-11-22 00:00:00.000'),('2007-10-03 00:00:00.000')
,('2007-09-21 00:00:00.000'),('2006-11-07 00:00:00.000')
,('2008-04-29 00:00:00.000'),('2006-10-13 00:00:00.000')
,('2008-05-07 00:00:00.000'),('2008-04-05 00:00:00.000')
,('2007-08-05 00:00:00.000'),('2008-06-02 00:00:00.000')
,('2007-12-26 00:00:00.000'),('2007-09-26 00:00:00.000')
,('2008-01-31 00:00:00.000') ) AS X(XDATE)
)
SELECT
SD.XDATE
,YEAR(SD.XDATE) + SIGN(1 + SIGN(MONTH(SD.XDATE) - 11 )) AS FiscalYear
FROM SAMPLE_DATE SD
April 12, 2014 at 8:22 am
There is another way, look at the last Fiscal Year calculation:
WITH SAMPLE_DATE(XDATE) AS
(
SELECT CONVERT(DATETIME2(0),XDATE,120) AS XDATE
FROM (VALUES
('2007-11-22 00:00:00.000'),('2007-10-03 00:00:00.000')
,('2007-09-21 00:00:00.000'),('2006-11-07 00:00:00.000')
,('2008-04-29 00:00:00.000'),('2006-10-13 00:00:00.000')
,('2008-05-07 00:00:00.000'),('2008-04-05 00:00:00.000')
,('2007-08-05 00:00:00.000'),('2008-06-02 00:00:00.000')
,('2007-12-26 00:00:00.000'),('2007-09-26 00:00:00.000')
,('2008-01-31 00:00:00.000') ) AS X(XDATE)
)
SELECT
SD.XDATE
,YEAR(SD.XDATE) + SIGN(1 + SIGN(MONTH(SD.XDATE) - 11 )) AS FiscalYear
,year(dateadd(month,2,SD.XDATE)) As AlsoFiscalYear -- This looks so much cleaner IMHO
FROM SAMPLE_DATE SD;
April 12, 2014 at 11:31 am
Lynn Pettis (4/12/2014)
There is another way, look at the last Fiscal Year calculation:
WITH SAMPLE_DATE(XDATE) AS
(
SELECT CONVERT(DATETIME2(0),XDATE,120) AS XDATE
FROM (VALUES
('2007-11-22 00:00:00.000'),('2007-10-03 00:00:00.000')
,('2007-09-21 00:00:00.000'),('2006-11-07 00:00:00.000')
,('2008-04-29 00:00:00.000'),('2006-10-13 00:00:00.000')
,('2008-05-07 00:00:00.000'),('2008-04-05 00:00:00.000')
,('2007-08-05 00:00:00.000'),('2008-06-02 00:00:00.000')
,('2007-12-26 00:00:00.000'),('2007-09-26 00:00:00.000')
,('2008-01-31 00:00:00.000') ) AS X(XDATE)
)
SELECT
SD.XDATE
,YEAR(SD.XDATE) + SIGN(1 + SIGN(MONTH(SD.XDATE) - 11 )) AS FiscalYear
,year(dateadd(month,2,SD.XDATE)) As AlsoFiscalYear -- This looks so much cleaner IMHO
FROM SAMPLE_DATE SD;
Nice 😎
And around 10% faster than my code :pinch:
April 12, 2014 at 6:25 pm
halifaxdal (4/12/2014)
Thank you Jeff, I appreciate your enlightening, here is what I got so far:
SELECTB = Sum(Case When
DocStatus = 'Active'
Then 1 Else 0 End),
C = Sum(Case When
DocStatus = 'Active' and
CAST(YEAR(TargetClosureDate) AS int) = dbo.fnFiscalYear(getdate())
Then 1 Else 0 End),
D = Sum(Case When
DocStatus = 'Active' and
CAST(YEAR(TargetClosureDate) AS int) > dbo.fnFiscalYear(getdate())
Then 1 Else 0 End),
A = Sum(Case When
DocStatus = 'Retired' and
CAST(YEAR(RetiredDate) AS int) = (dbo.fnFiscalYear(getdate())-1)
Then 1 Else 0 End),
Critical = Sum(Case When
DocStatus = 'Active' and
RiskRating = 4
Then 1 Else 0 End),
High = Sum(Case When
DocStatus = 'Active' and
RiskRating = 3
Then 1 Else 0 End),
Medium = Sum(Case When
DocStatus = 'Active' and
RiskRating = 2
Then 1 Else 0 End),
Low = Sum(Case When
DocStatus = 'Active' and
RiskRating = 1
Then 1 Else 0 End),
TBD = Sum(Case When
DocStatus = 'Active' and
RiskRating = 0
Then 1 Else 0 End),
BCD =Sum(Case When
DocStatus = 'Active'
Then 1 Else 0 End) ----B
+
Sum(Case When
DocStatus = 'Active' and
CAST(YEAR(TargetClosureDate) AS int) = dbo.fnFiscalYear(getdate())
Then 1 Else 0 End)-----C
+
Sum(Case When
DocStatus = 'Active' and
CAST(YEAR(TargetClosureDate) AS int) > dbo.fnFiscalYear(getdate())
Then 1 Else 0 End)
FROM Document d
left outer join ITSDivision i on d.ITSDivisionID = i.ID
Where
i.Division = @SVP and
DocType = 1
It returns all values for a specific @SVP except the DRR which is a percentage DRR = A/(B+C+D), can you help? Thanks.
Heh... rush, rush, rush. No one wants to peel just one potato at a time and that's why you're having problems with the DRR calculation and why your code will run slower than it needs to. 😉
You did real well in realizing that it wasn't a COUNT that you wanted and that you wanted to do conditional SUMs of 1's, instead. That technique is known as a CROSSTAB and you can read more about it at the following two links.
http://www.sqlservercentral.com/articles/T-SQL/63681/
http://www.sqlservercentral.com/articles/Crosstab/65048/
The reason why I accused you of not peeling one potato at a time is because you've had to do a recalculation of B, C, and D to calculate the value of the BCD column right now. As a result, your code is unneccessarily long and the extra calculations could make it run a fair bit slower unless the Optimizer recognizes those formulas as the same formulas previously calculated and just happens to reuse them.
With the idea that we want to make the code do as few duplicate calculations as necessary for reasons of performance and to make the code both easier to read and troubleshoot, let's begin the simplification process...
As you pointed out, your code returns all the values for a specific @SVP. Another way of saying that is that it returns all of the values for JUST ONE @SVP. If we look back at the original cursor code, @SVP isn't a parameter... it's a variable populated by the cursor and then it loops through each and every value of the ITSDivision table just to find the ITSDivisionID... WHICH IS ALREADY AVAILABLE IN THE DOCUMENT TABLE AND IT DOES A LEFT JOIN ON THE DOCUMENT TABLE. That at least implies that you want everything in the document table regardless. Also notice that the DIVISION column (presumably a name column) isn't actually used anywhere except in the final output, so let's get rid of the join to the ITSDivision table and GROUP BY the ITSDivisionID column of the document table and get rid of the join altogether. It's just not needed, yet, and there's no sense in doing the join on so many rows as what are in the Document table.
Another simplification is that there's actually no need to CAST the return value of YEAR to an INT. It's guaranteed to be an INT, so casting it just burns clock cycles without adding any value.
To get rid of repetative calculations, I removed the calculation for BCD. We'll add it in later just like we will for the "Division" and DRR columns... all of which will be incredibly simple because we're simplifying the base code.
I also shortened the length of the code and did some nice vertical alignment to make it easier to read. We still have some more simplification and alignment to do but, if we apply the simplifications that we have so far, here's what we end up with. Note that I don't have your tables or data so I'm doing this by "feel" and I've not actually tested the code.
SELECTITSDivisionID
,A = Sum(Case When DocStatus = 'Retired' and YEAR(RetiredDate) = (dbo.fnFiscalYear(getdate())-1) Then 1 Else 0 End)
,B = Sum(Case When DocStatus = 'Active' Then 1 Else 0 End)
,C = Sum(Case When DocStatus = 'Active' and YEAR(TargetClosureDate) = dbo.fnFiscalYear(getdate()) Then 1 Else 0 End)
,D = Sum(Case When DocStatus = 'Active' and YEAR(TargetClosureDate) > dbo.fnFiscalYear(getdate()) Then 1 Else 0 End)
,Critical = Sum(Case When DocStatus = 'Active' and RiskRating = 4 Then 1 Else 0 End)
,High = Sum(Case When DocStatus = 'Active' and RiskRating = 3 Then 1 Else 0 End)
,Medium = Sum(Case When DocStatus = 'Active' and RiskRating = 2 Then 1 Else 0 End)
,Low = Sum(Case When DocStatus = 'Active' and RiskRating = 1 Then 1 Else 0 End)
,TBD = Sum(Case When DocStatus = 'Active' and RiskRating = 0 Then 1 Else 0 End)
FROM dbo.Document
WHERE DocType = 1
GROUP BY d.ITSDivisionID
;
The next simplification that will help this code fly is to remove the year "calculations". Simple comparisons with no calculations to the left of the relational operator can be a fair bit faster than comparisons with equations on both sides of the relational operator. We also don't need to calculate the Fiscal year more than once. If we get lucky, the Optimizer will recognize the function calculation as a "run-time constant". If we don't get so lucky, it'll be doing a whole bunch of duplicated calculations. With that in mind and assuming that we have to write this as a direct replacement for a stored procedure instead of an iTVF (Inline Table Valued Function) or a view, we can make it so that we only do a couple of calculations for fiscal year for the whole proc instead of once for each row. With the idea that we don't want to have to calculate the integer based year for multiple times for every row in the Document table, we're going to do something a little different and not use the function, at all (and there's a HUGE wakup call coming, as well).
The first step in all of that is to calculate the start dates (1st of November for each respective year) for each required fiscal year... like this.
--===== Declare local variables for the start date of fiscal years
DECLARE @FYStart DATETIME --November 1st of current FY
,@FYNextStart DATETIME --November 1st of next FY
,@FYPrevStart DATETIME --November 1st of previous FY
;
--===== Calculate the start date of each named fiscal year
SELECT @FYStart = DateAdd(mm,-2,DateAdd(yy,DateDiff(yy,0,DateAdd(mm,2,Getdate())),0))
,@FYNextStart = DateAdd(yy,1,@FYStart)
,@FYPrevStart = DateAdd(yy,-1,@FYStart)
;
Now, we can remove some of those nasty ol' CPU eating calculations from the code and replace them with simple comparisons.
--===== Declare local variables for the start date of fiscal years
DECLARE @FYStart DATETIME --November 1st of current FY
,@FYNextStart DATETIME --November 1st of next FY
,@FYPrevStart DATETIME --November 1st of previous FY
;
--===== Calculate the start date of each named fiscal year
SELECT @FYStart = DateAdd(mm,-2,DateAdd(yy,DateDiff(yy,0,DateAdd(mm,2,Getdate())),0))
,@FYNextStart = DateAdd(yy, 1,@FYStart)
,@FYPrevStart = DateAdd(yy,-1,@FYStart)
;
SELECTITSDivisionID
,A = Sum(Case When DocStatus = 'Retired' and RetiredDate >= @FYPrevStart and RetiredDate < @FYStart Then 1 Else 0 End)
,B = Sum(Case When DocStatus = 'Active' Then 1 Else 0 End)
,C = Sum(Case When DocStatus = 'Active' and TargetClosureDate >= @FYStart and TargetClosureDate < @FYNextStart Then 1 Else 0 End)
,D = Sum(Case When DocStatus = 'Active' and TargetClosureDate >= @FYNextStart Then 1 Else 0 End)
,Critical = Sum(Case When DocStatus = 'Active' and RiskRating = 4 Then 1 Else 0 End)
,High = Sum(Case When DocStatus = 'Active' and RiskRating = 3 Then 1 Else 0 End)
,Medium = Sum(Case When DocStatus = 'Active' and RiskRating = 2 Then 1 Else 0 End)
,Low = Sum(Case When DocStatus = 'Active' and RiskRating = 1 Then 1 Else 0 End)
,TBD = Sum(Case When DocStatus = 'Active' and RiskRating = 0 Then 1 Else 0 End)
FROM dbo.Document
WHERE DocType = 1
AND DocStatus IN ('Active','Retired')
GROUP BY ITSDivisionID
;
[font="Arial Black"]HERE'S THE HUGE WAKEUP CALL!!!. [/font]Let's look at the way they did the date calculations in the original code...
YEAR(TargetClosureDate) = dbo.fnFiscalYear(getdate())
Do you see anything wrong with that code? Anything at all? Let's do a wee bit o' analysis. If GETDATE() is 2014-04-12, then the function will return the number 2014 as the fiscal year. All of the dates for TargetClosureDate must be in the same year of 2014. BUT, BECAUSE THEY DIDN'T APPLY THE SAME FUNCTION TO THE TARGETCLOSUREDATE COLUMN, [font="Arial Black"]IT'S RETURNING THE CALENDAR YEAR INSTEAD OF THE FISCAL YEAR!!!!! [/font]WHOOPS!!! How long has that problem been in place with the original code????
The method that I used correctly calculates the start date of fiscal years and then compares the dates for columns like TargetClosureDate against the actual dates in a fiscal year so the calculation is actually done correctly. This is what happens when people try to take such shortcuts. NEVER take shortcuts where date ranges are involved. ALWAYS (and I don't say that word often) follow the basic format for doing the comparisons as in the following...
WHERE SomeDateColumn >= SomePeriodStartDate
AND SomeDateColumn < NextPeriodStartDate
The method above is also a hidden performance enhancement because it doesn't have to calculate a value for each and ever bloody SomeDateColumn in a given table.
Don'cha just love surprises? 😛
Now... how to calculate BCD, DRR, and retrieve the value of the related "Division" column WITHOUT repeating a bunch of calculations? I'm going to let you think about that a bit with the following hint... Do you know what CTEs are?
--Jeff Moden
Change is inevitable... Change for the better is not.
April 12, 2014 at 7:01 pm
Thank you guys so much and especially to Jeff for the analysis.
The one with function was accepted by the Dundas and the other one with DECLARE was rejected as DECLARE is not allowed in building Virtual Table in Dundas Dashboard.
And, HUGE THANKS TO YOUR FINDING OF THE TARGETCLOSUREDATE AND FISCAL YEAR
Good thing is the original sp is not in production yet.
Yes I know CTE although I don't use it often, in Dundas I get this error for script:
WITH test (ID, Ref, Title )
AS
-- Define the CTE query.
(
SELECT ID, Ref, Title
FROM Document
WHERE DocStatus = 'Active' and DocType = 1
)
-- Define the outer query referencing the CTE name.
SELECT *
FROM test
Incorrect syntax near the keyword 'WITH'.
Incorrect syntax near the keyword 'with'. If this statement is a common table expression, an xmlnamespaces clause or a change tracking context clause, the previous statement must be terminated with a semicolon.
Incorrect syntax near ')'.
April 12, 2014 at 7:13 pm
halifaxdal (4/12/2014)
Thank you guys so much and especially to Jeff for the analysis.The one with function was accepted by the Dundas and the other one with DECLARE was rejected as DECLARE is not allowed in building Virtual Table in Dundas Dashboard.
And, HUGE THANKS TO YOUR FINDING OF THE TARGETCLOSUREDATE AND FISCAL YEAR
Good thing is the original sp is not in production yet.
Yes I know CTE although I don't use it often, in Dundas I get this error for script:
WITH test (ID, Ref, Title )
AS
-- Define the CTE query.
(
SELECT ID, Ref, Title
FROM Document
WHERE DocStatus = 'Active' and DocType = 1
)
-- Define the outer query referencing the CTE name.
SELECT *
FROM test
Incorrect syntax near the keyword 'WITH'.
Incorrect syntax near the keyword 'with'. If this statement is a common table expression, an xmlnamespaces clause or a change tracking context clause, the previous statement must be terminated with a semicolon.
Incorrect syntax near ')'.
The CTE is wrong for the problem we're working on. That's not what's causing the error though. Look at the error. I there code before the CTE? Such code would be known as the "previous statement". That's a hint.
And what the heck is "Dundas"??? If it's an application, I can't believe that such a thing wouldn't accept a DECLARE in a stored procedure (and we haven't yet finished converting this to a stored procedure).
--Jeff Moden
Change is inevitable... Change for the better is not.
April 12, 2014 at 8:06 pm
Jeff Moden (4/12/2014)
halifaxdal (4/12/2014)
Thank you guys so much and especially to Jeff for the analysis.The one with function was accepted by the Dundas and the other one with DECLARE was rejected as DECLARE is not allowed in building Virtual Table in Dundas Dashboard.
And, HUGE THANKS TO YOUR FINDING OF THE TARGETCLOSUREDATE AND FISCAL YEAR
Good thing is the original sp is not in production yet.
Yes I know CTE although I don't use it often, in Dundas I get this error for script:
WITH test (ID, Ref, Title )
AS
-- Define the CTE query.
(
SELECT ID, Ref, Title
FROM Document
WHERE DocStatus = 'Active' and DocType = 1
)
-- Define the outer query referencing the CTE name.
SELECT *
FROM test
Incorrect syntax near the keyword 'WITH'.
Incorrect syntax near the keyword 'with'. If this statement is a common table expression, an xmlnamespaces clause or a change tracking context clause, the previous statement must be terminated with a semicolon.
Incorrect syntax near ')'.
The CTE is wrong for the problem we're working on. That's not what's causing the error though. Look at the error. I there code before the CTE? Such code would be known as the "previous statement". That's a hint.
And what the heck is "Dundas"??? If it's an application, I can't believe that such a thing wouldn't accept a DECLARE in a stored procedure (and we haven't yet finished converting this to a stored procedure).
The CTE is just for testing if it can be accepted in the Dundas, it works in SQL with no problem. I talked to Dundas technical support on Friday and was replied only standard SELECT grammar can be used to create Virtual Table which is part of the procedure of creating Dashboard.
Dundas accepts 3 methods creating Virtual table. 1. Based on Table 2. Based on Stored Procedure 3. Based on Manual code
If I choose to use second method, there is no any problem using DECLARE or CTE or very complex grammar, but there are cases I want to use method 3. That's my current situation.
Dundas can be seen here: dundas.com
Thank you again for your time and patience.
Viewing 15 posts - 1 through 15 (of 24 total)
You must be logged in to reply to this topic. Login to reply