Serenity
Serenity

Reputation: 5098

Stored procedure won't return 0

this is my sp

CREATE PROCEDURE DeteleTitle
(
@TitleID int
)
AS
BEGIN
IF EXISTS(Select TitleID from Titles WHERE TitleID=@TitleID)
BEGIN
DELETE FROM Titles WHERE  TitleID=@TitleID
SELECT @TitleID
END
ELSE
BEGIN
SELECT 0
END
END

Method where I am calling it is:-

public Int32 DeleteTitle(Int32 TitleID)
{
try
{
int ds=0;
SqlParameter[] sqlparam=new SqlParameter[1];
sqlparam[0]=(@TitleID,TitleID);
ds=Convert.ToInt32(SqlHelper.ExecuteScalar(ConfigurationManager.ConnectionStrings["con"].ConnectionString,CommandType.StoredProcedure,"DeleteTitle",sqlparam).Tables[0]);
return ds;
}

catch(Exception ex)
{
return 0;
}
}

Now TitleID is a Foreign Key in many Tables. If some Table's record is using TitleID, then it throws this exception that says "Foreign Key Violation n stuff". In my Stored procedure above, I am selecting zero in the else block in case delete is not successful. When delete IS successful, it returns TitleID's value like 50, 99 or whatever. Now what is happening is , when delete is not successful, it is not returning zero. I wanted a message to be displayed on screen based on this zero value returned by Delete Stored procedure but when it didn't return any value (when delete failed), I returned zero in the catch block of my DeleteTitle() method.

Now I have two questions:-

  1. Why did the Stored procedure not return zero when delete failed?
  2. Is returning zero in the catch block like I have done above the right way? I didn't know how you retrieve Foreign key Exception number and stuff so I just returned zero there in catch block.

Upvotes: 4

Views: 995

Answers (5)

Biplab Sarker
Biplab Sarker

Reputation: 13

You can use @@ERROR for the output result. @@ERROR = 0 mean successful else unsuccessful operation

CREATE PROCEDURE DeteleTitle
(
    @TitleID int
)
AS

BEGIN
    IF EXISTS(Select TitleID from Titles WHERE TitleID=@TitleID)
    BEGIN
        DELETE FROM Titles WHERE  TitleID=@TitleID      
    END

    Select @@ERROR

END

Upvotes: 0

KM.
KM.

Reputation: 103579

use this:

CREATE PROCEDURE DeteleTitle
(
@TitleID int
)
AS
BEGIN TRY
    DELETE FROM Titles WHERE TitleID=@TitleID
    SELECT CASE 
               WHEN @@ROWCOUNT>0 THEN @TitleID 
               ELSE 0 --row did not exist
           END
END TRY
BEGIN CATCH
    SELECT 0 --delete failed
END CATCH
go

When multiple tables are "linked" via foreign keys and you delete a parent row you get an error like you are reporting, because the child data can't exist without a parent. You might want to look into cascade deletes, or add code in this procedure to delete from the tables that are associated with Titles via foreign keys. Add these deletes before the DELETE FROM Titles. do it like this:

CREATE PROCEDURE DeteleTitle
(
@TitleID int
)
AS
BEGIN TRY
    BEGIN TRANSACTION

    DELETE FROM YourOtherTablesA WHERE TitleID=@TitleID
    DELETE FROM YourOtherTablesB WHERE TitleID=@TitleID

    DELETE FROM Titles WHERE TitleID=@TitleID
    SELECT CASE 
               WHEN @@ROWCOUNT>0 THEN @TitleID 
               ELSE 0 --row did not exist
           END
    COMMIT
END TRY
BEGIN CATCH
    IF XACT_STATE()!=0
    BEGIN
        ROLLBACK TRANSACTION
    END
    SELECT 0 --delete failed
END CATCH
go

Upvotes: 2

Andomar
Andomar

Reputation: 238048

If Tables is a collection of tables, you'd need another [0] for the first column of the first table.

Upvotes: 2

Dave Swersky
Dave Swersky

Reputation: 34810

The problem is that your if statement will not execute the ELSE statement if it fails with an exception. Your IF statement also appears incorrect- shouldn't it be IF EXISTS, [then delete record?] The way it's written now, if the record exists it will NOT be deleted.

The extended problem is that it is considered bad practice to rely on an exception (in C#, SQL, or any other language) as a method for flow control.

You would be better off explicitly checking for related records by using the EXISTS statement for each related table.

Upvotes: 3

Joe Stefanelli
Joe Stefanelli

Reputation: 135729

You'd want a TRY...CATCH in your procedure, not an IF...ELSE.

If you think about it, you're already in the IF portion of your statement when the DELETE fails with the foreign key violation. How could your code possibly jump to the ELSE block?

Upvotes: 3

Related Questions