1
\$\begingroup\$

While checking my SPs today I found that I have,

SELECT  COUNT(*)
,CASE WHEN ISNULL(MIN(COALESCE(MP.NewPrice, MP.Price, 2147483647)),0) = 2147483647 THEN 0 ELSE ISNULL(MIN(COALESCE(MP.NewPrice, MP.Price, 2147483647)), 0) END
FROM    MerchantsProducts MP

I need to replace this with something which make more sense. Can someone please review this?

\$\endgroup\$
  • 1
    \$\begingroup\$ What is the intended effect of this SQL query? \$\endgroup\$ – 200_success Jan 18 '15 at 12:28
  • \$\begingroup\$ @200_success, its the same as in the statemenrt \$\endgroup\$ – user960567 Jan 18 '15 at 13:52
5
\$\begingroup\$

COALESCE(…, …, 2147483647) will never return NULL. Furthermore, MIN(…) will only return NULL when all of its inputs are NULL.

Assuming that every NewPrice and every Price is at most 2147483647, the following query should be equivalent.

SELECT COUNT(*)
     , ISNULL(MIN(COALESCE(NewPrice, Price)), 0)
    FROM MerchantsProducts
\$\endgroup\$
  • \$\begingroup\$ What if NewPrice or Price or both will NULL? Will it work? \$\endgroup\$ – user960567 Jan 19 '15 at 7:18
  • \$\begingroup\$ Why don't you test it? \$\endgroup\$ – 200_success Jan 19 '15 at 7:19
1
\$\begingroup\$

Vertical Space

Any time I see a horizontal scrollbar, I start to ask myself where I can use more newlines. I would reformat your query like this to make it easier to read:

SELECT
    COUNT(*) AS [Number of Products],
    CASE
        WHEN ISNULL(MIN(COALESCE(MP.NewPrice, MP.Price, 2147483647)),0) = 2147483647
        THEN 0
        ELSE ISNULL(MIN(COALESCE(MP.NewPrice, MP.Price, 2147483647)), 0)
    END AS [Minimum Price]
FROM MerchantsProducts MP

I personally find it mush easier to edit a query if I can see the entire thing without having to scroll.

I also took the liberty of adding column aliases, to prevent (No Column Name) potentially being returned.

An alternate approach

As far as I can tell, this should be slightly more efficient than the answer given by 200_success.

SELECT 
    COUNT(*) AS [Number of Products],
    MIN(COALESCE(NewPrice, Price, 0)) AS [Minimum Price]
FROM MerchantsProducts

Looking at the execution plan generated by these simple tests reveals that this method doesn't have the Compute Scalar step, which makes it that little bit more efficient. when I say a little bit, I mean it. You probably won't notice any difference unless you are running the query against multiple hundreds of thousands of records.

Testing my approach:

 WITH TEST(newprice,price, count) AS
 (
    SELECT
        CAST(((2000 + 1) - 1) * RAND() + 1 AS INT),
        CAST(((2000 + 1) - 1) * RAND() + 1 AS INT),
        CAST(1 AS INT)
    UNION ALL

    SELECT 
        CAST(((2000 + 1) - 1) * RAND() + 1 AS INT),
        CAST(((2000 + 1) - 1) * RAND() + 1 AS INT),
        count+1
    FROM TEST
    WHERE count < 1000000
)

SELECT 
    MIN(COALESCE(MP.NewPrice, MP.Price,0)) AS [Minimum Price]
FROM TEST MP

OPTION (MAXRECURSION 0);

Testing 200_success' approach:

WITH TEST2(newprice,price, count) AS
(
    SELECT
        CAST(((2000 + 1) - 1) * RAND() + 1 AS INT),
        CAST(((2000 + 1) - 1) * RAND() + 1 AS INT),
        CAST(1 AS INT)
    UNION ALL

    SELECT 
        CAST(((2000 + 1) - 1) * RAND() + 1 AS INT),
        CAST(((2000 + 1) - 1) * RAND() + 1 AS INT),
        count+1
    FROM TEST2
    WHERE count < 1000000
)

SELECT 
    ISNULL(MIN(COALESCE(NewPrice, Price)), 0) AS [Minimum Price]
FROM TEST2 MP

OPTION (MAXRECURSION 0)

If you have already altered the query to use 200_sucess' approach then I would leave it as is, unless you regularly run the query against very large data sets (100000+ records).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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