SLLLOOOWWW Trigger, what's wrong?(Still)

Please start any new threads on our new site at https://forums.sqlteam.com. We've got lots of great SQL Server experts to answer whatever question you can come up with.

All Forums

SQL Server 2000 Forums

SQL Server Development (2000)

SLLLOOOWWW Trigger, what's wrong?(Still)

Author

Topic

steamngn
Constraint Violating Yak Guru

306 Posts

Posted - 2006-09-29 : 12:36:08

Here we go again...
This trigger is incredibly slow; I'm sure I have something wrong.
Also, for some reason @MSG is returning 'Zone charge on ticket does not match Ship to zone!' even when they DO match. Is something wrong with my select query, or is the 'else' statement at the end wrong?

set ANSI_NULLS ON
set QUOTED_IDENTIFIER ON
GO

ALTER TRIGGER [dbo].[enforcezonecharges] ON [dbo].[PS_TKT_LIN]
FOR INSERT,UPDATE
as
set nocount on

declare @MSG varchar(1000)

if exists(select  l.item_no
from ps_tkt_hdr h
left outer join zip_cod_zones z
on h.ship_zip_cod=z.zip_cod
left outer join ps_tkt_lin l
on h.str_id=l.str_id
and h.sta_id=l.sta_id
and h.tkt_no=l.tkt_no
inner join inserted i
on l.str_id=i.str_id
and l.sta_id=i.sta_id
and l.tkt_no=i.tkt_no
and l.item_no=i.item_no
inner join im_item p
on l.item_no=p.item_no
where h.is_svc_call='Y'
and l.subcat_cod='labor'
and p.svc_zone not in ('00','77','88','99')
and p.svc_zone<>z.svc_zone
and z.svc_zone is not null)

set @MSG='Zone charge on ticket does not match Ship to zone!'
else if exists(
select l.item_no
from ps_tkt_hdr h
left outer join zip_cod_zones z
on h.ship_zip_cod=z.zip_cod
left outer join ps_tkt_lin l
on h.str_id=l.str_id
and h.sta_id=l.sta_id
and h.tkt_no=l.tkt_no
inner join inserted i
on l.str_id=i.str_id
and l.sta_id=i.sta_id
and l.tkt_no=i.tkt_no
and l.item_no=i.item_no
inner join im_item p
on l.item_no=p.item_no
where h.is_svc_call='Y'
and l.subcat_cod='labor'
and p.svc_zone ='88'
and z.svc_zone is not null
and z.svc_zone>'03')
and not exists(select  m.item_no
from ps_tkt_hdr n
left outer join zip_cod_zones y
on n.ship_zip_cod=y.zip_cod
left outer join ps_tkt_lin m
on n.str_id=m.str_id
and n.sta_id=m.sta_id
and n.tkt_no=m.tkt_no
inner join inserted i
on m.str_id=i.str_id
and m.sta_id=i.sta_id
and m.tkt_no=i.tkt_no
and m.item_no=i.item_no
inner join im_item o
on m.item_no=o.item_no
where n.is_svc_call='Y'
and m.subcat_cod='labor'
and o.svc_zone ='77'
and y.svc_zone is not null)

set @MSG='Out of Normal Service Area: Travel Charge needed on this warranty ticket!'
else
set @MSG=null
if @MSG is not null
begin
rollback tran
raiserror(@MSG,18,1)
end
set nocount off

For reference, here is a trigger that functions ok, but only works with one row of data (The reason for the trigger above):

set ANSI_NULLS ON
set QUOTED_IDENTIFIER ON
GO

ALTER TRIGGER [dbo].[enforcezonecharges] ON [dbo].[PS_TKT_LIN]
FOR INSERT,UPDATE

AS
set nocount on
/*declare local variables--*/
declare
@ZON T_FLG2,
@ZIP T_ZIP_COD,
@SVCFLG T_FLG2,
@Msg T_ERR_REF

select
@ZON=(select svc_zone from im_item where im_item.item_no=i.item_no),
@ZIP=(select ship_zip_cod
from ps_tkt_hdr
where ps_tkt_hdr.tkt_no=i.tkt_no
and ps_tkt_hdr.str_id=i.str_id
and ps_tkt_hdr.sta_id=i.sta_id),
@SVCFLG=(select IS_SVC_CALL
from ps_tkt_hdr
where ps_tkt_hdr.tkt_no=i.tkt_no
and ps_tkt_hdr.str_id=i.str_id
and ps_tkt_hdr.sta_id=i.sta_id)
from inserted i
where i.subcat_cod='LABOR'

