What's wrong with my TRIGGER?

[ARPRINCE]

I am creating a JOBNO using a trigger (VB front end). The code below works fine. However, when I insert a new record (via SPROCS) or manually and the JOBTYPE is not 'TAL', it always replaces the assigned value for the JOBNO to null.

I removed the trigger and my SPROCS and manual entry works fine. Any ideas would be appreciated.

Thank you.

CREATE TRIGGER tr_CREATE_TATL_JOBNO ON [WorkOrder] 
FOR INSERT
AS

Declare @JOBNO varchar(10)
Declare @JobType varchar(3)
Declare @WorkOrderID int
Declare @VRUTimeLine varchar(7)

SELECT @JobType = JobType FROM inserted
SELECT @WorkOrderID=WorkOrderID FROM inserted
SELECT @VRUTimeLine = VRUTimeLine FROM inserted

IF @JobType="TAL" 
	IF LEN(LTRIM(STR(@WorkOrderID))) < 6
		BEGIN
			SET @JOBNO = LEFT(@VRUTimeLine,2) + RIGHT(@VRUTimeLine,2) +  REPLICATE('0',6-LEN(LTRIM(STR(@WorkOrderID)))) + LTRIM(STR(@WorkOrderID))
		END
	ELSE
		BEGIN
			SET @JOBNO =  LEFT(@VRUTimeLine,2) + RIGHT(@VRUTimeLine,2) + RIGHT(STR(@WorkOrderID),6)
		END		
	
UPDATE WorkOrder SET JobID=@JOBNO WHERE WorkOrderID=@WorkOrderID

[SheldonPenner]

If any of those values you are getting from the inserted table are coming up as Nulls, then your @JOBNO will evaluate to Null too. Maybe you need to insert them inside an IsNull function.

And, of course, if the value of @JobType is anything other than 'TAL', then your code that sets the @JOBNO value will never run at all.

[techgnome]

That's because @JOBNO isn't getting set if @JobType <> 'TAL'

[ARPRINCE]

Thanks for the replies.

You see, I have 2 FORMS. The first form inserts a record with a job type of TAL. If the job type is not 'TAL', then it would do nothing, otherwise, trigger creates the JOB NUMBER. No problem here since it works well and good.

My second form inserts a record with a job type <> to 'TAL' using SPROCS. The SPROC has a JOB NUMBER parameter as input.

Basically, when the SPROC is called, it inserts record with a JOB TYPE and JOB NUMBER. It runs the trigger. Trigger sees that the inserted record's job type is not 'TAL'. So, it should have not done anything.

However, contrary to my assumption, the JOB NUMBER parameter input never registers. The NULL value gets inserted instead. If I remove the trigger, JOB NUMBER parameter input gets inserted so I figured I'm missing something on my trigger.

Also, no NULL values are inserted by the front end.

Any ideas? TNX

[brucevde]

It runs the trigger. Trigger sees that the inserted record's job type is not 'TAL'. So, it should have not done anything.

But it is doing something! Your Update WorkOrder statement is always executed, regardless the value of @JobType,

If @JobType <> "TAL", then @JobNo will be Null and the Update WorkOrder statement will overwrite the value placed there by the stored procedure.

If you only want the Update WorkOrder statement to execute when @JobNo = "TAL" then it must be part of the if statement. In SQL Server only the first line after an IF statement is considered part of the IF, unles BEGIN ... END are used.

CREATE TRIGGER tr_CREATE_TATL_JOBNO ON [WorkOrder] 
FOR INSERT
AS

Declare @JOBNO varchar(10)
Declare @JobType varchar(3)
Declare @WorkOrderID int
Declare @VRUTimeLine varchar(7)

SELECT @JobType = JobType FROM inserted
SELECT @WorkOrderID=WorkOrderID FROM inserted
SELECT @VRUTimeLine = VRUTimeLine FROM inserted

IF @JobType="TAL" 
    BEGIN
       IF LEN(LTRIM(STR(@WorkOrderID))) < 6
           BEGIN
                SET @JOBNO = LEFT(@VRUTimeLine,2) + RIGHT(@VRUTimeLine,2) +  REPLICATE('0',6-LEN(LTRIM(STR(@WorkOrderID)))) + LTRIM(STR(@WorkOrderID))
           END
       ELSE
           BEGIN
               SET @JOBNO =  LEFT(@VRUTimeLine,2) + RIGHT(@VRUTimeLine,2) + RIGHT(STR(@WorkOrderID),6)
       END		
	
       UPDATE WorkOrder SET JobID=@JOBNO WHERE WorkOrderID=@WorkOrderID
   END

Note that your trigger code contains a potentially serious bug. Since your Insert trigger will only work if one row is inserted into the database at a time.

If someone executes a statement like Insert Into WorkOrder Select * From TableBackup, all sorts of corrupt data may be added to your database. Actually, now that I think about it, the statement SELECT @JobType = JobType FROM inserted, may cause the whole transaction to abort if the Inserted table contains more than one record.

Always make sure Triggers handle the possibility of multiple records being accessed. Triggers are only executed once per action (insert,update, delete) not once for each row being modified by the action.

[ARPRINCE]

If you only want the Update WorkOrder statement to execute when @JobNo = "TAL" then it must be part of the if statement. In SQL Server only the first line after an IF statement is considered part of the IF, unles BEGIN ... END are used.

Thank you!!

This is exactly what I needed. You see, I'm not really familiar with SQL's syntax. I tried closing my IF with END IF or just with an END but...well...ofcourse it didn't work.

Note that your trigger code contains a potentially serious bug. Since your Insert trigger will only work if one row is inserted into the database at a time.

If someone executes a statement like Insert Into WorkOrder Select * From TableBackup, all sorts of corrupt data may be added to your database. Actually, now that I think about it, the statement SELECT @JobType = JobType FROM inserted, may cause the whole transaction to abort if the Inserted table contains more than one record.

Yes, the program is inserting one row at a time with JOBTYPE ='TAL'. I inserted 114 records (bulk) and it worked just fine too although the JOBTYPE <> 'TAL'.

However, as you pointed out, I may have serious problems when I do try to reinsert bulk records (i.e. backups) and those records have a 'TAL' jobtype.

Thank you for pointing that out. I do think I need to redesign my code and would most likely get rid of the trigger instead.

** edit ***

OK. I inserted bulk (112) records with a 'TAL' jobtype and an existing JOB NUMBER simulating a backup restore. It was actually successful (the trigger was not able to change the JOB NUMBER).
I'll do a little more scenarios and double check what happens.