Error checking and GOTO

[DerPflug]

I'm using transactions for the first time in the context of a stored procedure and I have a question. Using the following code:

DECLARE	@Count int,	
@ID int
SELECT @ID = ID FROM MyTable WHERE blah, blah, blah
SET @Count = @@ROWCOUNT
IF @Count = 0   
  BEGIN	
  SET @ResultMessage = 'There was no match' 	
  SET @Confirmation = 1  
  END
ELSE  --if it exists update fields  
  BEGIN	
    BEGIN TRAN	
     UPDATE AnotherTable SET This = That WHERE ID = @ID	
     IF @@ERROR <> 0 --if transaction fails rollback 
       transaction	  
      BEGIN		
          ROLLBACK TRAN		
          GOTO on_error	  
      END	
          COMMIT TRAN	
          SET @Confirmation = 0        
          SET @ResultMessage = 'Committed    successfully'  

  END
on_error:  
SET @Confirmation = 1
SET @ResultMessage = 'Failed and was rolled back'
GO

This always runs the on_error label code. I have the followinvnng solution:

DECLARE	@Count int,
@ID int
SELECT @ID = ID FROM MyTable WHERE blah, blah, blah 
SET @Count = @@ROWCOUNT
IF @Count = 0 --if does not exist send back error code      
  BEGIN	
    SET @ResultMessage = 'There was no match in the      
       database' 	
    SET @Confirmation = 1  
  END
ELSE  --if it exists update fields  
  BEGIN	
    BEGIN TRAN	
    UPDATE AnotherTable SET this = that WHERE ID = @ID	
       IF @@ERROR <> 0 --if transaction fails rollback 
            transaction	  
          BEGIN		
             ROLLBACK TRAN		
             GOTO on_error	  
          END	
             COMMIT TRAN	
             GOTO proc_end  
  END

on_error:  
SET @Confirmation = 1
SET @ResultMessage = 'Failed and was rolled back'

proc_end:
SET @Confirmation = 0
SET @ResultMessage = 'Committed successfully'
GO

What I need to know is whether or not it is within good stored procedure coding practices. Thanks in advance.

 

[nigelrivett]

This is a single statement so will automatically be atomic and there is not need for the explicit transaction

The code you have will always execute proc_end. You need a "return" after the error processing.

I would use
if @id is null rather than the rowcount.

You could do it all in one statement

DECLARE @Rowcount int ,
@Error int

UPDATE AnotherTable
SET This = That
FROM MyTable
WHERE blah, blah, blah
and AnotherTable.ID = MyTable.ID

select @Error = @@error, @Rowcount = @@rowcount -- this has to be a single statement
if @Error <> 0 or @Rowcount = 0
begin
SET @Confirmation = 1
SET @ResultMessage = 'Failed and was rolled back'
end
else
begin
SET @Confirmation = 0
SET @ResultMessage = 'Committed successfully'
end


======================================
Cursors are useful if you don't know sql.
DTS can be used in a similar way.
Beer is not cold and it isn't fizzy.

 

[DerPflug]

Something like this?

SELECT @ID = ID FROM MyTable WHERE filter information
IF @ID IS NULL 
  BEGIN
	SET @Confirmation = 0
	SET @ResultMessage = 'There was no match in the database'
  END
ELSE  --if it exists update fields
  BEGIN
	UPDATE MyTable SET blah, blah, blah WHERE ID = @ID
	SELECT @Error = @@Error, @Count = @@ROWCOUNT
	IF @Error <> 0  OR @Count = 0 --if transaction fails rollback transaction
	  BEGIN
		SET @Confirmation = 0
	  	SET @ResultMessage = 'Failed and was rolled back'
	  END
	ELSE
	  BEGIN
		SET @Confirmation = 1
		SET @ResultMessage = 'Committed successfully'
	  END
  END