Stored procedure using cursors dramatically slow

Question

I wrote a stored procedure that sometimes i need to call many times in a row. The sp uses one cursor. It is the first time I realize why Robert Vieira wrote "Cursors are slow" in his SS2000 Pro book i read years ago.

Could you suggest a better implementation? I am sorry to use copy and paste and not to use a simplified version, anyway a general suggestion is fine too I don't expect you to rewrite it.

here the code:

CREATE  PROC sp_DuplicaDocDistrib(@ID_DIP_SRC INT ,@ID_DIP_DEST int) AS
BEGIN
    DECLARE @ID_LISTA INT
    DECLARE @ID_DISTRIB INT
    DECLARE @TIPO_DISTRIB NVARCHAR(1)
    DECLARE @NRO_COPIE NVARCHAR(50)
    DECLARE @NOTE NVARCHAR(50)

    SET @ID_LISTA  = (SELECT LAST_ID FROM SW9_SEQUENCES WHERE SEQ_NAME ='DOCN_ID_LISTA_DISTRIBUZIONE') 
       SET @NOTE  = 'Automatically distributed on  '+  convert(varchar(25), getdate(), 103) 

    DECLARE CURSOR_DOCDISTRIB CURSOR FOR    
       SELECT ID_DISTRIBUZIONE,TIPO_DISTRIBUZIONE,NRO_COPIE from DOC_LISTE_DISTRIBUZIONE BASE
       WHERE BASE.ID_DIPENDENTE = @ID_DIP_SRC
        AND NOT EXISTS ( SELECT ID_LISTA FROM DOC_LISTE_DISTRIBUZIONE
         WHERE ID_DISTRIBUZIONE = BASE.ID_DISTRIBUZIONE AND ID_DIPENDENTE = @ID_DIP_DEST)

      OPEN CURSOR_DOCDISTRIB
      FETCH NEXT FROM CURSOR_DOCDISTRIB INTO @ID_DISTRIB,@TIPO_DISTRIB,@NRO_COPIE

      WHILE @@FETCH_STATUS = 0
      BEGIN
        SET @ID_LISTA = @ID_LISTA +1
        PRINT @ID_LISTA
        PRINT @ID_DISTRIB
        INSERT INTO DOC_LISTE_DISTRIBUZIONE (ID_LISTA,ID_DISTRIBUZIONE,ID_DIPENDENTE,NRO_COPIE,TIPO_DISTRIBUZIONE,NOTE)
           VALUES (@ID_LISTA,@ID_DISTRIB,@ID_DIP_DEST,@NRO_COPIE,@TIPO_DISTRIB,@NOTE)

        FETCH NEXT FROM CURSOR_DOCDISTRIB INTO @ID_DISTRIB,@TIPO_DISTRIB,@NRO_COPIE        
      END
      CLOSE CURSOR_DOCDISTRIB
      DEALLOCATE CURSOR_DOCDISTRIB

      UPDATE SW9_SEQUENCES
            SET LAST_ID = @ID_LISTA
            WHERE SEQ_NAME = 'DOCN_ID_LISTA_DISTRIBUZIONE'    

END

Answer 1

Use FAST_FORWARD cursors (it's a shorthand for static, forward_only, read-only), they're much faster than defaults.

However, I believe that you don't need a cursor here at all, it could be rewritten as a simple INSERT.. SELECT FROM.

Answer 2

Seems you can avoid the cursor here

Please replace your cursor with statement like below.. I haven't put all the columns here but sure you get the idea!!

INSERT INTO DOC_LISTE_DISTRIBUZIONE (ID_LISTA,ID_DISTRIBUZIONE,ID_DIPENDENTE,NRO_COPIE,TIPO_DISTRIBUZIONE,NOTE)
SELECT @ID_LISTA,ID_DISTRIBUZIONE, @ID_DIP_DEST, TIPO_DISTRIBUZIONE,NRO_COPIE 
From DOC_LISTE_DISTRIBUZIONE BASE
WHERE BASE.ID_DIPENDENTE = @ID_DIP_SRC
         AND NOT EXISTS ( SELECT ID_LISTA FROM DOC_LISTE_DISTRIBUZIONE
         WHERE ID_DISTRIBUZIONE = BASE.ID_DISTRIBUZIONE AND ID_DIPENDENTE = @ID_DIP_DEST

Read more about Insert Data From One Table to Another Table

If you need incremental field, then you can use ROW_NUMBER() to achieve this.

i.e.

Select ROW_NUMBER() Over (order By FieldName1) IncrementField ,FieldName2
From TableName

Answer 3

It appears that ID_LISTA is not an IDENTITY column, meaning you will need to populate this manually.

You are declaring your cursor as:

   SELECT ID_DISTRIBUZIONE,TIPO_DISTRIBUZIONE,NRO_COPIE from DOC_LISTE_DISTRIBUZIONE BASE
       WHERE BASE.ID_DIPENDENTE = @ID_DIP_SRC
         AND NOT EXISTS (
            SELECT ID_LISTA FROM DOC_LISTE_DISTRIBUZIONE
             WHERE ID_DISTRIBUZIONE = BASE.ID_DISTRIBUZIONE AND ID_DIPENDENTE = @ID_DIP_DEST
         )

However, when you loop through your cursor, the only processing you appear to be doing is to increment the value of @ID_LISTA. One solution therefore may be to start off creating a temporary table:

  CREATE TABLE #Temp (
     ID_LISTA_INC int IDENTITY(1,1),
     ID_DISTRIBUZIONE ...,
     ID_DIPENDENTE ...
     (etc)
  )

Then,

  INSERT INTO #Temp
       (ID_DISTRIBUZIONE, ID_DIPENDENTE, ...)
       SELECT ID_DISTRIBUZIONE,TIPO_DISTRIBUZIONE,NRO_COPIE from DOC_LISTE_DISTRIBUZIONE BASE
           WHERE BASE.ID_DIPENDENTE = @ID_DIP_SRC
             AND NOT EXISTS (
                 SELECT ID_LISTA FROM DOC_LISTE_DISTRIBUZIONE
                  WHERE ID_DISTRIBUZIONE = BASE.ID_DISTRIBUZIONE AND ID_DIPENDENTE = @ID_DIP_DEST
             )

to populate #Temp with the data you are inserting into DOC_LISTE_DISTRIBUZIONE. You should then be able to do:

INSERT INTO DOC_LISTE_DISTRIBUZIONE
    (ID_LISTA,ID_DISTRIBUZIONE,ID_DIPENDENTE,NRO_COPIE,TIPO_DISTRIBUZIONE,NOTE)
    SELECT ID_LISTA_INC + @ID_LISTA, ID_DISTRIBUZIONE,ID_DIPENDENTE, ... FROM #Temp

SELECT @ID_LISTA = @ID_LISTA + max(ID_LISTA_INC) FROM #Temp

UPDATE SW9_SEQUENCES
    SET LAST_ID = @ID_LISTA
    WHERE SEQ_NAME = 'DOCN_ID_LISTA_DISTRIBUZIONE'