3
\$\begingroup\$

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
\$\endgroup\$
7
  • 1
    \$\begingroup\$ 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'. \$\endgroup\$
    – Leonid
    Commented Sep 14, 2012 at 21:10
  • \$\begingroup\$ @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. \$\endgroup\$
    – MAW74656
    Commented Sep 14, 2012 at 21:12
  • \$\begingroup\$ 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. \$\endgroup\$
    – Leonid
    Commented Sep 14, 2012 at 21:17
  • \$\begingroup\$ Well these are cost, so 0 makes sense. \$\endgroup\$
    – MAW74656
    Commented Sep 14, 2012 at 21:23
  • \$\begingroup\$ 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. \$\endgroup\$
    – Leonid
    Commented Sep 14, 2012 at 21:43

1 Answer 1

Reset to default
1
\$\begingroup\$

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.

\$\endgroup\$
2
  • \$\begingroup\$ Quite right, not a proc at all. I meant to say "query". Whats the main advantage of using a CTE here? \$\endgroup\$
    – MAW74656
    Commented Sep 17, 2012 at 14:48
  • 2
    \$\begingroup\$ 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. \$\endgroup\$
    – jmoreno
    Commented Sep 17, 2012 at 15:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.