Is this select field really necessary if its not l

2019-09-11 08:50发布

问题:

In our code stack I came across this stored proc:

ALTER PROCEDURE [dbo].[MoveNodes]
(
    @id bigint,
    @left bigint,
    @right bigint,
    @parentid bigint,
    @offset bigint,
    @caseid bigint,
    @userid bigint

)
AS
BEGIN
WITH    q AS
        (
        SELECT  id, parent, lft, rgt, title, type, caseid, userid, 0 AS level,
                CAST(LEFT(CAST(id AS VARCHAR) + REPLICATE('0', 10), 10) AS VARCHAR) AS bc
        FROM    [dbo].DM_FolderTree hc
        WHERE   id = @id and caseid = @caseid
        UNION ALL
        SELECT  hc.id, hc.parent, hc.lft, hc.rgt, hc.title, hc.type, hc.caseid, hc.userid, level + 1,
                CAST(bc + '.' + LEFT(CAST(hc.id AS VARCHAR) + REPLICATE('0', 10), 10) AS VARCHAR)
        FROM    q
        JOIN    [dbo].DM_FolderTree hc
        ON      hc.parent = q.id        
        )
UPDATE [dbo].DM_FolderTree 
    SET lft = ((-lft) + @offset), rgt = ((-rgt) + @offset), userid = @userid
    WHERE id in (select id from q) 
    AND lft <= (-(@left)) AND rgt >= (-(@right)) 
    AND caseid = @caseid;
UPDATE [dbo].DM_FolderTree 
    SET parent = @parentid, userid = @userid 
    WHERE id = @id AND caseid = @caseid;
END

Please, note the field labeled bc and how that variable is used below in the second CAST function, which we'll call for this discussion the unlabeled field, UF.

Two question pop out at me from the above.

  1. UF is not used anywhere in the CTE, q, other than to select the field...does it serve any function?
  2. Since bc is only used to generate UF, is bc necessary at all, either?

回答1:

bc is an alias in the "anchor" level, not a variable, of the recursive CTE. Basically, the ID being forced into a 10 character ID.

1 = 100000000
217 = 2170000000
etc.

In the recursive level of the cte, this ID, bc, is being concatenated to itself separated with a period (.).

1000000000.1000000000
2170000000.2170000000

The reason it is unnamed (UF) is that the field takes the name created in the first SELECT of a UNION. If you ran the CTE on its own, you'd see it labeled bc.

It doesn't look to provide any useful purpose other than to designate the anchor record. This might be a holdover from development or part of a design feature that was never fleshed out.

I'd move the comma onto the same line and just comment it out for now. Be careful removing too many fields though. Unlike this one, the rest may be required to provide a distinct enough record for the recursive count to work properly.

SELECT  id, parent, lft, rgt, title, type, caseid, userid, 0 AS level
        --, CAST(LEFT(CAST(id AS VARCHAR) + REPLICATE('0', 10), 10) AS VARCHAR) AS bc
FROM    [dbo].DM_FolderTree hc
WHERE   id = @id and caseid = @caseid
UNION ALL
SELECT  hc.id, hc.parent, hc.lft, hc.rgt, hc.title, hc.type, hc.caseid, hc.userid, level + 1
        --, CAST(bc + '.' + LEFT(CAST(hc.id AS VARCHAR) + REPLICATE('0', 10), 10) AS VARCHAR)
FROM    q
JOIN    [dbo].DM_FolderTree hc


回答2:

Looks like several fields are not being used.

I would question if the update is doing what is really needed or if the query could be simplified and give the same results in the fields you are actually using. There is no point in doing work you don't need to do. But only you can only know which one is wrong based on your local business requirements. All I can do is tell you that likely one of them is wrong.

If you want to check if your simplified version of the CTE give the same results in the fields you use, do this:

Declare  @id bigint,
    @left bigint,
    @right bigint,
    @parentid bigint,
    @offset bigint,
    @caseid bigint,
    @userid bigint

-- then set the variables to typical values


WITH    q AS
        (
        SELECT  id, parent, lft, rgt, title, type, caseid, userid, 0 AS level,
                CAST(LEFT(CAST(id AS VARCHAR) + REPLICATE('0', 10), 10) AS VARCHAR) AS bc
        FROM    [dbo].DM_FolderTree hc
        WHERE   id = @id and caseid = @caseid
        UNION ALL
        SELECT  hc.id, hc.parent, hc.lft, hc.rgt, hc.title, hc.type, hc.caseid, hc.userid, level + 1,
                CAST(bc + '.' + LEFT(CAST(hc.id AS VARCHAR) + REPLICATE('0', 10), 10) AS VARCHAR)
        FROM    q
        JOIN    [dbo].DM_FolderTree hc
        ON      hc.parent = q.id        
        )
        SELECT dft.id, dft.lft, q.lft + @offset), dft.rgt,((-q.rgt) + @offset),
        FROM [dbo].DM_FolderTree dft
        JOIN q on q.id = dft.id
        WHERE  lft <= (-(@left)) 
            AND rgt >= (-(@right)) 
            AND caseid = @caseid;

        WITH    q2 AS
        (
        SELECT  id, parent, lft, rgt
        FROM    [dbo].DM_FolderTree hc
        WHERE   id = @id and caseid = @caseid
        UNION ALL
        SELECT  hc.id, hc.parent, hc.lft, hc.rgt,
        FROM    q2
        JOIN    [dbo].DM_FolderTree hc
        ON      hc.parent = q2.id        
        )

        SELECT dft.Id, dft.lft, q2.lft + @offset), dft.rgt,((-q2.rgt) + @offset),
        FROM [dbo].DM_FolderTree dft
        JOIN q2 on q2.id = dft.id
        WHERE  lft <= (-(@left)) 
            AND rgt >= (-(@right)) 
            AND caseid = @caseid;

If your updated CTE gives the same results in the queries then the change is good. I used the id field and the values that are currently in the dft table and what you are replacing them with in the update to do the check. This can also tell you if the update results were what you were expecting. You can return any other fields that might help you make that determination if you want. Not knowing your data meaning, I don't know what else might be useful to you.

It's just me but I would not use the correlated subquery in the update either, I would rather see it as a join similar to what I used in the queries to check results.