Use cursor for with 2 tables to insert rows-CodePudding

I have 2 tables: items and cost.

In table items, I have old item code and new item code
In table cost, I have the item code and the cost

I want to create new rows in table cost for the new item code according to the cost that already exist for the old item code.

For example:

I expect to see this result after I run the cursor in the cost table:

I try to run this, but it's been running forever and duplicates every row 100000 times:

DECLARE @item_code_old nvarchar (50)
DECLARE @item_code_new nvarchar (50)

DECLARE CostCURSOR CURSOR FOR
    SELECT item_code_old, item_code_new 
    FROM item 
    WHERE company = 'AEW' AND item_code_new IS NOT NULL

OPEN CostCURSOR 

FETCH NEXT FROM CostCURSOR INTO @item_code_old, @item_code_new

WHILE (@@FETCH_STATUS = 0)
BEGIN
    SELECT @item_code_old = item_code_old 
    FROM cost 
    WHERE company = 'AEW' AND year = 2021

    INSERT INTO cost
        SELECT
            company,
            year,
            month,
            @item_code_new,
            unit_cost
        FROM
            cost
        WHERE
            company = 'AEW' AND year = 2021

    FETCH NEXT FROM CostCURSOR INTO @item_code_old, @item_code_new
END

CLOSE CostCURSOR 
DEALLOCATE CostCURSOR

What am I missing?

CodePudding user response：

What you are doing with your variables doesn't really make sense, you fetch your next value into @item_code_old, but then at the start of each loop you then assign it a value pretty much at random:

SELECT  @item_code_old = item_code_old
FROM    cost
WHERE   company = 'AEW'
AND     year = 2021;

Then when it comes to inserting the new cost, you don't use this variable at all:

INSERT INTO cost
SELECT  company, year, month, @item_code_new, unit_cost
FROM    cost
WHERE   company = 'AEW'
AND     year = 2021;

I'd guess that you don't need this first step that reassigns @item_code_old, and you do need to apply the filter to the insert query.

With that being said, a cursor is completely unnecessary here (which you'll find is often the case with cursors), you can do this with a single insert, using something like this:

INSERT Cost (Company, Year, Month, item_code_old, unit_cost)
SELECT  c.company, c.year, c.month, i.item_code_new, c.unit_cost
FROM    cost AS c
        INNER JOIN Item AS i
            ON c.item_code_old = i.item_code_old
WHERE   c.company = 'AEW'
AND     c.year = 2021
AND     i.item_code_new IS NOT NULL;

For the sake of completeness, your cursor solution would be something the following, but I would stress this is not a good approach. Cursors should be a last resort, and are probably something it is not even worth practising as it is not something you will find very useful. Instead of practicing cursors, instead practice trying to find the set-based approach, this will be significantly better use of your time. Anyway, rant over:

DECLARE @item_code_old NVARCHAR(50);
DECLARE @item_code_new NVARCHAR(50);


DECLARE CostCURSOR CURSOR LOCAL FAST_FORWARD FOR
    SELECT  item_code_old, item_code_new
    FROM    item
    WHERE   company = 'AEW'
    AND     item_code_new IS NOT NULL;

OPEN CostCURSOR;
FETCH NEXT FROM CostCURSOR INTO @item_code_old, @item_code_new;

WHILE(@@FETCH_STATUS = 0)
BEGIN

    INSERT INTO cost (Company, Year, Month, Item_Code_Old, unit_cost)
    SELECT  company, year, month, @item_code_new, unit_cost
    FROM    cost
    WHERE   company = 'AEW'
    AND     year = 2021
    AND     item_code_old = @item_code_old;

    FETCH NEXT FROM CostCURSOR INTO @item_code_old, @item_code_new;

END;
CLOSE CostCURSOR;
DEALLOCATE CostCURSOR;