Ryan
Ryan

Reputation: 672

Best practice for complex TSQL view

I am writing a complex View for a report below:

SELECT     TOP (100) PERCENT dbo.tbl_SFCWRK_SQL_NF.ID, dbo.tbl_SFCWRK_SQL_NF.STD_LBR_HRS, dbo.tbl_SFCWRK_SQL_NF.CUSTNAME, 
                  dbo.tbl_SFCWRK_SQL_NF.JOBNBR, CASE WHEN dbo.tbl_SFCWRK_SQL_NF.COMPQTY IS NULL THEN 0 ELSE CONVERT(decimal(10, 5), 
                  dbo.tbl_SFCWRK_SQL_NF.COMPQTY) END AS COMPQTY, CASE WHEN dbo.tbl_SFCWRK_SQL_NF.ORDQTY IS NULL THEN 0 ELSE CONVERT(decimal(10, 5), 
                  dbo.tbl_SFCWRK_SQL_NF.ORDQTY) END AS ORDQTY, dbo.tbl_SFCWRK_SQL_NF.PLAN_CD, dbo.tbl_SFCWRK_SQL_NF.PLANQTY, 
                  dbo.tbl_SFCWRK_SQL_NF.ITEMNBR, dbo.tbl_ITEMMAST_NF.ITEM_NBR, dbo.tbl_SFCWRK_SQL_NF.WODUEDT, tbl_WORK_CENTR_NF.WRK_CENTER_DESC, 
                  tbl_WORK_CENTR_NF.LABOR_CAPACITY, dbo.tbl_SFCWRK_SQL_NF.STD_SETUP_HRS, dbo.tbl_WIPOPER_NF.STD_LBR_HOURS, 
                  dbo.tbl_WIPOPER_NF.SCHED_COMP_DATE, dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR, dbo.tbl_WIPOPER_NF.OPER_STATUS, 
                  dbo.tbl_WIPOPER_NF.OPERATION_NBR, tbl_WORK_CENTR_NF.WAREHOUSE, 
                  CASE WHEN dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20005' THEN 1 WHEN dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20010' THEN 2 WHEN dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR
                   = '20020' THEN 3 WHEN dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20030' THEN 4 WHEN dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20040' THEN 5 WHEN
                   dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20050' THEN 6 WHEN dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20110' THEN 7 ELSE dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR
                   END AS WCOrder, DATEADD(dd, 7 - DATEPART(dw, dbo.tbl_WIPOPER_NF.SCHED_COMP_DATE), dbo.tbl_WIPOPER_NF.SCHED_COMP_DATE) AS WeekEnd, 
                  DATEPART(wk, dbo.tbl_WIPOPER_NF.SCHED_COMP_DATE) AS WkNum, 
                  CASE WHEN dbo.tbl_SFCWRK_SQL_NF.COMPQTY <> 0 THEN ((dbo.tbl_SFCWRK_SQL_NF.STD_LBR_HRS / dbo.tbl_SFCWRK_SQL_NF.ORDQTY) 
                  * (dbo.tbl_SFCWRK_SQL_NF.ORDQTY - dbo.tbl_SFCWRK_SQL_NF.COMPQTY)) 
                  * 2 ELSE ((dbo.tbl_SFCWRK_SQL_NF.STD_LBR_HRS / dbo.tbl_SFCWRK_SQL_NF.ORDQTY) * (dbo.tbl_SFCWRK_SQL_NF.ORDQTY)) * 2 END AS RmnLabor, 
                  CASE WHEN dbo.tbl_SFCWRK_SQL_NF.COMPQTY <> 0 THEN ((dbo.tbl_SFCWRK_SQL_NF.STD_SETUP_HRS / tbl_SFCWRK_SQL_NF.ORDQTY) 
                  * (dbo.tbl_SFCWRK_SQL_NF.ORDQTY - dbo.tbl_SFCWRK_SQL_NF.COMPQTY)) 
                  / 100 ELSE ((dbo.tbl_SFCWRK_SQL_NF.STD_SETUP_HRS / dbo.tbl_SFCWRK_SQL_NF.ORDQTY) * (dbo.tbl_SFCWRK_SQL_NF.ORDQTY)) 
                  / 100 END AS RmnSetup
