SQL view, better way?

Question

I there a better way to write this query? I feel sure it can be done in one statement with joins, but I couldn't get it, and I'm afraid I've made it too slow. What suggestions do you have? I'm running it in SQL Server 2000 (yeah, I know, upgrade is coming).

Basically, I want to match estimated and actual costs, but sometimes the estimate is done on 1 cost center, and the actual costs are set to another cost center (hence the possibility of having null in est or act. I want to get all possible combinations for that job.

ALTER view [dbo].[JobCost_EstVsAct] --SELECT * FROM JobCost_EstVsAct Where jobnumber = '122773'
as
SELECT JobNumber, CostCenter, sum(Amount) as Est, sum(cost) as Act
FROM 
(
SELECT JobNumber, CostCenter, Amount, 0 as cost
FROM Avanti_ActiveJobBudgets AJB
UNION
SELECT jobnumber, costcentre as CostCenter, 0 as Est, cost as Act
FROM Avanti_ActiveCostDetails
) temp
--where cost + Amount > 0 This line is a bug
GROUP by JobNumber, CostCenter
HAVING sum(Amount) + sum(cost) > 0 --bug correction

• Leonid: Small remark: why are you doing something like '' as Est rather than null as Est? I think you are forcing an unnecessary cast. I do not think a join will help. Algorithmically your query is pretty fast; the only potential concern is the elimination of duplicates performed by UNION. If not for that, then your query would be potentially linear. 'Join' is not a magic keyword that makes everything fast; neither is 'index'.
• MAW74656: @Leonid -The elimination of duplicates makes sense in this dataset, so I'm not worried there. You think null as Est is better? I just thought I could do this same thing with a join that would not bring in the duplicates in the first place.
• Leonid: Try select '' union select 1; - you get 0 and 1. It converts '' to 0 which is surprising - might as well write 0 instead of ''. If you did want an empty string, then null is better (unless 0 makes even better sense). This way you can reuse this view for further computation or you can plug it into a report. Most reporting tools allow you to replace a null with whatever string or value you wish.
• MAW74656: Well these are cost, so 0 makes sense.
• Leonid: Ok, then do 0 as cost. I do not like having an implicit cast where one is not necessary. For example: this creates problems (but only at run time): create procedure foo as begin select 1 union select 'a' end. There is no compiler that will catch problems for you. It is a general principle of programming - keep things simple and readable. If you want 0 then type 0. Just because you have memorized the implicit cast table i.msdn.microsoft.com/dynimg/IC170617.gif does not mean that the next programmer will or should. Other than that it looks ok to me.

Answer 1

First off, it's not a stored procedure, it's a view.

That said, the main change I would make is to use a CTE, once you have upgraded.

ALTER view [dbo].[JobCost_EstVsAct]
 --SELECT * FROM JobCost_EstVsAct Where jobnumber = '122773'
AS
WITH cte1 as (
 SELECT JobNumber, CostCenter, Amount, 0 as cost
 FROM Avanti_ActiveJobBudgets AJB
),
cte2 as (
 SELECT jobnumber, costcentre as CostCenter, 0 as Amount, cost as Act
 FROM Avanti_ActiveCostDetails
),
cte3 as (
 Select * From cte1
 Union
 Select * From cte2
)
SELECT JobNumber, CostCenter, sum(Amount) as Est, sum(cost) as Act
FROM cte3
--where cost + Amount > 0   /* This line is a bug */
GROUP by JobNumber, CostCenter
HAVING sum(Amount) + sum(cost) > 0  --bug correction

Replacing the cteX with more appropriate names.

• MAW74656: Quite right, not a proc at all. I meant to say "query". Whats the main advantage of using a CTE here?
• jmoreno: The advantage in this case is readability, but IMO that is a significant advantage. It's also easier to work with as you are developing your query, switching between different parts of the final query as needed. You could even add a cte4 that does your group, and the looking at the different resultsets is dead easy.