Skip to main content
added 61 characters in body
Source Link
Mathieu Guindon
  • 75.6k
  • 18
  • 195
  • 469

The size of the horizontal scrollbar is the first smell. I'd start by breaking down the CASE WHEN's into multiple lines:

SELECT
    ROW_NUMBER() OVER(ORDER BY (CONVERT(varchar, effectivedate, 102)) ASC) Rn,
    CONVERT(varchar, effectivedate, 102) effectivedate,
    SUM(EuroCurrentBalance/1000000) TotalArrears,
    SUM(CASE WHEN Multiplier > 2 THEN EuroCurrentBalance/1000000 
             ELSE 0 
        END) NPLs,
    CASE WHEN ROW_NUMBER() OVER(ORDER BY (CONVERT(varchar, effectivedate, 102)) ASC) = 1 THEN 0 
         ELSE SUM(EuroCurrentBalance/1000000) - (LAG(SUM(EuroCurrentBalance/1000000),1,0) OVER (ORDER BY (CONVERT(varchar, effectivedate, 102)))) 
    END MonthlyChange
FROM
    DataWarehouse.dwa.FinanceMonthEndData
GROUP BY
    CONVERT(varchar, effectivedate, 102)
GO

Note that you can group by effectivedate without doing the conversion again.


Then I'd look at the redundant pieces: EuroCurrentBalance/1000000 is repeated quite often, and CONVERT(varchar, effectivedate, 102) as well. Consider a subquery, or a CTE:

WITH PreSelectionCTE
AS
(
    SELECT 
        CONVERT(varchar, effectivedate, 102) EffectiveDate,
        EuroCurrentBalance/1000000 Arrear,
        CASE WHEN Multiplier > 2 THEN EuroCurrentBalance/1000000
             ELSE 0
             END NPL
    FROM
        DataWarehouse.dwa.FinanceMonthEndData
)
SELECT
    ROW_NUMBER() OVER(ORDER BY EffectiveDate) Rn,
    SUM(Arrear) TotalArrears,
    SUM(NPL) NPLs,
    CASE WHEN ROW_NUMBER() OVER(ORDER BY (EffectiveDate)) = 1 THEN 0
         ELSE SUM(Arrear) - (LAG(Arrear,1,0) OVER(ORDER BY EffectiveDate))
         END MonthlyChange
FROM
    PreSelectionCTE
GROUP BY
    EffectiveDate

GO

This could probably be improved further, but at least it's much easier to read now! Note that ASC is implicit in an ORDER BY - I'd simply omit it, or specify it consistently: you have it explicit in 2 of your 3 ORDER BY's.

The size of the horizontal scrollbar is the first smell. I'd start by breaking down the CASE WHEN's into multiple lines:

SELECT
    ROW_NUMBER() OVER(ORDER BY (CONVERT(varchar, effectivedate, 102)) ASC) Rn,
    CONVERT(varchar, effectivedate, 102) effectivedate,
    SUM(EuroCurrentBalance/1000000) TotalArrears,
    SUM(CASE WHEN Multiplier > 2 THEN EuroCurrentBalance/1000000 
             ELSE 0 
        END) NPLs,
    CASE WHEN ROW_NUMBER() OVER(ORDER BY (CONVERT(varchar, effectivedate, 102)) ASC) = 1 THEN 0 
         ELSE SUM(EuroCurrentBalance/1000000) - (LAG(SUM(EuroCurrentBalance/1000000),1,0) OVER (ORDER BY (CONVERT(varchar, effectivedate, 102)))) 
    END MonthlyChange
FROM
    DataWarehouse.dwa.FinanceMonthEndData
GROUP BY
    CONVERT(varchar, effectivedate, 102)
GO

Then I'd look at the redundant pieces: EuroCurrentBalance/1000000 is repeated quite often, and CONVERT(varchar, effectivedate, 102) as well. Consider a subquery, or a CTE:

WITH PreSelectionCTE
AS
(
    SELECT 
        CONVERT(varchar, effectivedate, 102) EffectiveDate,
        EuroCurrentBalance/1000000 Arrear,
        CASE WHEN Multiplier > 2 THEN EuroCurrentBalance/1000000
             ELSE 0
             END NPL
    FROM
        DataWarehouse.dwa.FinanceMonthEndData
)
SELECT
    ROW_NUMBER() OVER(ORDER BY EffectiveDate) Rn,
    SUM(Arrear) TotalArrears,
    SUM(NPL) NPLs,
    CASE WHEN ROW_NUMBER() OVER(ORDER BY (EffectiveDate)) = 1 THEN 0
         ELSE SUM(Arrear) - (LAG(Arrear,1,0) OVER(ORDER BY EffectiveDate))
         END MonthlyChange
FROM
    PreSelectionCTE
GROUP BY
    EffectiveDate

GO

This could probably be improved further, but at least it's much easier to read now! Note that ASC is implicit in an ORDER BY - I'd simply omit it, or specify it consistently: you have it explicit in 2 of your 3 ORDER BY's.