FROM         dbo.tbl_ITEMMAST_NF INNER JOIN
                  dbo.tbl_SFCWRK_SQL_NF ON dbo.tbl_ITEMMAST_NF.CPN =  dbo.tbl_SFCWRK_SQL_NF.CPN_NO INNER JOIN
                  dbo.tbl_WORK_CENTR_NF AS tbl_WORK_CENTR_NF ON dbo.tbl_SFCWRK_SQL_NF.WC_NBR = tbl_WORK_CENTR_NF.ID INNER JOIN
                  dbo.tbl_WIPOPER_NF ON dbo.tbl_SFCWRK_SQL_NF.ID = dbo.tbl_WIPOPER_NF.ID
WHERE     (dbo.tbl_WIPOPER_NF.SCHED_COMP_DATE <= DATEADD(m, 2, GETDATE())) AND (dbo.tbl_WIPOPER_NF.OPER_STATUS <> 'C' OR
                  dbo.tbl_WIPOPER_NF.OPER_STATUS IS NULL OR
                  dbo.tbl_WIPOPER_NF.OPER_STATUS = '') AND (tbl_WORK_CENTR_NF.WAREHOUSE = '02') AND (dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR IN ('20005', '20010', 
                  '20020', '20030', '20040', '20060', '20110')) AND (NOT (dbo.tbl_ITEMMAST_NF.ITEM_NBR LIKE 'A%')) AND (CASE WHEN tbl_SFCWRK_SQL_NF.ORDQTY IS NULL 
                  THEN 0 ELSE CONVERT(decimal(10, 5), tbl_SFCWRK_SQL_NF.ORDQTY) END <> 0) AND (CASE WHEN tbl_SFCWRK_SQL_NF.ORDQTY IS NULL 
                  THEN 0 ELSE CONVERT(decimal(10, 5), tbl_SFCWRK_SQL_NF.ORDQTY) END <> 0)
ORDER BY dbo.tbl_SFCWRK_SQL_NF.WODUEDT DESC

I would like to make this run more efficient. I am also curious if what I am doing is the proper way to write the View and if I should refactor it...??

Ryan

Upvotes: 0

Views: 468

Answers (3)

paparazzo
paparazzo

Reputation: 45096

This just makes no sense to me

where      
(CASE 
      WHEN tbl_SFCWRK_SQL_NF.ORDQTY IS NULL THEN 0 
      ELSE CONVERT(decimal(10, 5), tbl_SFCWRK_SQL_NF.ORDQTY) END 
       <> 0)

If it is null then it returns false.
But any comparison to null returns false.

It has to be a numeric type for convert to decimal to succeed.
With any numeric type <> 0 is the same.

Pretty sure that can be reduced to

where tbl_SFCWRK_SQL_NF.ORDQTY <> 0

With your syntax you kill the use of an index on that column

Also <> 'c' or = '' is redundant

Saw comment that tbl_SFCWRK_SQL_NF.ORDQTY is varchar.
That is just messed up.
That convert will fail if any varchar value is not decimal.
I just assumed it had to be some type of numeric.

Upvotes: 2

JoeFletch
JoeFletch

Reputation: 3960

I made some minor updates and formatting to the query that may be helpful; checking for IS NULL and empty string by using COALESCE instead of with CASE statements. See below for details. I'm not sure if these will affect performance, but you can try it.

Please note that your last criteria was listed the twice. I'm not sure if that was intentional or not or if it will even make a difference in performance. Let me know. If not, then I will delete this answer.

