T-SQL loop for inserting data with incrementing id

Question

I have this T-SQL procedure to store the result of a query in my database. This query could extract more than a result therefore I need to create a counter to insert row by row at correct id value:

DECLARE @abi NVARCHAR(10)
DECLARE @username NVARCHAR(100)
DECLARE @password NVARCHAR(100)
DECLARE @id_unita_produttiva int
DECLARE @row_count int

DECLARE @data CURSOR

SET @data = CURSOR FOR
    SELECT 
        b.abi AS abi, cb.username AS username, cb.password AS password, 
        f.id AS id_unita_produttiva 
    FROM
        unita_produttiva_mcc f, credenziali_banche_mcc cb, banche b 
    WHERE
        f.banca_id = b.id AND 
        b.abi = cb.abi_banca AND
        f.banca_id <> f.id

OPEN @data -- should contain 2 rows from the above query

FETCH NEXT FROM @data INTO @abi, @username, @password, @id_unita_produttiva 

WHILE @@FETCH_STATUS = 0
BEGIN
    SET @row_count = (SELECT MAX(id) 
                      FROM fea.credenziali_banche_mcc) -- I need this because I must set an incremental ID

    INSERT INTO fea.credenziali_banche_mcc (id, abi_banca, username, password, id_unita_produttiva) 
    VALUES (@row_count + 1, @abi, @username, @password, @id_unita_produttiva);

    FETCH NEXT FROM @data INTO @abi, @username, @password, @id_unita_produttiva 
END

CLOSE @data
DEALLOCATE @data

I don't know why but the query loops, and when I stop it manually (for example after 20 second), I find thousands of rows inserted instead of two..

What's wrong?

Is it possible that, inserting data into one of the table with which I fill the cursor @data the FETCH never end because, increasing the size of table, I'm increasing even the variable @data (even if I set it out of the loop)?

---

The @Charlieface solution below works perfectly without T-SQL procedure.

Unfortunately I must change the script to check if row already exist before insert it and I couldn't do it with just a select... insert

Therefore I modify the first script as follows to show the declaration of the LOCAL STATIC READ_ONLY FORWARD_ONLY CURSOR because the sintax is a little bit different from the one I wrote above:

DECLARE @abi NVARCHAR(10)
DECLARE @username NVARCHAR(100)
DECLARE @password NVARCHAR(100)
DECLARE @id_unita_produttiva int
DECLARE @row_count int

--DECLARE @data LOCAL FORWARD_ONLY STATIC READ_ONLY CURSOR // this should be remove, I couldn't manage to declare this kind of cursor right here

--SET @data = CURSOR FOR // even this should be removed and replaced with
  DECLARE data01 CURSOR LOCAL STATIC READ_ONLY FORWARD_ONLY FOR // data01 without @
  SELECT 
    b.abi AS abi, cb.username AS username, cb.password AS password, 
    f.id AS id_unita_produttiva 
  FROM
    fea.unita_produttiva_mcc f, fea.credenziali_banche_mcc cb, fea.banche b 
  WHERE
     f.banca_id = b.id AND 
     b.abi = cb.abi_banca AND
     f.banca_id <> f.id

OPEN data01

FETCH NEXT FROM data01 INTO @abi, @username, @password, @id_unita_produttiva 

WHILE @@FETCH_STATUS = 0
BEGIN
  SET @row_count = (SELECT MAX(id) 
                  FROM fea.credenziali_banche_mcc) -- I need this because I must set an incremental ID
                  
  IF EXISTS (SELECT abi_banca, username, password, id_unita_produttiva from fea.credenziali_banche_mcc where abi_banca = @abi AND username = @username AND password = @password AND id_unita_produttiva = @id_unita_produttiva)
    print('Row Already exists');
  ELSE
    INSERT INTO fea.credenziali_banche_mcc (id, abi_banca, username, password, id_unita_produttiva) 
    VALUES (@row_count + 1, @abi, @username, @password, @id_unita_produttiva);

FETCH NEXT FROM data01 INTO @abi, @username, @password, @id_unita_produttiva 
END

CLOSE data01
DEALLOCATE data01

Hope helps.

Answer 1

Your primary issue was that you are inserting from the same table you are selecting, and using a dynamic cursor, so you are getting the Halloween Problem. You could in theory solve that by using a static cursor.

But you don't need a cursor at all. You can just do a single insert ... select ....

• You can use `ROW_NUMBER to generate the correct IDs.
• You need a transaction, and use updlock to prevent anyone else writing to the top of the table at the same time.
• You should use explicit join syntax, rather than the ancient comma-join.

SET XACT_ABORT ON;

BEGIN TRAN;

declare @rowcount int = (
    select max(id)
    from fea.credenziali_banche_mcc
    with (updlock));

insert into fea.credenziali_banche_mcc
  (id, abi_banca, username, password, id_unita_produttiva)
select
  @rowcount + ROW_NUMBER() OVER (ORDER BY (SELECT NULL)),
  b.abi,
  cb.username,
  cb.password,
  f.id
from unita_produttiva_mcc f
join credenziali_banche_mcc cb on b.abi = cb.abi_banca
join banche b on f.banca_id = b.id
where f.banca_id <> f.id;

COMMIT;

Having said that, you should really just use an IDENTITY column, which auto-increments. Then you can just leave that column off the insert. No transactions or extra select needed.

insert into fea.credenziali_banche_mcc
  (abi_banca, username, password, id_unita_produttiva)
select
  b.abi,
-- etc