The size of the horizontal scrollbar is the first smell. I'd start by breaking down the CASE WHEN's into multiple lines:

SELECT
    ROW_NUMBER() OVER(ORDER BY (CONVERT(varchar, effectivedate, 102)) ASC) Rn,
    CONVERT(varchar, effectivedate, 102) effectivedate,
    SUM(EuroCurrentBalance/1000000) TotalArrears,
    SUM(CASE WHEN Multiplier > 2 THEN EuroCurrentBalance/1000000 
             ELSE 0 
        END) NPLs,
    CASE WHEN ROW_NUMBER() OVER(ORDER BY (CONVERT(varchar, effectivedate, 102)) ASC) = 1 THEN 0 
         ELSE SUM(EuroCurrentBalance/1000000) - (LAG(SUM(EuroCurrentBalance/1000000),1,0) OVER (ORDER BY (CONVERT(varchar, effectivedate, 102)))) 
    END MonthlyChange
FROM
    DataWarehouse.dwa.FinanceMonthEndData
GROUP BY
    effectivedate
GO

Note that you can group by effectivedate without doing the conversion again.


Then I'd look at the redundant pieces: EuroCurrentBalance/1000000 is repeated quite often, and CONVERT(varchar, effectivedate, 102) as well. Consider a subquery, or a CTE:

WITH PreSelectionCTE
AS
(
    SELECT 
        CONVERT(varchar, effectivedate, 102) EffectiveDate,
        EuroCurrentBalance/1000000 Arrear,
        CASE WHEN Multiplier > 2 THEN EuroCurrentBalance/1000000
             ELSE 0
             END NPL
    FROM
        DataWarehouse.dwa.FinanceMonthEndData
)
SELECT
    ROW_NUMBER() OVER(ORDER BY EffectiveDate) Rn,
    SUM(Arrear) TotalArrears,
    SUM(NPL) NPLs,
    CASE WHEN ROW_NUMBER() OVER(ORDER BY (EffectiveDate)) = 1 THEN 0
         ELSE SUM(Arrear) - (LAG(Arrear,1,0) OVER(ORDER BY EffectiveDate))
         END MonthlyChange
FROM
    PreSelectionCTE
GROUP BY
    EffectiveDate

GO

This could probably be improved further, but at least it's much easier to read now! Note that ASC is implicit in an ORDER BY - I'd simply omit it, or specify it consistently: you have it explicit in 2 of your 3 ORDER BY's.

Source Link
Mathieu Guindon
  • 75.6k
  • 18
  • 195
  • 469

The size of the horizontal scrollbar is the first smell. I'd start by breaking down the CASE WHEN's into multiple lines:

SELECT
    ROW_NUMBER() OVER(ORDER BY (CONVERT(varchar, effectivedate, 102)) ASC) Rn,
    CONVERT(varchar, effectivedate, 102) effectivedate,
    SUM(EuroCurrentBalance/1000000) TotalArrears,
    SUM(CASE WHEN Multiplier > 2 THEN EuroCurrentBalance/1000000 
             ELSE 0 
        END) NPLs,
    CASE WHEN ROW_NUMBER() OVER(ORDER BY (CONVERT(varchar, effectivedate, 102)) ASC) = 1 THEN 0 
         ELSE SUM(EuroCurrentBalance/1000000) - (LAG(SUM(EuroCurrentBalance/1000000),1,0) OVER (ORDER BY (CONVERT(varchar, effectivedate, 102)))) 
    END MonthlyChange
FROM
    DataWarehouse.dwa.FinanceMonthEndData
GROUP BY
    CONVERT(varchar, effectivedate, 102)
GO

Then I'd look at the redundant pieces: EuroCurrentBalance/1000000 is repeated quite often, and CONVERT(varchar, effectivedate, 102) as well. Consider a subquery, or a CTE:

WITH PreSelectionCTE
AS
(
    SELECT 
        CONVERT(varchar, effectivedate, 102) EffectiveDate,
        EuroCurrentBalance/1000000 Arrear,
        CASE WHEN Multiplier > 2 THEN EuroCurrentBalance/1000000
             ELSE 0
             END NPL
    FROM
        DataWarehouse.dwa.FinanceMonthEndData
)
SELECT
    ROW_NUMBER() OVER(ORDER BY EffectiveDate) Rn,
    SUM(Arrear) TotalArrears,
    SUM(NPL) NPLs,
    CASE WHEN ROW_NUMBER() OVER(ORDER BY (EffectiveDate)) = 1 THEN 0
         ELSE SUM(Arrear) - (LAG(Arrear,1,0) OVER(ORDER BY EffectiveDate))
         END MonthlyChange
FROM
    PreSelectionCTE
GROUP BY
    EffectiveDate

GO

This could probably be improved further, but at least it's much easier to read now! Note that ASC is implicit in an ORDER BY - I'd simply omit it, or specify it consistently: you have it explicit in 2 of your 3 ORDER BY's.