SELECT
    dbo.tbl_SFCWRK_SQL_NF.ID,
    dbo.tbl_SFCWRK_SQL_NF.STD_LBR_HRS,
    dbo.tbl_SFCWRK_SQL_NF.CUSTNAME,
    dbo.tbl_SFCWRK_SQL_NF.JOBNBR,
    --CASE WHEN dbo.tbl_SFCWRK_SQL_NF.COMPQTY IS NULL THEN 0 ELSE CONVERT(decimal(10,   5), dbo.tbl_SFCWRK_SQL_NF.COMPQTY) END AS COMPQTY,
    COALESCE(CONVERT(decimal(10, 5), dbo.tbl_SFCWRK_SQL_NF.COMPQTY), 0) AS COMPQTY
    --CASE WHEN dbo.tbl_SFCWRK_SQL_NF.ORDQTY IS NULL THEN 0 ELSE CONVERT(decimal(10,    5), dbo.tbl_SFCWRK_SQL_NF.ORDQTY) END AS ORDQTY,
    COALESCE(CONVERT(decimal(10, 5), dbo.tbl_SFCWRK_SQL_NF.ORDQTY), 0) AS ORDQTY
    dbo.tbl_SFCWRK_SQL_NF.PLAN_CD,
    dbo.tbl_SFCWRK_SQL_NF.PLANQTY,
    dbo.tbl_SFCWRK_SQL_NF.ITEMNBR,
    dbo.tbl_ITEMMAST_NF.ITEM_NBR,
    dbo.tbl_SFCWRK_SQL_NF.WODUEDT,
    tbl_WORK_CENTR_NF.WRK_CENTER_DESC,
    tbl_WORK_CENTR_NF.LABOR_CAPACITY,
    dbo.tbl_SFCWRK_SQL_NF.STD_SETUP_HRS,
    dbo.tbl_WIPOPER_NF.STD_LBR_HOURS,
    dbo.tbl_WIPOPER_NF.SCHED_COMP_DATE,
    dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR,
    dbo.tbl_WIPOPER_NF.OPER_STATUS,
    dbo.tbl_WIPOPER_NF.OPERATION_NBR,
    tbl_WORK_CENTR_NF.WAREHOUSE,
    CASE
        WHEN
            dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20005'
        THEN
            1
        WHEN
            dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20010'
        THEN
            2
        WHEN
            dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR  = '20020'
        THEN
            3
        WHEN
            dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20030'
        THEN
            4
        WHEN
            dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20040'
        THEN
            5
        WHEN
            dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20050'
        THEN
            6
        WHEN
            dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20110'
        THEN
            7
        ELSE
            dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR
        END
            AS WCOrder,
    DATEADD(dd, 7 - DATEPART(dw, dbo.tbl_WIPOPER_NF.SCHED_COMP_DATE), dbo.tbl_WIPOPER_NF.SCHED_COMP_DATE) AS WeekEnd,
    DATEPART(wk, dbo.tbl_WIPOPER_NF.SCHED_COMP_DATE) AS WkNum,
    CASE
        WHEN
            dbo.tbl_SFCWRK_SQL_NF.COMPQTY <> 0
        THEN
            ((dbo.tbl_SFCWRK_SQL_NF.STD_LBR_HRS / dbo.tbl_SFCWRK_SQL_NF.ORDQTY) * (dbo.tbl_SFCWRK_SQL_NF.ORDQTY - dbo.tbl_SFCWRK_SQL_NF.COMPQTY)) * 2
        ELSE
            ((dbo.tbl_SFCWRK_SQL_NF.STD_LBR_HRS / dbo.tbl_SFCWRK_SQL_NF.ORDQTY) * (dbo.tbl_SFCWRK_SQL_NF.ORDQTY)) * 2
        END AS RmnLabor,
    CASE
        WHEN
            dbo.tbl_SFCWRK_SQL_NF.COMPQTY <> 0
        THEN
            ((dbo.tbl_SFCWRK_SQL_NF.STD_SETUP_HRS / tbl_SFCWRK_SQL_NF.ORDQTY) * (dbo.tbl_SFCWRK_SQL_NF.ORDQTY - dbo.tbl_SFCWRK_SQL_NF.COMPQTY)) / 100
        ELSE
            ((dbo.tbl_SFCWRK_SQL_NF.STD_SETUP_HRS / dbo.tbl_SFCWRK_SQL_NF.ORDQTY) * (dbo.tbl_SFCWRK_SQL_NF.ORDQTY)) / 100
        END AS RmnSetup
FROM
    dbo.tbl_ITEMMAST_NF INNER JOIN dbo.tbl_SFCWRK_SQL_NF
        ON
            dbo.tbl_ITEMMAST_NF.CPN = dbo.tbl_SFCWRK_SQL_NF.CPN_NO INNER JOIN dbo.tbl_WORK_CENTR_NF AS tbl_WORK_CENTR_NF
                ON
                    dbo.tbl_SFCWRK_SQL_NF.WC_NBR = tbl_WORK_CENTR_NF.ID INNER JOIN dbo.tbl_WIPOPER_NF
                        ON
                            dbo.tbl_SFCWRK_SQL_NF.ID = dbo.tbl_WIPOPER_NF.ID
