2
\$\begingroup\$

These IDs seem like they should be a table as, there are several queries that use the same list of IDs. The following needs to be refactored.

SELECT SUM(CASE WHEN s.TypeID IN(1,2,3,4,7,8,10) THEN 1 ELSE 0 END) FROM Sales s

The problem is a subquery can't be used in an aggregate function. I'd like to write it as follows, but it causes an error:

SELECT SUM(CASE WHEN s.TypeID IN(SELECT ID FROM SaleTypes WHERE SaleType.Desc = 'US') THEN 1 ELSE 0 END) FROM Sales s
\$\endgroup\$
1
  • \$\begingroup\$ If this code is in an aggregate function, it might be better to include the whole CREATE FUNCTION code. \$\endgroup\$ Sep 17, 2014 at 18:36

3 Answers 3

4
\$\begingroup\$

I'm assuming the poster pasted just the relevant snippet and this is merely one column of many. For that purpose, I would prefer an outer join.

SELECT SUM(CASE WHEN st.ID IS NOT NULL THEN 1 ELSE 0 END)
  FROM Sales s
  LEFT JOIN SaleTypes st
         ON (s.TypeId=st.ID AND st.Desc='US')
\$\endgroup\$
4
  • \$\begingroup\$ Very observant :-) the example I gave was incredibly simple, there are several counted columns in the query. \$\endgroup\$
    – user30586
    Sep 18, 2014 at 12:53
  • \$\begingroup\$ All good solutions. I guess its not possible to get this as concise as I would like. Thanks. \$\endgroup\$
    – user30586
    Sep 18, 2014 at 15:50
  • \$\begingroup\$ A convoluted sum(case when x then 1 else 0) is far inferior to a simple count. \$\endgroup\$
    – janos
    Sep 19, 2014 at 7:26
  • \$\begingroup\$ @janos Mmm, I'm not sure "SELECT count(st.ID)" and the implicit null check is safer. Tends to get lost in the maintenance and replaced by "SELECT count(*)." \$\endgroup\$
    – DKATyler
    Sep 22, 2014 at 18:17
5
\$\begingroup\$

Inside your SUM, each record has a value of either 1 or 0. The value is 1 if some condition is true for that record, and 0 if false. In other words, you're really counting the records where the condition is true.

The condition appears to be, Sales.TypeID should be one of 1,2,3,4,7,8,10. And if I understood it correctly, these correspond to the ids of the SaleType records where SaleType.Desc = 'US'.

It sounds like you want to join the Sales and SaleTypes tables, where Sales.TypeID match SaleTypes.ID, and SaleType.Desc = 'US'. So this query:

SELECT count(*) 
FROM Sales s 
JOIN SaleTypes st 
ON s.TypeID = st.ID 
WHERE SaleTypes.Desc = 'US'
\$\endgroup\$
4
\$\begingroup\$

Nitpicks

SQL code (like pretty much all code) reads a lot easier when using line breaks and indentation.


s is not a very good alias. You want your aliases to say something about what it means, not just shorten the code. In this case I would not even use one, I feel Sales is plenty short.


Your code so far:

SELECT SUM(
    CASE WHEN Sales.TypeID IN(1,2,3,4,7,8,10) THEN 1 
    ELSE 0 
    END
) 
FROM Sales;

Common Table Expression

You could organize the code a little better by using a CTE, although it is a bit more verbose:

WITH SalesInUS AS(
    SELECT ID
    FROM SalesType
    WHERE SalesType.Desc = 'US'
),

SELECT SUM(
    CASE WHEN Sales.TypeID IN SalesInUS THEN 1 
    ELSE 0 
    END
) 
FROM Sales;

But if this is often referenced, you are correct: a table would work better.

Example:

INSERT INTO #SalesInUS
    SELECT ID
    FROM SalesType
    WHERE SalesType.Desc = 'US'

Then you just join that table when you need it. Note I changed SUM to COUNT as well.

SELECT COUNT(Sales.TypeID)
FROM Sales 
INNER JOIN #SalesInUS
ON Sales.TypeID = #SalesInUS.ID

Alternatively, you could also use a simple JOIN

SELECT COUNT(Sales.TypeID)
FROM Sales 
INNER JOIN SalesType
ON Sales.TypeID = SalesType.ID
AND SalesType.Desc = 'US'
\$\endgroup\$
2
  • \$\begingroup\$ CASE WHEN Sales.TypeID IN SalesInUS THEN 1 is not valid SQL. Would need to be CASE WHEN Sales.TypeID IN (Select id from SalesInUs) THEN 1 Which is back to a subquery. \$\endgroup\$ Sep 17, 2014 at 22:53
  • \$\begingroup\$ Good catch. I crossed out that section. Thanks for pointing this out. \$\endgroup\$
    – Phrancis
    Sep 18, 2014 at 1:36

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.