CODEWITHSUNDEEP
sql-servert-sqldatabase-trigger
Ravishkumar patil
Ravishkumar patil

Reputation: 31

How do i correct this trigger?

I have implemented a trigger on a table which makes sure that table does not contain more than one row with 1 as bit value in one of its column. But the trigger somehow is not working fine.

create trigger [dbo].[flyingdutchman_needs_a_captain]
on [dbo].[flyingdutchman]
after insert, update, delete as
begin
set nocount on;
set rowcount 0;
begin try
declare @captainsum int
set @captainsum = 0     
set @captainsum = (select sum(case when uniontable.iscaptain = 1 then 1 else 0 end) 
from (
    select iscaptain from dbo.flyingdutchman 
    union all
    select inserted.iscaptain as iscaptain from inserted
    union all
    select deleted.iscaptain as iscaptain from deleted
    ) as uniontable
having sum(case when uniontable.iscaptain = 1 then 1 else 0 end)  <> 1)     
if(@captainsum <> 1)
begin
    throw 50000,'flying dutchman is cursed and needs a captain.',1;
end     
end try
begin catch
if xact_state()<>0
    rollback transaction;
throw;
end catch
end

flyingdutchman is a table with three columns:

SailorId(int),SailorName(varchar),IsCaptain(bit)

and the following rows:

enter image description here

When executing the query:

insert into dbo.flyingdutchman(Sailorname,IsCaptain)
values('Davy Jones',1)

I get the error:

Msg 50000, Level 16, State 1, Procedure FLYINGDUTCHMAN_NEEDS_A_CAPTAIN, Line xx FLYING DUTCHMAN IS CURSED AND NEEDS A CAPTAIN.

I expect that this query should run fine and trigger should not get fired.

Upvotes: 2

Views: 61

Answers (3)

masoud
masoud

Reputation: 478

your trigger always get fire , because your condition if(@captainsum <> 1) is always true when your summary in sub query equals 1 , the @captainsum = null because of your having statement , then your variable is null or other values except 1 

create trigger [dbo].[flyingdutchman_needs_a_captain]
on [dbo].[flyingdutchman]
after insert, update, delete as
begin
set nocount on;
set rowcount 0;
begin try
declare @captainsum int
set @captainsum = 0     
set @captainsum = (select sum(case when uniontable.iscaptain = 1 then 1 else 0 end) 
from (
    select iscaptain from dbo.flyingdutchman where SailorId not in (select SailorId from inserted)
    union all
    select inserted.iscaptain as iscaptain from inserted
    union all
    select deleted.iscaptain as iscaptain from deleted
    ) as uniontable
having sum(case when uniontable.iscaptain = 1 then 1 else 0 end)  <> 1)     
if(@captainsum <> 1)
begin
    throw 50000,'flying dutchman is cursed and needs a captain.',1;
end     
end try
begin catch
if xact_state()<>0
    rollback transaction;
throw;
end catch
end

Upvotes: 2

Alexander Volok
Alexander Volok

Reputation: 5940

Can be simplified to:

create trigger [dbo].[flyingdutchman_needs_a_captain]
on [dbo].[flyingdutchman]
after insert, update, delete as
begin
set nocount on;

begin try   

-- prevents multiple captains
IF  (SELECT COUNT(*) FROM [dbo].[flyingdutchman] WHERE iscaptain = 1   ) > 1 
    throw 50000,'Too many captains is not good.',1;

-- prevents setting all captains off, if one was already assigned
IF NOT EXISTS(SELECT * FROM [dbo].[flyingdutchman] WHERE iscaptain = 1   )
   AND EXISTS(SELECT * FROM deleted WHERE iscaptain = 1   ) 
    throw 50001,'flying dutchman is cursed and needs a captain.',1;


end try
begin catch
if xact_state()<>0
    rollback transaction;
throw;
end catch

end

Upvotes: 2

Zohar Peled
Zohar Peled

Reputation: 82554

Since this is an after trigger, all the changes are already in the table - you don't need to query the inserted and deleted tables - that's the reason you get the wrong result.

You can simply do this:

if 1 <> (select count(*) from dbo.flyingdutchman where iscaptain = 1)
    throw 50000,'flying dutchman is cursed and needs a captain.',1;

However, I suspect you should probably do something like this instead:

declare @captainsCount int;
select @captainsCount = count(*) from dbo.flyingdutchman where iscaptain = 1
if @captainsCount = 0 
    throw 50000,'flying dutchman is cursed and needs a captain.',1;
if @captainsCount > 1 
    throw 50000,'flying dutchman has too many captains.',1;

Upvotes: 2

Related Questions