WHERE
    dbo.tbl_WIPOPER_NF.SCHED_COMP_DATE <= DATEADD(m, 2, GETDATE())
        AND
    (
        dbo.tbl_WIPOPER_NF.OPER_STATUS <> 'C'
            OR
        --dbo.tbl_WIPOPER_NF.OPER_STATUS IS NULL OR dbo.tbl_WIPOPER_NF.OPER_STATUS = ''
        --Replace the above statement with the following...
        COALESCE(dbo.tbl_WIPOPER_NF.OPER_STATUS, N'') = N''
    )
        AND
    tbl_WORK_CENTR_NF.WAREHOUSE = '02'
        AND
    dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR IN ('20005', '20010', '20020', '20030', '20040', '20060','20110')
        AND
    dbo.tbl_ITEMMAST_NF.ITEM_NBR NOT LIKE 'A%'
        AND
    --CASE WHEN tbl_SFCWRK_SQL_NF.ORDQTY IS NULL THEN 0 ELSE CONVERT(decimal(10, 5), tbl_SFCWRK_SQL_NF.ORDQTY) END <> 0
    --I would change the above statement to this...
    COALESCE(tbl_SFCWRK_SQL_NF.ORDQTY, 0) <> 0

        --AND
    --CASE WHEN tbl_SFCWRK_SQL_NF.ORDQTY IS NULL THEN 0 ELSE CONVERT(decimal(10, 5), tbl_SFCWRK_SQL_NF.ORDQTY) END <> 0
    --The above statement is a duplicate!
ORDER BY
    dbo.tbl_SFCWRK_SQL_NF.WODUEDT DESC

Upvotes: 1

Kenneth Fisher
Kenneth Fisher

Reputation: 3812

First, best practice on views is not to include an ORDER BY. So get rid of the TOP (100) PERCENT and the ORDER BY dbo.tbl_SFCWRK_SQL_NF.WODUEDT DESC statements. Next format your query so it's easily readable. I've done this below, but if you have a different format you prefer use that. It's all about making it readable for YOU and YOUR teammates/eventual replacement.

Having formatted it I can tell you that you should check on the following indexes.

  • tbl_ITEMMAST_NF(CPN, ITEM_NBR)
  • tbl_SFCWRK_SQL_NF(CPN_NO, WC_NBR, ID, ORDQTY)
  • tbl_WIPOPER_NF(ID, SCHED_COMP_DATE, OPER_STATUS)

These are a starting place based on the columns you are using in the JOINs and the WHERE clause. They may not be optimal but will probably be better than nothing. You might also check the execution plan and see if the optimizer suggests something. If it does review your existing indexes and see if it fits in with them, and if it does, then add it. If not it may be possible to modify one of your existing indexes to cover what it is asking for.

For example let's say you created the indexes above but the optimizer wants the following index.

tbl_WIPOPER_NF(ID, SCHED_COMP_DATE, OPER_STATUS) INCLUDE WORK_CENTER_NBR

Don't just add a new index. Modify the old one to include the INCLUDEed columns. Or the optimizer may want you to change the order of the columns in the index. If this is the only place you are using that index (you added it because I suggested it) then modify it. Don't add a new one. Same goes for my suggestions above when compared to the indexes you already have.

As far as the query itself. It looks fine. There are some other ways to handle things, for example using ISNULL instead of your CASE statements in the WHERE clause. But the way you have it should work just fine.

