views:

3077

answers:

5

Main problem is about changing the index of rows to 1,2,3.. where contact-id and type is the same. but all columns can contain exactly the same data because of some ex-employee messed up and update all rows by contact-id and type. somehow there are rows that aren't messed but index rows are same. It is total chaos.

I tried to use an inner cursor with the variables coming from the outer cursor. But It seems that its stuck in the inner cursor.

A part of the query looks like this:

Fetch NEXT FROM OUTER_CURSOR INTO @CONTACT_ID,  @TYPE
While (@@FETCH_STATUS <> -1)
BEGIN
IF (@@FETCH_STATUS <> -2)

    DECLARE INNER_CURSOR Cursor 
    FOR 
    SELECT * FROM CONTACTS
    where CONTACT_ID = @CONTACT_ID
    and TYPE = @TYPE 

    Open INNER_CURSOR 

    Fetch NEXT FROM INNER_CURSOR 
    While (@@FETCH_STATUS <> -1)
    BEGIN
    IF (@@FETCH_STATUS <> -2)

What can be the problem? Is @@FETCH_STATUS ambiguous or something?

EDIT: everything looks fine if i don't use this code inside inner cursor:

UPDATE CONTACTS
SET INDEX_NO = @COUNTER
where current of INNER_CURSOR

EDIT: here is the big picture:

BEGIN TRAN

DECLARE @CONTACT_ID VARCHAR(15)
DECLARE @TYPE VARCHAR(15)
DECLARE @INDEX_NO  SMALLINT
DECLARE @COUNTER SMALLINT
DECLARE @FETCH_STATUS INT 

DECLARE OUTER_CURSOR CURSOR 

FOR 

