sqlsql-serverquery-optimizationdatabase-cursor

Avoid using a cursor when creating new records in a table and update another with the newly generated keys?


Background

I recently had to fix a bug in an old WCF/Angular project. It is a To Do app that allows you to create tasks (A 'List' record) and mark it completed in 'ToDo' with some time calculations using a third table. Let's ignore everything wrong with it (and there's a lot) as I am certain nothing will be changed even if I made suggestions.

The Tables

The two tables look something like this:

The Bug

The bug I was asked to fix is that when a user copies a ToDo Task (which is a combination of data from the three tables) to different days and makes a change in the text value, every instance of that item has their text value changed. The bug was fixed with a trivial change to the API (when a copy was being made, a new 'ToDo' item was created that pointed to the same 'List' item). However, I was also asked to fix the existing records so they do not show this behavior in the future.

My Attempt

I created a query that would create a new List item for each ToDo item that was a copy of a List item and then assign to those ToDo items the new ListId. I am fairly unfamiliar with SQL (and certainly know nothing about writing performant queries) and would like to know alternate strategies. In the end the three hours I spent reading documentation and writing the query were useless as it was decided the earlier records didn't matter. My code took about 9 minutes to execute for (if I recall correctly) about 6600 ToDo items.

My Question

I'm quite certain the primary reason the query is this slow is my use of a cursor. Is there anyway I could do this without looping over each record one at a time? I do not have a big knowledge base or good intuition for SQL and would enjoy any resources that help me get better.

My Code

-- omitted declarations
    SELECT *
    INTO #tempLists
    FROM List
    WHERE ListId IN (
            SELECT ListId
            FROM todo
            GROUP BY ListId
            HAVING count(ListId) >= 2
            );

    WITH cte
    AS (
        SELECT ToDoId,
            ROW_NUMBER() OVER (
                PARTITION BY ListId ORDER BY [Date]
                ) AS rn
        FROM ToDo
        WHERE ListId IN (
                SELECT ListId
                FROM #tempLists
                )
        )
    SELECT *
    INTO #tempToDos
    FROM cte
    WHERE rn > 1

    DECLARE db_cursor CURSOR
    FOR
    SELECT TodoId,
        ListId
    FROM ToDo
    WHERE TodoId IN (
            SELECT TodoId
            FROM #tempToDos
            )

    OPEN db_cursor

    FETCH NEXT
    FROM db_cursor
    INTO @TodoId,
        @ListId

    WHILE @@FETCH_STATUS = 0
    BEGIN
        INSERT INTO List (
            SiteUserId,
            ListText,
            [Status],
            CreatedBy,
            CreatedDate
            )
        SELECT SiteUserId,
            ListText,
            [Status],
            CreatedBy,
            CreatedDate
        FROM #tempLists
        WHERE ListId = @ListId

        UPDATE ToDo
        SET ListId = (
                SELECT SCOPE_IDENTITY()
                )
        WHERE TodoId = @TodoId

        PRINT @TodoId
        PRINT @ListId

        FETCH NEXT
        FROM db_cursor
        INTO @TodoId,
            @ListId
    END
-- omitted transaction and deallocation stuff

Solution

  • Using the advice from @Dale K and @TN I came up with the following to replace the cursor operations. It executed in 1 second.

    DECLARE @insertedLists TABLE (
        ListId INT,
        ToDoId INT
        );
    
    WITH cte
    AS (
        SELECT tl.*,
            td.TodoId
        FROM #tempLists tl
        CROSS JOIN #tempToDos td
        WHERE tl.ListId = td.ListId
        )
    MERGE List AS TGT
    USING cte AS SRC
        ON 1 = 0
    WHEN NOT MATCHED
        THEN
            INSERT (
                SiteUserId,
                ListText,
                [Status],
                CreatedBy,
                CreatedDate
                )
            VALUES (
                SRC.SiteUserId,
                SRC.ListText,
                0,
                SRC.CreatedBy,
                SRC.CreatedDate
                )
    OUTPUT INSERTED.ListId,
        SRC.ToDoId
    INTO @insertedLists;
    
    UPDATE ToDo
    SET ToDo.ListId = tl.ListId
    FROM @insertedLists AS tl
    WHERE ToDo.ToDoId = tl.ToDoId