/*make sure zone charge has been set if service related */
if @SVCFLG='Y' and @ZON not in ('00','77','88','99') and @ZON<>(select svc_zone from zip_cod_zones where zip_cod_zones.zip_cod=@ZIP)
begin
rollback
raiserror('Zone charge on ticket does not match Ship to zone!',18,1)
end

Help! Andy

There's never enough time to type code right,
but always enough time for a hotfix...

Kristen
Test

22859 Posts

Posted - 2006-09-29 : 14:49:29

"Help!"

Help indeed!

This is a mess I'm afraid ...

You have a LEFT OUTER JOIN to PS_TKT_LIN and then INNER JOIN that to INSERTED - that won't work (well it will, but it breaks the Outer Join). And then you have a WHERE clause condition on PS_TKT_LIN - so if no row is found for PS_TKT_LIN (Outer Join), and thus subcat_cod Is Null, then the WHERE clause is going to throw that record away L.subcat_cod='labor' - so the Outer Join is bust again.

I would suggest you start with

FROM inserted

and the Join tables from there outwards. The INSERTED pseudo table is going to have one row for each record changed, so that's the thing to base your criteria on.

If you use an Outer Join make sure there are NO references to it in the WHERE clause (with the exception of a test for IS NULL if you are using the OUTER JOIN as a substitute for Exists)

Consider putting your WHERE clause items in the conditions of the JOIN - might make it easier to see what is going on - so, for example, instead of:


	INNER JOIN im_item AS P
		on L.item_no=P.item_no
WHERE	    ...
	AND P.svc_zone not in ('00','77','88','99')

consider


	INNER JOIN im_item AS P
		 ON L.item_no = P.item_no
		AND P.svc_zone NOT IN ('00','77','88','99')
WHERE	    ...

Kristen

X002548
Not Just a Number

15586 Posts

Posted - 2006-09-29 : 16:38:00

Can you strat from the beginning?

What are you trying to accomplish?

Read the hint link in my sig (the one sans margarita)

Brett

8-)

Hint: Want your questions answered fast? Follow the direction in this link
http://weblogs.sqlteam.com/brettk/archive/2005/05/25/5276.aspx

Add yourself!
http://www.frappr.com/sqlteam

steamngn
Constraint Violating Yak Guru

306 Posts

Posted - 2006-09-30 : 09:03:07

Hi Guys,
Kristen, You are so right about this being a mess....
Here is what we are trying to accomplish, and the silly ass problems that are bringing this about:
We have an App that is used to enter service tickets for field repair. our service areas are listed in zip_cod_zones, and any area >03 has travel charges for warranty work. What we need to have happen is twofold: first, we need to make sure that non-warranty labor is billed at the correct rate for the ship zip code on the ticket header. second, we need to make sure that any ticket that has a ship zip code >03 AND has warranty labor as a ticket line also has travel charge labor as a line item on the ticket.
Now, our app has a quirk:
When a ticket is first created, it is saved using normal INSERT operations into PS_TKT_HDR(header) and corresponding PS_TKT_LIN(lines). BUT when an existing ticket is edited, the app grabs the header and line data with a proc and then deletes everything from the two tables before re-inserting it. This creates all kinds of headaches, not the least of which is that update triggers never seem to function as planned. My coding isn't always the best(Do tell!

), but this quirck has given grief to the gods and goddesses here before when I've posted. Kristen, you've been so helpful I think I should be throwing money instead of accolades(not to mention the so many other helpful souls)! If it helps in any way, most of the other users of this app that I've spoken with use atrigger that contains a cursor when they are working with PS_TKT_LIN. Note that the poorly written code that I posted returns the proper results in QA; so even though written by a caged monkey on a drinking binge it should be rebuildable to do the job(right, like the 6 million dollar man!

)
Andy

There's never enough time to type code right,
but always enough time for a hotfix...

Kristen
Test

22859 Posts

Posted - 2006-09-30 : 14:44:08

If the App Deletes and Inserts could you store the relevant detail in a "holding table" via a DELETE trigger and have the INSERT trigger check for stuff in the "holding" table, and put back whatever was needed?

A regular Insert would find nothing in the Holding Table of course.