SELECT CONTACT_ID, TYPE, INDEX_NO FROM CONTACTS
WHERE  
CONTACT_ID IN (SELECT CONTACT_ID FROM dbo.CONTACTS
WHERE CONTACT_ID IN(...)
GROUP BY CONTACT_ID, TYPE, INDEX_NO
HAVING COUNT(*) > 1

OPEN OUTER_CURSOR 

FETCH NEXT FROM OUTER_CURSOR INTO @CONTACT_ID,  @TYPE, @INDEX_NO
WHILE (@@FETCH_STATUS <> -1)
BEGIN
IF (@@FETCH_STATUS <> -2)

SET @COUNTER = 1

     DECLARE INNER_CURSOR CURSOR 
     FOR 
     SELECT * FROM CONTACTS
     WHERE CONTACT_ID = @CONTACT_ID
     AND TYPE = @TYPE 
     FOR UPDATE 

     OPEN INNER_CURSOR 

     FETCH NEXT FROM INNER_CURSOR 

     WHILE (@@FETCH_STATUS <> -1)
     BEGIN
     IF (@@FETCH_STATUS <> -2)

     UPDATE CONTACTS
     SET INDEX_NO = @COUNTER
     WHERE CURRENT OF INNER_CURSOR

     SET @COUNTER = @COUNTER + 1

     FETCH NEXT FROM INNER_CURSOR 
     END
     CLOSE INNER_CURSOR
     DEALLOCATE INNER_CURSOR

FETCH NEXT FROM OUTER_CURSOR INTO @CONTACT_ID,  @TYPE, @INDEX_NO
END
CLOSE OUTER_CURSOR
DEALLOCATE OUTER_CURSOR

COMMIT TRAN
A: 

Do you do any more fetches? You should show those as well. You're only showing us half the code.

It should look like:

FETCH NEXT FROM @Outer INTO ...
WHILE @@FETCH_STATUS = 0
BEGIN
  DECLARE @Inner...
  OPEN @Inner
  FETCH NEXT FROM @Inner INTO ...
  WHILE @@FETCH_STATUS = 0
  BEGIN
  ...
    FETCH NEXT FROM @Inner INTO ...
  END
  CLOSE @Inner
  DEALLOCATE @Inner
  FETCH NEXT FROM @Outer INTO ...
END
CLOSE @Outer
DEALLOCATE @Outer

Also, make sure you do not name the cursors the same... and any code (check your triggers) that gets called does not use a cursor that is named the same. I've seen odd behavior from people using 'theCursor' in multiple layers of the stack.

David B
rest of the query looks just like you described. cursor names and fetches are properly named.
Orkun Balkancı
+1  A: 

You have a variety of problems. First, why are you using your specific @@FETCH_STATUS values? It should just be @@FETCH_STATUS = 0.

Second, you are not selecting your inner Cursor into anything. And I cannot think of any circumstance where you would select all fields in this way - spell them out!

Here's a sample to go by. Folder has a primary key of "ClientID" that is also a foreign key for Attend. I'm just printing all of the Attend UIDs, broken down by Folder ClientID:

Declare @ClientID int;
Declare @UID int;

DECLARE Cur1 CURSOR FOR
 SELECT ClientID From Folder;

OPEN Cur1
FETCH NEXT FROM Cur1 INTO @ClientID;
WHILE @@FETCH_STATUS = 0
BEGIN
 PRINT 'Processing ClientID: ' + Cast(@ClientID as Varchar);
 DECLARE Cur2 CURSOR FOR
  SELECT UID FROM Attend Where ClientID=@ClientID;
 OPEN Cur2;
 FETCH NEXT FROM Cur2 INTO @UID;
 WHILE @@FETCH_STATUS = 0
 BEGIN
  PRINT 'Found UID: ' + Cast(@UID as Varchar);
  FETCH NEXT FROM Cur2 INTO @UID;
 END;
 CLOSE Cur2;
 DEALLOCATE Cur2;
 FETCH NEXT FROM Cur1 INTO @ClientID;
END;
PRINT 'DONE';
CLOSE Cur1;
DEALLOCATE Cur1;

Finally, are you SURE you want to be doing something like this in a stored procedure? It is very easy to abuse stored procedures and often reflects problems in characterizing your problem. The sample I gave, for example, could be far more easily accomplished using standard select calls.

Mark Brittingham
i dont select anything into because i just wanted to update the current row of the inner cursor. do i have to use into @somevariable?
Orkun Balkancı
Do you have more than one record in your Contact table for each "Contact_ID"? If so, your "Having" is spurious - it will always be false. If not then never mind. I'm just trying to figure out your logic. Also, shouldn't there be a closing ")" after the having clause?
Mark Brittingham
Yikes - you are reassigning to the INDEX_NO field on a record-by-record basis but only for those records where there are multiples even after taking the old INDEX_NO into account?
Mark Brittingham
yes and while data is sensitive i cannot delete or update any other column.
Orkun Balkancı
Yea old index_no became a problem. but running the query a few times solved that problem.
Orkun Balkancı
+1  A: 

You could also sidestep nested cursor issues, general cursor issues, and global variable issues by avoiding the cursors entirely.

declare @rowid int
declare @rowid2 int
declare @id int
declare @type varchar(10)
declare @rows int
declare @rows2 int
declare @outer table (rowid int identity(1,1), id int, type varchar(100))
declare @inner table (rowid int  identity(1,1), clientid int, whatever int)

insert into @outer (id, type) 
Select id, type from sometable

select @rows = count(1) from @outer
while (@rows > 0)
Begin
    select top 1 @rowid = rowid, @id  = id, @type = type
    from @outer
    insert into @innner (clientid, whatever ) 
    select clientid whatever from contacts where contactid = @id
    select @rows2 = count(1) from @inner
    while (@rows2 > 0)
    Begin
     select top 1 /* stuff you want into some variables */
     /* Other statements you want to execute */
     delete from @inner where rowid = @rowid2
     select @rows2 = count(1) from @inner
    End  
    delete from @outer where rowid = @rowid
    select @rows = count(1) from @outer
End
cmsjr
Thanks a lot nice example! the problem is i kinda have to use current row of cursor. because think of 2 or more rows can contain exactly the same data or different data u cannot risk to lose. problem with your this solution is it is impossible to match data with external table.
Orkun Balkancı
+1  A: 

This smells of something that should be done with a JOIN instead. Can you share the larger problem with us?


Hey, I should be able to get this down to a single statement, but I haven't had time to play with it further yet today and may not get to. In the mean-time, know that you should be able to edit the query for your inner cursor to create the row numbers as part of the query using the ROW_NUMBER() function. From there, you can fold the inner cursor into the outer by doing an INNER JOIN on it (you can join on a sub query). Finally, any SELECT statement can be converted to an UPDATE using this method:

UPDATE [YourTable/Alias]
   SET [Column] = q.Value
FROM
(
   ... complicate select query here ...
) q

Where [YourTable/Alias] is a table or alias used in the select query.

Joel Coehoorn
sure. im editting.
Orkun Balkancı
thanks for your time joel.
Orkun Balkancı
A: 

I don't fully understand what was the problem with the "update current of cursor" but it is solved by using the fetch statement twice for the inner cursor:

FETCH NEXT FROM INNER_CURSOR

WHILE (@@FETCH_STATUS <> -1)
BEGIN

UPDATE CONTACTS
SET INDEX_NO = @COUNTER
WHERE CURRENT OF INNER_CURSOR

SET @COUNTER = @COUNTER + 1

FETCH NEXT FROM INNER_CURSOR
FETCH NEXT FROM INNER_CURSOR
END
Orkun Balkancı