May 8, 2007 at 3:53 am
Hi all,
this is my question:
i have a table with this trigger
CREATE TRIGGER [insertSuLavoro] ON dbo.T_Lavoro
AFTER INSERT
AS
DECLARE @IDinserito INT
DECLARE @IDcommessa sql_variant
DECLARE @IDoffertaOrig sql_variant
SET @IDinserito =
(SELECT ID
FROM inserted)
SET @IDcommessa =
(SELECT IDcommessaOrig
FROM inserted)
SET @IDoffertaOrig =
(SELECT IDoffertaOrig
FROM inserted)
DECLARE @IDcommessaAsInt int
DECLARE @IDoffertaOrigAsInt int
SET @IDcommessaAsInt = CAST(@IDcommessa as int)
SET @IDoffertaOrigAsInt = CAST(@IDoffertaOrig as int)
RAISERROR ('@IDcommessa %d', 16, 1, @IDcommessaAsInt)
RAISERROR ('@IDoffertaOrig %d', 16, 1, @IDoffertaOrigAsInt)
if (@IDcommessa != null)
begin
RAISERROR ('@IDcommessa %d', 16, 1, @IDcommessaAsInt)
UPDATE T_Lavoro
SET IDswCollegato = @IDinserito
WHERE ID = @IDcommessa
end
else
begin
RAISERROR ('@IDcommessa null ', 16, 1)
end
if ( @IDoffertaOrig != null)
begin
RAISERROR ('@IDoffertaOrig %d', 16, 1, @IDoffertaOrigAsInt)
UPDATE T_Lavoro
SET IDswCollegato = @IDinserito
WHERE ID = @IDoffertaOrig
end
else
begin
RAISERROR ('@IDoffertaOrig null ', 16, 1)
end
when i try to execute this statement:
INSERT INTO t_lavoro(IDbu, nome, idoffertaorig) VALUES(1, 'aa', 346)
i obtain this output and the update isn't done.
Server: Msg 50000, Level 16, State 1, Procedure insertSuLavoro, Line 26
@IDcommessa 0
Server: Msg 50000, Level 16, State 1, Procedure insertSuLavoro, Line 27
@IDoffertaOrig 346
Server: Msg 50000, Level 16, State 1, Procedure insertSuLavoro, Line 39
@IDcommessa null
Server: Msg 50000, Level 16, State 1, Procedure insertSuLavoro, Line 51
@IDoffertaOrig null
(1 row(s) affected)
why the comparisons fail?
thanks a lot for any kind of help!
Alan
May 8, 2007 at 3:59 am
Couple problems here. The first, and the one that you're noticing is that you don't do comparisons to NULL using = or !=. The rules of NULL comparisons say that both of the following return NULL (rather than true or false)
IF (@var = NULL)
IF (@var != NULL)
The corect way is to use the IS NULL expression. So one of your comparisons would be
if (@IDcommessa IS NOT NULL)
The second, less obvious problem is that your trigger is written assuming that there will only be a single row in the inserted table. Triggers fire once for an operation, so if an insert occurs that inserts 10 rows, the inserted table will contain 10 rows.
[Edit] Just noticed a 3rd issue. Never use the sql_variant data type.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
May 8, 2007 at 4:11 am
Thank you gail, you are great!
i've fixed the problem with the first suggestion.
about the second: i don't think the problem is goig to happen in my app, but in the future i'll remember your suggestion
about the third: i've replaced sql_variant with int
again thank you very much for your great and fast reply!
May 8, 2007 at 4:15 am
Pleasure.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
May 10, 2007 at 1:51 am
There are other issues with your trigger
SET @IDinserito =
(SELECT ID
FROM inserted)
Assumes the inserted table will only have one row of data. Triggers should always be written from the perspective the inserted or deleted tables will have, zero, one or more rows. If you do this your code will be more robust.
May 10, 2007 at 2:00 am
thankyou for your suggestion.
actually i've done a little change: now i iterate throw iserted data, and for each one i do all things.
thanks anyway
bye, Alan
May 10, 2007 at 2:08 am
Iterating through rows is never a good idea in SQL. Set-based operations work much, much faster.
Could you post your revised trigger here please? Might be able to get something more optimal for you.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
May 10, 2007 at 2:33 am
i've not the code under my eyes, but i've done something like this
DECLARE @IDinserito INT
DECLARE @IDcommessa INT
DECLARE @IDoffertaOrig INT
declare CurIns cursor for select Id, IDcommessaOrig, IDOffertaOrig from inserted
open CurIns
set @Body=''
Fetch Next From CurIns into @IDinserito, @IDcommessa, @IDoffertaOrig
WHILE (@@FETCH_STATUS -1)
BEGIN
if (@IDcommessa IS NOT NULL)
begin
--RAISERROR ('@IDcommessa %d', 16, 1, @IDcommessa)
UPDATE T_Lavoro
SET IDswCollegato = @IDinserito
WHERE ID = @IDcommessa
end
if ( @IDoffertaOrig IS NOT NULL)
begin
--RAISERROR ('@IDoffertaOrig %d', 16, 1, @IDoffertaOrig)
UPDATE T_Lavoro
SET IDswCollegato = @IDinserito
WHERE ID = @IDoffertaOrig
end
Fetch Next From CurIns into @IDinserito, @IDcommessa, @IDoffertaOrig
end
Close CurIns
DEALLOCATE CurIns
please tell me if it is the correct idea.
thank you very much!
Alan
May 10, 2007 at 2:44 am
Ow.
Cursors are never a good way of doing things in SQL. Set-based operations are far more optimal. Try something like this (untested).
CREATE TRIGGER ....
AS
UPDATE T_Lavoro
SET IDswCollegato = i.ID
FROM inserted i
WHERE T_Lavoro.ID = i.IDcommessaOrig
UPDATE T_Lavoro
SET IDswCollegato = i.ID
FROM inserted i
WHERE ID = IDOffertaOrig
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
May 10, 2007 at 2:54 am
as soon as possible i'll try in the way you suggested me, but i'm sure it will work properly!
thanks a lot, Alan
Viewing 10 posts - 1 through 10 (of 10 total)
You must be logged in to reply to this topic. Login to reply