I think your performance issues probably stem from the fact that the statement is written with Outer Joins which actually aren't and so on, hence my suggestion to recode it starting with "FROM inserted" and join everything from there - that approach should more logically arrange the tables that are in some way related to "inserted"

Kristen

steamngn
Constraint Violating Yak Guru

306 Posts

Posted - 2006-09-30 : 14:45:29

Ok,
so are we talking about something like this?:

set nocount on

declare @MSG varchar(1000)

if exists(select  l.item_no
from inserted i
inner join ps_tkt_lin l
on l.str_id=i.str_id
and l.sta_id=i.sta_id
and l.tkt_no=i.tkt_no
and l.item_no=i.item_no
and l.subcat_cod='labor'
inner join im_item p
on l.item_no=p.item_no
and p.svc_zone not in ('00','77','88','99')
left outer join ps_tkt_hdr h
on h.str_id=l.str_id
and h.sta_id=l.sta_id
and h.tkt_no=l.tkt_no
left outer join zip_cod_zones z
on h.ship_zip_cod=z.zip_cod
and z.svc_zone<>p.svc_zone
where h.is_svc_call='Y'
and z.svc_zone is not null)
set @MSG='Zone charge on ticket does not match Ship to zone!'

else if exists(select  l.item_no
from inserted i
inner join ps_tkt_lin l
on l.str_id=i.str_id
and l.sta_id=i.sta_id
and l.tkt_no=i.tkt_no
and l.item_no=i.item_no
and l.subcat_cod='labor'
inner join im_item p
on l.item_no=p.item_no
and p.svc_zone ='88'
left outer join ps_tkt_hdr h
on h.str_id=l.str_id
and h.sta_id=l.sta_id
and h.tkt_no=l.tkt_no
left outer join zip_cod_zones z
on h.ship_zip_cod=z.zip_cod
and z.svc_zone>'03'
where h.is_svc_call='Y'
and z.svc_zone is not null)
and not exists(select  l.item_no
from inserted i
inner join ps_tkt_lin l
on l.str_id=i.str_id
and l.sta_id=i.sta_id
and l.tkt_no=i.tkt_no
and l.item_no=i.item_no
and l.subcat_cod='labor'
inner join im_item p
on l.item_no=p.item_no
and p.svc_zone ='77'
left outer join ps_tkt_hdr h
on h.str_id=l.str_id
and h.sta_id=l.sta_id
and h.tkt_no=l.tkt_no
left outer join zip_cod_zones z
on h.ship_zip_cod=z.zip_cod
where h.is_svc_call='Y'
and z.svc_zone is not null)

set @MSG='Out of Normal Service Area: Travel Charge needed on this warranty ticket!'
else
set @MSG=null
if @MSG is not null
begin
rollback tran
raiserror(@MSG,18,1)
end
set nocount off

Andy

There's never enough time to type code right,
but always enough time for a hotfix...

Kristen
Test

22859 Posts

Posted - 2006-09-30 : 15:24:28

