CODEWITHSUNDEEP
sql-serverstored-procedurescursor
Soner
Soner

Reputation: 115

T-SQL Cursor in stored procedure

I am using a stored procedure and I want to use cursor for inserting new data (if data exist I want to update)

ALTER Procedure [dbo].[conn]
    @ResellerID int,
    @GWResellerID int,
    @UserName varchar(50),
    @Password varchar(50),
    @URL varchar(100),
    @ServiceType int,
    @ServiceDesc varchar(50),
    @FeedFrom bit,
    @PublicKey varchar(max)
AS
    declare gateway cursor for
         select * 
         from reseller_profiles 
         where main_reseller_ID = @ResellerID

    OPEN gateway

    FETCH NEXT FROM gateway INTO @ResellerID

    WHILE @@FETCH_STATUS = 0
    BEGIN
       INSERT INTO [dbo].tblGatewayConnection([ResellerID],[GWResellerID], [UserName], [Password], [URL], [ServiceType], [ServiceDesc],[feedFromMain], publicKey)
       VALUES (@ResellerID, @GWResellerID, @UserName, @Password, @URL, @ServiceType, @ServiceDesc, @FeedFrom, @PublicKey)

       FETCH NEXT FROM gateway INTO @ResellerID
    END

    CLOSE gateway
    DEALLOCATE gateway

My table name is tblGatewayConnection has these columns:

resellerID
gwResellerID
userName
password
url
serviceType
serviceDesc
feedFromMain
publicKey

While I insert data using the stored procedure, I get an exception

Cursorfetch: The number of variables declared in the INTO list must match that of selected columns.

What did I miss ?

Any help will be appreciated.

Thanks.

Upvotes: 1

Views: 2241

Answers (2)

marc_s
marc_s

Reputation: 754508

Why even bother with a cursor?!?!?!?!? I won't tell you what's wrong with your cursor - because instead of fixing the cursor, you should learn to avoid it in the first place!

Seriously - avoid RBAR (row-by-agonizing-row) processing whenever you can, and here, it's really utterly pointless to use a cursor - just use this nice and clean set-based statement:

ALTER PROCEDURE [dbo].[conn] @ResellerID   INT,
                             @GWResellerID INT,
                             @UserName     VARCHAR(50),
                             @Password     VARCHAR(50),
                             @URL          VARCHAR(100),
                             @ServiceType  INT,
                             @ServiceDesc  VARCHAR(50),
                             @FeedFrom     BIT,
                             @PublicKey    VARCHAR(max)
AS
    INSERT INTO dbo.tblGatewayConnection
                (ResellerID, GWResellerID, UserName, Password,
                 URL, ServiceType, ServiceDesc, feedFromMain,
                 publicKey)
       SELECT 
          ResellerID, GWResellerID, UserName, Password,
          URL, ServiceType, ServiceDesc, feedFromMain,
          publicKey
       FROM   
          dbo.reseller_profiles
       WHERE  
          main_reseller_ID = @ResellerID

and you're done!! No messy cursor, no unnecessary local variables - just a simple INSERT ... SELECT and you've achieved what you want!

Upvotes: 6

GarethD
GarethD

Reputation: 69769

I am not sure the error message could be any more self explanatory:

Cursorfetch: The number of variables declared in the INTO list must match that of selected columns.

You are selecting all the columns from reseller_profiles

declare gateway cursor for
     select * 
     from reseller_profiles 
     where main_reseller_ID = @ResellerID

And trying to put them into a single variable:

FETCH NEXT FROM gateway INTO @ResellerID

The number of columns you select in your cursor must match the number of variables you are inserting to, so you would need something like

declare gateway cursor for
     select reseller_id
     from reseller_profiles 
     where main_reseller_ID = @ResellerID

HOWEVER you should not be using a cursor for this, you can use the same thing using INSERT .. SELECT:

INSERT INTO [dbo].tblGatewayConnection
(   [ResellerID],[GWResellerID], [UserName], [Password], [URL], 
    [ServiceType], [ServiceDesc],[feedFromMain], publicKey
)
SELECT  Resellerid, @GWResellerID, @UserName, @Password, 
        @URL, @ServiceType, @ServiceDesc, @FeedFrom, @PublicKey
FROM    reseller_profiles 
WHERE   main_reseller_ID = @ResellerID;

As has been said, you should avoid cursors at all costs, if you absolutely have to use a cursor, declare the most light weight cursor you can. In your case for example, you were only moving forwards within the cursor, only reading data, not modifying it, and only accessing the cursor locally, therefore you would declare the cursor as follows:

DECLARE gateway CURSOR LOCAL STATIC FAST_FORWARD
FOR
    SELECT  ...
    FROM ..

Although cursors perform terribly at the best of times they are given an even worse reputation by lazy declarations.

Finally, You should get out of the habit of using SELECT *

Upvotes: 5

Related Questions