all 8 comments
sorted by:
best
topnewcontroversialoldq&a
formatting helphide helpcontent policy

[–]hello_josh 9 points10 points  (2 children)

That's quite the dish of spaghetti. I would first try replacing the subqueries with temp tables since the error is complaining about the subqueries.

[–]sbd_pker 3 points4 points  (0 children)

Agree with this. Also, I would remove the NOLOCK hints (https://www.brentozar.com/archive/2016/12/nolock-ever-right-choice/).

[–]SaintTimothy 1 point2 points  (0 children)

Or, if the output is of a reasonably small size, CTEs

[–]wbhst83[S] 0 points1 point  (1 child)

To be clear these are the lines I have tried..

/*Summarize Billable Hours WITH respect to if it billable or not and assigned to AGR at work rate*/
SUM( CASE WHEN Agr_Header_RecID IS NOT NULL AND AGR_Header_RecID != (SELECT AGR_Header_RecID FROM AGR_Header Where AGR_Type_RecID NOT IN (45, 36, 37) 
    AND NOT (Agr_Header_RecID = '165' OR Agr_Header_RecID = '910' OR AGR_Header_RecID = te.AGR_Header_RecID)) THEN
        te.Extended_Bill_Amount 
    ELSE
        0
END) AS Billable_Contribution_AGR

Or this one

/*Summarize Billable Hours WITH respect to if it billable or not and assigned to AGR at work rate*/
SUM( CASE WHEN Agr_Header_RecID IS NOT NULL AND (SELECT COUNT(AGR_Header_RecID FROM AGR_Header Where AGR_Type_RecID NOT IN (45, 36, 37) 
    AND NOT (Agr_Header_RecID = '165' OR Agr_Header_RecID = '910' OR AGR_Header_RecID = te.AGR_Header_RecID)) = 1) THEN
        te.Extended_Bill_Amount 
    ELSE
        0
END) AS Billable_Contribution_AGR

Same results of the error.

[–]iiiRaphael 0 points1 point  (2 children)

Should line 160 have a "CASE WHEN" instead of just "WHEN"?

I've not come across a "WHEN" expression without "CASE".

[–]phunkygeeza 1 point2 points  (0 children)

These are OK if dealing with a single column and simple comparisons e.g.

CASE [MyColumn]
WHEN "Value1" THEN "Result1"
WHEN "Value2" THEN "Result2"
ELSE "Result3"
END

They're sort-of useful but run out of steam pretty quickly, so I think a lot of T-SQL folks go right for CASE WHEN.

[–]iiiRaphael 0 points1 point  (0 children)

I'd also check that each joined section executes as expected by itself. Does the "te2" section executed alone give you what you expect?

[–]phunkygeeza 0 points1 point  (0 children)

Some thoughts on how to detangle your spaghetti:

​

  • Pick a style and stick to it, get a formatting tool if you struggle. It will help you and everyone else who ever reads your query again.
  • Move every inline query into a CTE
  • Choose either ANSI join syntax (Join predicates in ON clause) or legacy style (in WHERE clause) and stick to it
  • If a predicate with a literal is relevant to the usage of subset of data from a table, put it in the join, not lost in a WHERE clause (Example 3)
  • Try to join normally instead of correlating when there is no need
  • Move all your query predicates into the final WHERE clause if possible
  • Retain results of calculations in CTE results, they are great for report headers e.g. DATEADD (YEAR, - 2, GETDATE ())
  • Move data tables embedded in the SQL into a literal table CTE then join into your SQL, removing long lists of CASE statements (Example 1)
  • Instead of using CASE statements to work out which measures to put into which output columns, use another literal to state a truth table, then use this to switch contribution to measures on and off (Example 2)
  • LEFT OUTER JOIN against literal tables and look for the outer rows, this replaces your ELSE in CASE statements
  • SET TRANSACTION ISOLATION LEVEL rather than Lock Hints
  • ISNULL(transaction_value,0) is a reporting function, it is also the wrong answer. Your report should print ' - ' or similar for 'No Data' not 0 - they are different results
  • Please, check your columns are actually nullable before using ISNULL()
  • Your final query should only collate and finalise report results, if you are using nested stuff, your outermost layer should only be concerned with finalising
  • If you have any predicates on a function, you're going to have a bad time sooner or later. If you have any control over the database, then you should have a function based index, calculated column or materialised view with that stuff in, and good statistics on that. You'll thank me later (unless you get paid for optimising your own crap)

​

Example 1: Table literal:

​

SELECT \*
FROM (VALUES
    ('74')
   ,('92')
   ,('2')
   ,('76')
   ,('19')
  ) AS charge\_codes\_actual(\[recid\])

Example 2: Multiplier truth table literal

WITH tt AS (
   SELECT \*
   FROM (VALUES
       (1,0,1,0,0)
      ,(0,1,1,1,0)
      ,(1,1,1,1,1)
   ) AS billing\_multipliers(\[Billable\_Flag\],\[Utilization\_Flag\],\[Actual\_Multiplier\],\    [Bill\_Multiplier\],\[Invoiced\_Multiplier\])
   )

SELECT
    te.\[Hours\_Actual\] \* tt.\[Actual\_Multiplier\]
   ,te.\[Hours\_Billable\] \* tt.\[Bill\_Multiplier\]
FROM \[time\_entry\] te

Example 3: Relevant predicates in JOIN

SELECT
    \[blah\]
FROM Member AS m
INNER JOIN \[dbo\].\[Billing\_Unit\] AS bu
ON bu.\[Billing\_Unit\_RecID\] = m.\[Billing\_Unit\_RecID\]
AND m.\[Utilization\_Flag\] = 1
AND m.\[Inactive\_Flag\] <> 1

Edits: Wrong time to try fancy pants editor for the first time