[code]
if exists
(
select l.item_no
from inserted i
inner join ps_tkt_lin l
on l.str_id=i.str_id
and l.sta_id=i.sta_id
and l.tkt_no=i.tkt_no
and l.item_no=i.item_no
and l.subcat_cod='labor'
inner join im_item p
on l.item_no=p.item_no
and p.svc_zone not in ('00','77','88','99')
left outer join ps_tkt_hdr h
on h.str_id=l.str_id
and h.sta_id=l.sta_id
and h.tkt_no=l.tkt_no
left outer join zip_cod_zones z
on h.ship_zip_cod=z.zip_cod
and z.svc_zone<>p.svc_zone
where h.is_svc_call='Y'
and z.svc_zone is not null
)
[/code]
You've got H and Z as OUTER JOINs. You can't put a condition on them in there WHERE clause because inherently you have asked for an OUTER JOIN on H and Z, and that can return NULL for all columns - and if you put "h.is_svc_call='Y'" that will NOT match H.AnyColumn IS NULL from the Outer Join! (so in effect you are forcing it to an INNER JOIN - and I'm sure the query optimiser will thank you for changing the syntax if you do indeed find you actually need an Inner Join!

So either you want an INNER JOIN for H and Z, OR you need to move the WHERE clause criteria into the JOIN criteria for the relevant table - but that will probably return more rows that you are currently checking so WILL change the flow of your statement, so you'll need to test if that is what you actually want (perhaps you could insert any matching rows into a "debug table" as part of the Trigger code, so then you could see what had actually matched, AND you could check that you had successfully created a scenario where the OUTER JOIN was actually being properly exercise - i.e. the other tables matched a record, the Outer Join tables matched Nothing.

Kristen

steamngn
Constraint Violating Yak Guru

306 Posts

Posted - 2006-09-30 : 17:18:23

Oh! I get it! thank you for the excellent explanation...
I will putter around with this tonight, and post my attempts as they come along. Don't go too far away...

Andy

There's never enough time to type code right,
but always enough time for a hotfix...

steamngn
Constraint Violating Yak Guru

306 Posts

Posted - 2006-10-02 : 10:20:29

Kristen,
I am suuuuuuuuuuuuuuuuuuuuuuuuuuuuuch an idiot...
I am working on PS_TKT_LIN, which is the many table for PS_TKT_HDR; I don't need a left outer join, I need an inner join

!
Not only that, but if we only want to process records where zip_cod_zones.svc_zone is not null, that can also be an inner join, right?(see the green text below)

if exists
(
	select	l.item_no
	from	inserted i
		inner join ps_tkt_lin l
			on l.str_id=i.str_id
			and l.sta_id=i.sta_id
			and l.tkt_no=i.tkt_no
			and l.item_no=i.item_no
			and l.subcat_cod='labor'
		inner join im_item p
			on l.item_no=p.item_no
			and p.svc_zone not in ('00','77','88','99')
		inner join ps_tkt_hdr h
			on h.str_id=l.str_id
			and h.sta_id=l.sta_id
			and h.tkt_no=l.tkt_no
                        and h.is_svc_call='Y'
		inner join zip_cod_zones z
			on h.ship_zip_cod=z.zip_cod
			and z.svc_zone<>p.svc_zone
			and z.svc_zone is not null
)

So, if I apply this to the rest of the trigger, things may work just a BIT better, no?

Andy

There's never enough time to type code right,
but always enough time for a hotfix...

Kristen
Test

22859 Posts

Posted - 2006-10-02 : 12:10:33

based on how you've described it I agree, but you need to test it to make sure it delivers what you want.

What happens for PS_TKT_LIN records that have a ps_tkt_hdr record with a value in h.ship_zip_cod which cannot be found in zip_cod_zones ??

And what about PS_TKT_LIN records that have NO ps_tkt_hdr records?

And I suppose

PS_TKT_LIN that have no record in the im_item table?

Note that

and z.svc_zone<>p.svc_zone
and z.svc_zone is not null

the second line is superfluous here (unless you have a weird ANSI_NULLS setting!) because the test:

z.svc_zone<>p.svc_zone

will always be false z.svc_zone is null. It does no harm leaving it in, and I don't think it will confuse the query optimiser at all, so its a bit of a moot point.

Kristen

steamngn
Constraint Violating Yak Guru

306 Posts

Posted - 2006-10-02 : 15:43:57

quote:
What happens for PS_TKT_LIN records that have a ps_tkt_hdr record with a value in h.ship_zip_cod which cannot be found in zip_cod_zones ??

We want to ignore the record and let it pass. We current print a note on the ticket advising the user that there is no matching zip code zone on file.

quote:

And what about PS_TKT_LIN records that have NO ps_tkt_hdr records?

This will never happen, as the PS_TKT_HDR record must exist before the app can insert into PS_TKT_LIN.

quote:

And I suppose PS_TKT_LIN that have no record in the im_item table?

Users aren't allowed to add items through POS, and the only records they can insert is looked up from im_item.

quote:

Note that
and z.svc_zone<>p.svc_zone
and z.svc_zone is not null
the second line is superfluous here

Yup, I forgot to delete it when I posted. Got all excited about figuring some of this out on my own and then got all goofyfingers

Andy

There's never enough time to type code right,
but always enough time for a hotfix...

Kristen
Test

22859 Posts

Posted - 2006-10-03 : 01:18:07

"Got all excited about figuring some of this out on my own"

That's the way ...

steamngn
Constraint Violating Yak Guru

306 Posts

Posted - 2006-10-03 : 08:39:39

Well, yeah!!
I read about a billion posts on this site where people don't/won't even try to do the work; either that or they want someone to do their homework! I'm in the middle of school myself, and am trrrrrrryyyyyyyyyiiiiiinnnngg to get to the point where I can get certification. Long way to go yet, especially when doing multiple jobs (I own a martial arts school as well). It doesn't help that I am a one-man show as far as I.T. goes, makes progress painfully slow...
Andy

There's never enough time to type code right,
but always enough time for a hotfix...

steamngn
Constraint Violating Yak Guru

306 Posts

Posted - 2006-10-04 : 16:37:23

How's this look?

set ANSI_NULLS ON
set QUOTED_IDENTIFIER ON
GO

ALTER TRIGGER [dbo].[enforcezonecharges] ON [dbo].[PS_TKT_LIN]
FOR INSERT,UPDATE
as
set nocount on

declare @MSG varchar(1000)


if exists
(
	select	l.item_no
	from	inserted i
		inner join ps_tkt_lin l
			on l.str_id=i.str_id
			and l.sta_id=i.sta_id
			and l.tkt_no=i.tkt_no
			and l.item_no=i.item_no
			and l.subcat_cod='labor'
		inner join im_item p
			on l.item_no=p.item_no
			and p.svc_zone not in ('00','77','88','99')
		inner join ps_tkt_hdr h
			on h.str_id=l.str_id
			and h.sta_id=l.sta_id
			and h.tkt_no=l.tkt_no
                        and h.is_svc_call='Y'
		inner join zip_cod_zones z
			on h.ship_zip_cod=z.zip_cod
			and z.svc_zone<>p.svc_zone)



set @MSG='Zone charge on ticket does not match Ship to zone!'
else 
if exists
(
	select	l.item_no
	from	inserted i
		inner join ps_tkt_lin l
			on l.str_id=i.str_id
			and l.sta_id=i.sta_id
			and l.tkt_no=i.tkt_no
			and l.item_no=i.item_no
			and l.subcat_cod='labor'
		inner join im_item p
			on l.item_no=p.item_no
			and p.svc_zone ='88'
		inner join ps_tkt_hdr h
			on h.str_id=l.str_id
			and h.sta_id=l.sta_id
			and h.tkt_no=l.tkt_no
                        and h.is_svc_call='Y'
		inner join zip_cod_zones z
			on h.ship_zip_cod=z.zip_cod
			and z.svc_zone>'03')

and not exists
(
	select	l.item_no
	from	inserted i
		inner join ps_tkt_lin l
			on l.str_id=i.str_id
			and l.sta_id=i.sta_id
			and l.tkt_no=i.tkt_no
			and l.item_no=i.item_no
			and l.subcat_cod='labor'
		inner join im_item p
			on l.item_no=p.item_no
			and p.svc_zone ='77'
		inner join ps_tkt_hdr h
			on h.str_id=l.str_id
			and h.sta_id=l.sta_id
			and h.tkt_no=l.tkt_no
                        and h.is_svc_call='Y'
		inner join zip_cod_zones z
			on h.ship_zip_cod=z.zip_cod)

set @MSG='Out of Normal Service Area: Travel Charge needed on this warranty ticket!'
else
set @MSG=null
if @MSG is not null
begin
rollback tran
raiserror(@MSG,18,1)
end
set nocount off

Andy

There's never enough time to type code right,
but always enough time for a hotfix...

Kristen
Test

22859 Posts

Posted - 2006-10-05 : 01:40:07

Yup, looks like you are good-to-go with that.

In the [insert / update] trigger the INSERT pseudo table data will be identical [won't it?

] to the PS_TKT_LIN data (i.e. this is an AFTER trigger) , so you perhaps don't need to JOIN PS_TKT_LIN at all.

Kristen

steamngn
Constraint Violating Yak Guru

306 Posts

Posted - 2006-10-06 : 09:32:34

Good Morning...
Yup, thst is correct. The pseudo table is redundant. I will test,test,test the night away and let you know what happens!
(Man do I need some coffee...)
Andy

There's never enough time to type code right,
but always enough time for a hotfix...

Kristen
Test

22859 Posts

Posted - 2006-10-06 : 10:05:23

"The pseudo table is redundant"

Errmm ... its the other way round! You need INSERTED (to know what has been inserted) but the rows are identical to PS_TKT_LIN, so you don't also need PS_TKT_LIN.

"Man do I need some coffee..."

Second that!

Kristen

steamngn
Constraint Violating Yak Guru

306 Posts

Posted - 2006-10-10 : 20:29:13

!yletal tol a sdrawkcab gnipyt neeb ev'I
I knew what you meant, and took PS_TKT_LIN out, not INSERTED!

Andy

There's never enough time to type code right,
but always enough time for a hotfix...

steamngn
Constraint Violating Yak Guru

306 Posts

Posted - 2006-10-11 : 17:49:02

Unfortunately, I'm still having problems. I was having an issue with this new trigger conflicting somehow with another trgger on PS_TKT_LIN, so I tried combining them. That fixed one issue, but I still have another. Here is the complete trigger code:

set ANSI_NULLS ON
set QUOTED_IDENTIFIER ON
GO

ALTER TRIGGER [dbo].[enforcezonecharges] ON [dbo].[PS_TKT_LIN]
FOR INSERT,UPDATE,delete
as
set nocount on

declare @MSG varchar(1000)


if exists
(
	select	i.item_no
	from	inserted i
		inner join im_item p
			on i.item_no=p.item_no
			and p.svc_zone not in ('00','77','88','99')
		inner join ps_tkt_hdr h
			 on h.str_id=i.str_id
			and h.sta_id=i.sta_id
			and h.tkt_no=i.tkt_no
            and h.is_svc_call='Y'
		inner join zip_cod_zones z
			on h.ship_zip_cod=z.zip_cod
			and z.svc_zone<>p.svc_zone
where i.subcat_cod='labor')

set @MSG='Zone charge on ticket does not match Ship to zone!'

else 
if exists
(
	select	i.item_no
	from	inserted i
		inner join im_item p
			on i.item_no=p.item_no
			and p.svc_zone ='88'
		inner join ps_tkt_hdr h
			 on h.str_id=i.str_id
			and h.sta_id=i.sta_id
			and h.tkt_no=i.tkt_no
            and h.is_svc_call='Y'
		inner join zip_cod_zones z
			on h.ship_zip_cod=z.zip_cod
			and z.svc_zone>'03'
where i.subcat_cod='labor')

and not exists
(
	select	i.item_no
	from	inserted i
		inner join im_item p
			on i.item_no=p.item_no
			and p.svc_zone ='77'
		inner join ps_tkt_hdr h
			 on h.str_id=i.str_id
			and h.sta_id=i.sta_id
			and h.tkt_no=i.tkt_no
            and h.is_svc_call='Y'
		inner join zip_cod_zones z
			on h.ship_zip_cod=z.zip_cod
where i.subcat_cod='labor')

set @MSG='Out of Normal Service Area: Travel Charge needed on this warranty ticket!'
else
set @MSG=null
if @MSG is not null
begin
rollback tran
raiserror(@MSG,18,1)
end


/*When a row is inserted in table PS_TKT_LIN
set the ITEM_SHIP_DAT to PS_TKT_HDR.SHIP_DAT provided that:
a) no value was explicitly provided for ITEM_SHIP_DAT
b) there is a non-null SHIP_DAT value in PS_TKT_HDR
*/

update tl
set tl.TKT_SHIP_DAT = th.SHIP_DAT

from  PS_TKT_LIN tl
 join PS_TKT_HDR th
on tl.STR_ID = th.STR_ID
and tl.STA_ID = th.STA_ID
and tl.TKT_NO = th.TKT_NO



update tl
set tl.PROMPT_ALPHA_1 = th.PROF_ALPHA_1
from inserted i
inner join PS_TKT_LIN tl
on i.STR_ID = tl.STR_ID
and i.STA_ID = tl.STA_ID
and i.TKT_NO = tl.TKT_NO
and i.SEQ_NO = tl.SEQ_NO
inner join IM_ITEM th
on tl.ITEM_NO = th.ITEM_NO

update tl
set tl.PROMPT_ALPHA_2 = th.PROF_ALPHA_2
from inserted i
inner join PS_TKT_LIN tl
on i.STR_ID = tl.STR_ID
and i.STA_ID = tl.STA_ID
and i.TKT_NO = tl.TKT_NO
and i.SEQ_NO = tl.SEQ_NO
inner join IM_ITEM th
on tl.ITEM_NO = th.ITEM_NO

/* When SHIP_DAT is updated in table PS_TKT_HDR then update ITEM_SHIP_DATE
 in table PS_TKT_LIN under the following conditions:
a) There is a non-null SHIP_DAT value in PS_TKT_HDR
b) If tl.ITEM_SHIP_DAT is NULL then set it to th.SHIP_DAT
c) If tl.ITEM_SHIP_DAT is NOT NULL then only update if it matches old th.SHIP_DAT
*/

if update(TKT_SHIP_DAT)
update tl set
tl.ITEM_SHIP_DAT=tl.TKT_SHIP_DAT
from PS_TKT_LIN tl

join deleted d
on tl.STR_ID=d.STR_ID
and tl.STA_ID=d.STA_ID
and tl.TKT_NO=d.TKT_NO
where
tl.TKT_SHIP_DAT is not null
and (d.TKT_SHIP_DAT=tl.ITEM_SHIP_DAT or tl.item_ship_dat is null)
set nocount off

When a ticket has the wrong zone charge (blue) the trigger works fine. When the ticket has warranty labor and no travel charge (red) it still allows the ticket to save. I THINK this portion of the trigger needs brackets, but everything I've tried so far has come up short. Can you see what's missing?
Andy

There's never enough time to type code right,
but always enough time for a hotfix...

Kristen
Test

22859 Posts

Posted - 2006-10-13 : 05:13:15

Looks OK to me - i.e. the bracketing, but I can't speak for whether it fits your business logic or not! Stick some SELECT statements in there and run it from QA so you can see what gets output. You can run

BEGIN TRANSACTION
INSERT INTO dbo.PS_TKT_LIN(... columns ...)
VALUES(... values ...)
ROLLBACK

as many times as you like without mucking anything up (except that any IDENTITY columns will increment to the next-number regardless of the Rollback), so you can have several goes until the "debug" info comes good.

However I would make some "Defensive Programming" Coding Style suggestions:

ALWAYS put BEGIN ... END around your IF / ELSE logic


IF this
BEGIN
    ...
END
ELSE
IF that
BEGIN
    ...
END
ELSE
BEGIN
    ...
END

then if you accidentally, or otherwise, slip an extra statements in somewhere that will not effect the logic flow.

I suggest that you use EXISTS (SELECT * ... (instead of an explicitly named column) so that SQL Server can choose the most appropriate index for the query etc. I assume your i.item_no is NOT NULL at present, but if it IS able to be NULL (in the example you explain which is failing) that will effect the outcome of EXISTS (which SELECT * would not effect)

I think you should work on your indentation. I know it gets a bit mucked up cutting & pasting into here, but you've put a [code] tag around it, and therefore I reckon you are not religious about indentation - Get Religion! please

I don't know if this would work for you / help, but we do the JOIN criteria the other way round:


select	i.item_no
from	inserted AS i
	inner join im_item AS p
		 on p.item_no = i.item_no
		and p.svc_zone = '88'
	inner join ps_tkt_hdr AS h
		 on h.str_id = i.str_id
		and h.sta_id = i.sta_id
		and h.tkt_no = i.tkt_no
		and h.is_svc_call = 'Y'
	inner join zip_cod_zones AS z
		 on z.zip_cod = h.ship_zip_cod
		and z.svc_zone > '03'
	where i.subcat_cod = 'labor'

the aim is that we put the joined table's column on the left, so that we can more easily see that we have fulfilled the criteria for all the columns we need to.

I've also added AS because I think this accentuates the Alias name for the table - and if you do this as routine it prevents a pit-fall where you have missed a comma

FROM FOO F
and
FROM FOO , F
are NOT the same thing at all!!!

Similarly
SELECT FOO AS BAR
is the same as
SELECT FOO BAR
but in the second one you may have typed


SELECT FOO
       BAR

and meant


SELECT FOO ,
       BAR

and having the habit of putting "AS" does help spot that one, and


SELECT FOO AS xxx no comma
       BAR AS yyy

will raise a syntax error (but so will SELECT FOO xxx BAR yyy - its just an example of the style of defensive programmer we adopt here)

I also put some spaces around "=" and so on.

These are purely subjective, and what is important is that you are consistent about whatever style you adopt. But your current style is not very readable, and IME and IMHO that tends to lead to bugs that are less easily spotted.

Kristen

steamngn
Constraint Violating Yak Guru

306 Posts

Posted - 2006-10-13 : 09:34:41

Hey Kristen,
You are absolutely right about my formatting being lax...

I usually spend some time cleaning up what I write after the fact, which is the wrong way to go about it

(do it right the first time!).
Now, on to the failure issue:
I haven't tested this yet, but wouldn't it be better this way?

if exists
(
	select	*
	  from	inserted i
		inner join im_item p
			on i.item_no = p.item_no
			and p.svc_zone ='88'
                        and not exists
         (select	i.item_no
	  from	inserted i
		inner join im_item p
			on i.item_no = p.item_no
			and p.svc_zone = '77'
		inner join ps_tkt_hdr h
			 on h.str_id = i.str_id
			and h.sta_id = i.sta_id
			and h.tkt_no = i.tkt_no
            and h.is_svc_call = 'Y'
		inner join zip_cod_zones z
			on h.ship_zip_cod = z.zip_cod
          where i.subcat_cod = 'labor')

		inner join ps_tkt_hdr h
			 on h.str_id = i.str_id
			and h.sta_id = i.sta_id
			and h.tkt_no = i.tkt_no
            and h.is_svc_call = 'Y'
		inner join zip_cod_zones z
			on h.ship_zip_cod = z.zip_cod
			and z.svc_zone > '03'
            where i.subcat_cod = 'labor')

Moving the NOT EXISTS portion into the join makes more sense, right?
BTW, I really appreciate your taking the time to type out the rather lengthy and well detailed instructions. You make an excellent mentor! I owe you one!!(Proper formatting is the least I can do you for you!

)
I noticed also that the portion of the trigger that I added from the vendor (The update SHIP_DAT stuff) isn't written very 'clean' either; shouldn't

/*When a row is inserted in table PS_TKT_LIN
set the ITEM_SHIP_DAT to PS_TKT_HDR.SHIP_DAT provided that:
a) no value was explicitly provided for ITEM_SHIP_DAT
b) there is a non-null SHIP_DAT value in PS_TKT_HDR
*/

update tl
set tl.TKT_SHIP_DAT = th.SHIP_DAT

from  PS_TKT_LIN tl
 join PS_TKT_HDR th
on tl.STR_ID = th.STR_ID
and tl.STA_ID = th.STA_ID
and tl.TKT_NO = th.TKT_NO



update tl
set tl.PROMPT_ALPHA_1 = th.PROF_ALPHA_1
from inserted i
inner join PS_TKT_LIN tl
on i.STR_ID = tl.STR_ID
and i.STA_ID = tl.STA_ID
and i.TKT_NO = tl.TKT_NO
and i.SEQ_NO = tl.SEQ_NO
inner join IM_ITEM th
on tl.ITEM_NO = th.ITEM_NO

update tl
set tl.PROMPT_ALPHA_2 = th.PROF_ALPHA_2
from inserted i
inner join PS_TKT_LIN tl
on i.STR_ID = tl.STR_ID
and i.STA_ID = tl.STA_ID
and i.TKT_NO = tl.TKT_NO
and i.SEQ_NO = tl.SEQ_NO
inner join IM_ITEM th
on tl.ITEM_NO = th.ITEM_NO

be more like

/*When a row is inserted in table PS_TKT_LIN
set the ITEM_SHIP_DAT to PS_TKT_HDR.SHIP_DAT provided that:
a) no value was explicitly provided for ITEM_SHIP_DAT
b) there is a non-null SHIP_DAT value in PS_TKT_HDR
*/
     update tl
        set tl.TKT_SHIP_DAT = th.SHIP_DAT,
            tl.PROMPT_ALPHA_1 = th.PROF_ALPHA_1,
            tl.PROMPT_ALPHA_2 = th.PROF_ALPHA_2
               from inserted i
                    inner join PS_TKT_LIN tl
                        on i.STR_ID = tl.STR_ID
                       and i.STA_ID = tl.STA_ID
                       and i.TKT_NO = tl.TKT_NO
                       and i.SEQ_NO = tl.SEQ_NO
                    inner join IM_ITEM th
                       on tl.ITEM_NO = th.ITEM_NO
                    inner join PS_TKT_HDR th
                        on tl.STR_ID = th.STR_ID
                       and tl.STA_ID = th.STA_ID
                       and tl.TKT_NO = th.TKT_NO

(LOOK!!! Formatting!!

{I feel all warm and fuzzy inside!)

There's never enough time to type code right,
but always enough time for a hotfix...

Next Page

Subscribe to SQLTeam.com

SQLTeam.com Articles via RSS

SQLTeam.com Weblog via RSS

- Advertisement -

Resources