SELECT --TOP (100) PERCENT 
    dbo.tbl_SFCWRK_SQL_NF.ID, dbo.tbl_SFCWRK_SQL_NF.STD_LBR_HRS, 
    dbo.tbl_SFCWRK_SQL_NF.CUSTNAME, dbo.tbl_SFCWRK_SQL_NF.JOBNBR, 
    CASE WHEN dbo.tbl_SFCWRK_SQL_NF.COMPQTY IS NULL THEN 0 
        ELSE CONVERT(decimal(10, 5), dbo.tbl_SFCWRK_SQL_NF.COMPQTY) END AS COMPQTY, 
    CASE WHEN dbo.tbl_SFCWRK_SQL_NF.ORDQTY IS NULL THEN 0 
        ELSE CONVERT(decimal(10, 5), dbo.tbl_SFCWRK_SQL_NF.ORDQTY) END AS ORDQTY, 
    dbo.tbl_SFCWRK_SQL_NF.PLAN_CD, dbo.tbl_SFCWRK_SQL_NF.PLANQTY, 
    dbo.tbl_SFCWRK_SQL_NF.ITEMNBR, dbo.tbl_ITEMMAST_NF.ITEM_NBR, 
    dbo.tbl_SFCWRK_SQL_NF.WODUEDT, tbl_WORK_CENTR_NF.WRK_CENTER_DESC, 
    tbl_WORK_CENTR_NF.LABOR_CAPACITY, dbo.tbl_SFCWRK_SQL_NF.STD_SETUP_HRS, 
    dbo.tbl_WIPOPER_NF.STD_LBR_HOURS, dbo.tbl_WIPOPER_NF.SCHED_COMP_DATE, 
    dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR, dbo.tbl_WIPOPER_NF.OPER_STATUS, 
    dbo.tbl_WIPOPER_NF.OPERATION_NBR, tbl_WORK_CENTR_NF.WAREHOUSE, 
    CASE WHEN dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20005' THEN 1 
        WHEN dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20010' THEN 2 
        WHEN dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20020' THEN 3 
        WHEN dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20030' THEN 4 
        WHEN dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20040' THEN 5 
        WHEN dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20050' THEN 6 
        WHEN dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR = '20110' THEN 7 
        ELSE dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR END AS WCOrder, 
    DATEADD(dd, 7 - DATEPART(dw, dbo.tbl_WIPOPER_NF.SCHED_COMP_DATE), 
    dbo.tbl_WIPOPER_NF.SCHED_COMP_DATE) AS WeekEnd, DATEPART(wk, dbo.tbl_WIPOPER_NF.SCHED_COMP_DATE) AS WkNum, 
    CASE WHEN dbo.tbl_SFCWRK_SQL_NF.COMPQTY <> 0 
        THEN ((dbo.tbl_SFCWRK_SQL_NF.STD_LBR_HRS / dbo.tbl_SFCWRK_SQL_NF.ORDQTY) *
            (dbo.tbl_SFCWRK_SQL_NF.ORDQTY - dbo.tbl_SFCWRK_SQL_NF.COMPQTY)) * 2 
        ELSE ((dbo.tbl_SFCWRK_SQL_NF.STD_LBR_HRS / dbo.tbl_SFCWRK_SQL_NF.ORDQTY) * 
            (dbo.tbl_SFCWRK_SQL_NF.ORDQTY)) * 2 END AS RmnLabor, 
    CASE WHEN dbo.tbl_SFCWRK_SQL_NF.COMPQTY <> 0 
        THEN ((dbo.tbl_SFCWRK_SQL_NF.STD_SETUP_HRS / tbl_SFCWRK_SQL_NF.ORDQTY) * 
            (dbo.tbl_SFCWRK_SQL_NF.ORDQTY - dbo.tbl_SFCWRK_SQL_NF.COMPQTY)) / 100 
        ELSE ((dbo.tbl_SFCWRK_SQL_NF.STD_SETUP_HRS / dbo.tbl_SFCWRK_SQL_NF.ORDQTY) * 
            (dbo.tbl_SFCWRK_SQL_NF.ORDQTY)) / 100 END AS RmnSetup
FROM dbo.tbl_ITEMMAST_NF 
INNER JOIN dbo.tbl_SFCWRK_SQL_NF 
    ON dbo.tbl_ITEMMAST_NF.CPN =  dbo.tbl_SFCWRK_SQL_NF.CPN_NO 
INNER JOIN dbo.tbl_WORK_CENTR_NF AS tbl_WORK_CENTR_NF 
    ON dbo.tbl_SFCWRK_SQL_NF.WC_NBR = tbl_WORK_CENTR_NF.ID 
INNER JOIN dbo.tbl_WIPOPER_NF 
    ON dbo.tbl_SFCWRK_SQL_NF.ID = dbo.tbl_WIPOPER_NF.ID
WHERE (dbo.tbl_WIPOPER_NF.SCHED_COMP_DATE <= DATEADD(m, 2, GETDATE())) 
  AND (dbo.tbl_WIPOPER_NF.OPER_STATUS <> 'C' OR
        dbo.tbl_WIPOPER_NF.OPER_STATUS IS NULL OR
        dbo.tbl_WIPOPER_NF.OPER_STATUS = '') 
  AND (tbl_WORK_CENTR_NF.WAREHOUSE = '02') 
  AND (dbo.tbl_WIPOPER_NF.WORK_CENTER_NBR IN ('20005', '20010', '20020', '20030', 
                                                '20040', '20060', '20110')) 
  AND (NOT (dbo.tbl_ITEMMAST_NF.ITEM_NBR LIKE 'A%')) 
  AND (CASE WHEN tbl_SFCWRK_SQL_NF.ORDQTY IS NULL THEN 0 
        ELSE CONVERT(decimal(10, 5), tbl_SFCWRK_SQL_NF.ORDQTY) END <> 0) 
  AND (CASE WHEN tbl_SFCWRK_SQL_NF.ORDQTY IS NULL THEN 0 
        ELSE CONVERT(decimal(10, 5), tbl_SFCWRK_SQL_NF.ORDQTY) END <> 0)
--ORDER BY dbo.tbl_SFCWRK_SQL_NF.WODUEDT DESC

Upvotes: 1

Related Questions