Help with Cursor

[techipa]

Hello All,

I have to insert the rows from the #table to the user table. I have written a cursor to implement this process.

The #table has two rows which shoule be inserted in the user table. While inserting records, I have to insert the PK column as well. So I get the max(col) from the table and add one number.

I have to commit first insert since I have to use max(col) and aviod duplicate PK reference. But the cursor goes in the infinite loop.

Can anyone tell me what am I missing here?

Here is the code

DECLARE @SaveError int,
		@Name	varchar(50),
		@Code	varchar(50),
		@Category	varchar(50),
		@RefID		int

SET NOCOUNT ON

CREATE TABLE #AddNew
	(
		Code	varchar(50),
		Revised	varchar(50),
		Name	varchar(50),
		Category varchar(50)	
	)

INSERT INTO #AddNew (Code, Revised, Name, Category)
SELECT 'code1','Newcode1','Name1', 'Cate1'

INSERT INTO #AddNew (Code, Revised, Name, Category)
SELECT 'code2','Newcode2','Name2', 'Cate2'

IF @@ROWCOUNT > 0
BEGIN 

	DECLARE @NewCur CURSOR 
         SET @NewCur = CURSOR FOR 
			SELECT Code, Name, Category
			FROM #AddNew
	
	OPEN @NewCur
		FETCH NEXT FROM @NewCur INTO @Code, @Name, @Category
		
         WHILE @@Fetch_Status = 0
				
				BEGIN TRAN
		
				INSERT INTO dbo.Table1
					(ColID, SID, Name, ExtRef, Category, Active)
				SELECT (SELECT max(ColID)+1 FROM dbo.Table1), 0, @Name, @Code, @Category, 1
				FROM #AddNew
		
				IF @@ERROR <> 0
					BEGIN
			ROLLBACK TRAN
				     RAISERROR('Error encountered', 15, 1)
					 RETURN
					END
				ELSE 
			COMMIT TRAN
						
		FETCH NEXT FROM @NewCur INTO @Code, @Name, @Category
END
		CLOSE @NewCur
		DEALLOCATE @NewCur

END

Thanks in advance.
techiPA

 

[SQLSister]

First, you should set the id field to increment automatically by setting the field as an identity field. It is a very bad practice to select the max value and use that because if two people do inserts at the same time both might get the same max value. Very bad idea.

Once you fix that problem, then you do not need to use a cursor to do the insert at all. Cursors are very bad for performance and should never be used to do inserts, updates, selects or deletes. Do the insert by joining to the table withthe data you want to insert instead.


Questions about posting. See faq183-874

 

[techipa]

SQLSister,

I did not make PK field as identity because at my work place they do not prefer PK being an Identity column.

Secondly, this is going to be a scheduled batch job. The user will never add rows to this table except updating the rows.

techiPA

 

[SQLSister]

I don't see anything that immediately strikes me about the code but I don't generally write cursors, so I may have missed something. You could try putting the insert part in a spearate sp and then calling the sp from the cursor. Maybe that would fix the error. The infinite loop doesn't make sense to me because you clearly go to the next record and you clearly check to see if you are at a record before processing. If you comment out all the processing stuff (everything except the statements to run the cursor) and just put a print statement in the cursor, does it still infinite loop? If not then you know the cursor code itself is fine it is the transaction stuff which is messing it up. If it does then you know there is a proble withthe structure of your cursor bu the transaction processing and insert are probably fine.

Why does your company object to identity columns?

Questions about posting. See faq183-874

 

[ptheriault]

I'm a little confused about some of the lines of code in your cursor. For example this line.

FETCH NEXT FROM @NewCur INTO @Code, @Name, @Category

You do a fetch next before you even do a fetch?

I've always written my cursors like this and I've never had any problems.

  DECLARE NewCur CURSOR FOR
            SELECT Code,
                   Name, 
                   Category
            FROM #AddNew
    
        OPEN NewCur

        FETCH NewCur INTO @Code, 
                          @Name, 
                          @Category
        
         WHILE @@Fetch_Status = 0
                
                BEGIN TRAN
        
                INSERT INTO dbo.Table1
                    (ColID, SID, Name, ExtRef, Category, Active)
                SELECT (SELECT max(ColID)+1 FROM dbo.Table1), 0, @Name, @Code, @Category, 1
                FROM #AddNew
        
                IF @@ERROR <> 0
                    BEGIN
            ROLLBACK TRAN
                     RAISERROR('Error encountered', 15, 1)
                     RETURN
                    END
                ELSE
            COMMIT TRAN
                        
        FETCH NewCur INTO @Code, @Name, @Category

- Paul
- If at first you don't succeed, find out if the loser gets anything.

 

[techipa]

Paul,

I changed the cursor syntax as per your suggestion.

Why are not you fetching next record in the last fetch?

 

[ptheriault]

The Next Keyword isn't needed. I've never used it.
Does your cursor work now?

- Paul
- If at first you don't succeed, find out if the loser gets anything.

 

[techipa]

Paul,
No, it does not work either way. It goes in the loop I guess.

I tried to put the select statements after FETCH

DECLARE NewCur CURSOR FOR
         	SELECT Code, Name, Category
			FROM #AddNew
	
	OPEN NewCur
		FETCH NewCur INTO @Code, @Name, @Category
		select 'Test1'	
		select @Code, @Name, @Category

I could get the 'test1' but not the first record in fetch.

 

[ptheriault]

I would do what SQL Sister said. Take out all your logic and just get the work flow. Once you have that, put back in your logic.

- Paul
- If at first you don't succeed, find out if the loser gets anything.

 

[techipa]

Will do, thanks to you and SQLSister
-techiPA