Hi all,
I brought this up some time back, but I have a trigger that I was tasked,
begrudgingly, with converting from Oracle to SQLserver. After the comments
I received, which were great, it has me concerned that I'm trying to convert
line-for-line a trigger that might very well be poorly written in the first
place. Would any of you mind taking a look and giving me some feedback?
Unfortunately, I have no data tables to refer to myself, so can't provide
them to you.
I am going to attack the conversion in parts, and since I've already run
into some unfamiliar issues to deal with, I thought I'd ask before I go too
far down the road and find that the procedure has too many problems itself.
Trust me, I'd rather stick to my user interface code than have to deal with
converting a trigger. I'm only doing this to help the customer get through
where the vendor has given up.
CREATE OR REPLACE TRIGGER PartCost_WO AFTER INSERT ON PartTrans FOR EACH ROW
DECLARE
aNextVal NUMBER(9);
PricePer FLOAT;
BEGIN
IF (:new.HISTKEY) > 1 THEN
SELECT Z.NEXTVAL INTO aNextVal
FROM ZEQ_120 Z;
aNextVal := aNextVal + 1;
PricePer := :new.TRNCST/:new.PRTSISS;
INSERT INTO COSTPART
(USAGE, PRTKEY, STKKEY, TOTCOST, ADDDTTM, ADDBY, COSTKEY,
HISTKEY, CHRGDTTM, RATE)
VALUES
(:new.PRTSISS, :new.PRTKEY, :new.STKKEY, :new.TRNCST,
:new.ADDDTTM, 'PartCost_WO TRIGGER', aNextVal, :new.HISTKEY,
:new.ADDDTTM, PricePer);
UPDATE ZEQ_120 Z SET Z.NEXTVAL = Z.NEXTVAL + 1;
END IF;
END;
/
show errors trigger PartCost_WO;Mike
Have you looked at CREATE TRIGGER topic in the BOL?
CREATE TRIGGER PartCost_WO ON PartTrans FOR INSERT
DECLARE @.aNextVal DECIMAL or INT...
DECLARE @.PricePer FLOAT;
BEGIN
--Did not undesrtand what does it mean?
IF (:new.HISTKEY) > 1 THEN
SELECT Z.NEXTVAL INTO aNextVal
FROM ZEQ_120 Z;
SET @.aNextVal = @.aNextVal + 1;
SET @.PricePer := :new.TRNCST/:new.PRTSISS; --Where do these
columns/variables come from?
INSERT INTO COSTPART
(USAGE, PRTKEY, STKKEY, TOTCOST, ADDDTTM, ADDBY, COSTKEY,
HISTKEY, CHRGDTTM, RATE)
SELECT
new.PRTSISS, :new.PRTKEY, :new.STKKEY, :new.TRNCST,
:new.ADDDTTM, 'PartCost_WO TRIGGER', aNextVal, :new.HISTKEY,
:new.ADDDTTM, PricePer FROM SomeTable
UPDATE ZEQ_120 Z SET Z.NEXTVAL = Z.NEXTVAL + 1;--Don't you have a WHERE
condition here?
END;
"mikeb" <mike@.nohostanywhere.com> wrote in message
news:%23U5cDxwlFHA.3780@.tk2msftngp13.phx.gbl...
> Hi all,
> I brought this up some time back, but I have a trigger that I was tasked,
> begrudgingly, with converting from Oracle to SQLserver. After the
> comments I received, which were great, it has me concerned that I'm trying
> to convert line-for-line a trigger that might very well be poorly written
> in the first place. Would any of you mind taking a look and giving me
> some feedback? Unfortunately, I have no data tables to refer to myself, so
> can't provide them to you.
> I am going to attack the conversion in parts, and since I've already run
> into some unfamiliar issues to deal with, I thought I'd ask before I go
> too far down the road and find that the procedure has too many problems
> itself. Trust me, I'd rather stick to my user interface code than have to
> deal with converting a trigger. I'm only doing this to help the customer
> get through where the vendor has given up.
> CREATE OR REPLACE TRIGGER PartCost_WO AFTER INSERT ON PartTrans FOR EACH
> ROW
> DECLARE
> aNextVal NUMBER(9);
> PricePer FLOAT;
> BEGIN
> IF (:new.HISTKEY) > 1 THEN
> SELECT Z.NEXTVAL INTO aNextVal
> FROM ZEQ_120 Z;
> aNextVal := aNextVal + 1;
> PricePer := :new.TRNCST/:new.PRTSISS;
> INSERT INTO COSTPART
> (USAGE, PRTKEY, STKKEY, TOTCOST, ADDDTTM, ADDBY, COSTKEY,
> HISTKEY, CHRGDTTM, RATE)
> VALUES
> (:new.PRTSISS, :new.PRTKEY, :new.STKKEY, :new.TRNCST,
> :new.ADDDTTM, 'PartCost_WO TRIGGER', aNextVal, :new.HISTKEY,
> :new.ADDDTTM, PricePer);
> UPDATE ZEQ_120 Z SET Z.NEXTVAL = Z.NEXTVAL + 1;
> END IF;
> END;
> /
> show errors trigger PartCost_WO;
>
>
>|||Uri,
Here are some possible answers to your questions. Maybe you
can give it another shot - I'm reticent, because SQL Server doesn't
have FOR EACH ROW triggers, and it looks like this is trying to
emulate a home-grown identity-like column, which is tricky to do
all at once.
A FOR EACH ROW trigger will be executed once for each row
inserted, so what's needed here is a set-based query, a loop, or
a cursor.
The expression :new.<column> is probably the Oracle equivalent
of inserted.<column> for a row, so it can't be used in INSERT .. VALUES,
but you can do INSERT INTO .. SELECT .. FROM inserted.
aNextVal is a variable, so SELECT Z.NEXTVAL INTO aNextVal
is probably SET @.aNextVal = (SELECT Z.NEXTVAL FROM ZEQ_120 Z)
It looks very much like ZEQ_120 contains only one row, and that row
is the last home-grown identity value used. This would explain SELECT INTO
a variable, as well as the UPDATE of ZEQ_120 with no WHERE clause.
The hardest part of this is to get the incrementing column values that
go into
COSTPART correct. The key column of PartTrans would be helpful
to know here.
If this trigger will only be called for single-row inserts, then it's
easier, and
IF @.@.rowcount > 1 BEGIN <report error> <rollback transaction> END
can help out.
Steve Kass
Drew University
Uri Dimant wrote:
>Mike
>Have you looked at CREATE TRIGGER topic in the BOL?
>CREATE TRIGGER PartCost_WO ON PartTrans FOR INSERT
>DECLARE @.aNextVal DECIMAL or INT...
>DECLARE @.PricePer FLOAT;
>BEGIN
>--Did not undesrtand what does it mean?
> IF (:new.HISTKEY) > 1 THEN
> SELECT Z.NEXTVAL INTO aNextVal
> FROM ZEQ_120 Z;
>
> SET @.aNextVal = @.aNextVal + 1;
> SET @.PricePer := :new.TRNCST/:new.PRTSISS; --Where do these
>columns/variables come from?
> INSERT INTO COSTPART
> (USAGE, PRTKEY, STKKEY, TOTCOST, ADDDTTM, ADDBY, COSTKEY,
> HISTKEY, CHRGDTTM, RATE)
>SELECT
> new.PRTSISS, :new.PRTKEY, :new.STKKEY, :new.TRNCST,
>:new.ADDDTTM, 'PartCost_WO TRIGGER', aNextVal, :new.HISTKEY,
>:new.ADDDTTM, PricePer FROM SomeTable
>
>UPDATE ZEQ_120 Z SET Z.NEXTVAL = Z.NEXTVAL + 1;--Don't you have a WHERE
>condition here?
>
>END;
>"mikeb" <mike@.nohostanywhere.com> wrote in message
>news:%23U5cDxwlFHA.3780@.tk2msftngp13.phx.gbl...
>
>
>|||On Mon, 1 Aug 2005 19:45:23 -0700, mikeb wrote:
(snip)
>Trust me, I'd rather stick to my user interface code than have to deal with
>converting a trigger. I'm only doing this to help the customer get through
>where the vendor has given up.
Hi mikeb,
Okay. With the help of Uri and Steve, I think I can help you help the
customer get through <g>.
I agree with Steve: the ZEQ_120 table must contain just one row that
holds a "last used" value. This code really wouldn't make sense if it
were otherwise.
The easiest way to port this to SQL Server is to change the COSTKEY
column in the COSTPART table to an IDENTITY column. That way, SQL Server
will dish out a new, continuously increasing number. Without the need
for a seperate one-row "last number used" table, and without the locking
issues, deadlock opportunities and scalability restrictions that such a
table incurs.
Note however that the IDENTITY column does not guarantee that there are
no gaps. If a transaction aborts and is rolled back, the IDENTITY values
used in that transaction won't be reused. The Oracle code you posted
will reuse those values, since the change to the ZEQ_120 table will also
be rolled back.
If you can change COSTPART.COSTKEY to an identity column, the equivalent
trigger in SQL server becomes very easy:
CREATE TRIGGER PartCost_WO
ON PartTrans
AFTER INSERT
AS
INSERT INTO COSTPART
(USAGE, PRTKEY, STKKEY, TOTCOST, ADDDTTM, ADDBY,
HISTKEY, CHRGDTTM, RATE)
SELECT PRTSISS, PRTKEY, STKKEY, TRNCST, ADDTTM, 'PartCost_WO TRIGGER',
HISTKEY, ADDDTTM, TRNCST/PRTSISS
FROM inserted
go
Note that this changes the logic from a row-based trigger to a set-based
trigger (the only trigger type supported in SQL Server). Probably a lot
faster too.
Also, it might make sense to redefine COSTPART.RATE as a computed
column. Storing a value that can be computed from two other values in
the same row only makes sense if there's the possiblity that one of the
values might change at a later time without the others changing along.
But if RATE always has to be equal to TPRTKEY/USAGE, then it's better to
use a computed column.
Final advice: don't use all caps. Your code will be much eaasier to read
and understand if you use ALL CAPS for keywords, and PascalCase for
table and column names.
CREATE TABLE CostPart
(CostKey int NOT NULL IDENTITY,
-- other columns,
Rate AS PrtKey / Usage,
PRIMARY KEY (...),
-- other constraints,
)
Best, Hugo
--
(Remove _NO_ and _SPAM_ to get my e-mail address)|||Thanks Hugo.
I couldn't agree with you more on all points - even down to the all caps and
confusing column names (oh wait, maybe that was my own peve).
Unfortunately, I can't change anything about the tables other than adding in
a trigger. This database was written for another vendors software, I'm only
augmenting it to make it work the way it should.
And you guys are right, the zeq_# table is used to gain a unique value for a
specific inserted column of data in a row. Again, I'd do this differently,
probably even without use of an identity column.
That aside, I appreciate all the comments and continued input! I'm just
very unfamiliar with oracle sql and only at the tip of the 'berg of t-sql
(as I continuously humble myself when reading through this NG).
Thanks again!
btw, here's how I converted the trigger so far -- no database yet in my
hands to be able to test it - but can I get some input? Would be very nice
to be able to insert it into a test database and have it work right from the
start!! But I don't know it enough to really test the logic against the
oracle code.
Create Trigger WO_PrtCost ON PRTTRNI After Insert
As
Declare @.aNextVal int
Declare @.PricePer float
Begin
If (Inserted.HISTKEY > 1)
Begin
SELECT @.aNextVal = Z.NEXTVAL FROM ZEQ_120 Z
Select @.aNextVal = @.aNextVal + 1
Select @.PricePer = Inserted.TRNCST/Inserted.PRTSISS
Insert COSTPART
(
USAGE,
PRTKEY,
STKKEY,
TOTCOST,
ADDDTTM,
ADDBY,
COSTKEY,
HISTKEY,
CHRGDTTM,
RATE
)
Select
PRTSISS,
PRTKEY,
STKKEY,
TRNCST,
ADDDTTM,
'WO_PRTCOST TRIGGER',
@.aNextVal,
HISTKEY,
ADDDTTM,
PricePer
From
Inserted
Update ZEQ_120
Set NEXTVAL = NEXTVAL + 1
End
End
--RaiseError('trigger WO_PRTCOST', 1, 1)
"Hugo Kornelis" <hugo@.pe_NO_rFact.in_SPAM_fo> wrote in message
news:fgv4f1tf6q28otl42arm8q488m24ht2so2@.
4ax.com...
> On Mon, 1 Aug 2005 19:45:23 -0700, mikeb wrote:
> (snip)
> Hi mikeb,
> Okay. With the help of Uri and Steve, I think I can help you help the
> customer get through <g>.
> I agree with Steve: the ZEQ_120 table must contain just one row that
> holds a "last used" value. This code really wouldn't make sense if it
> were otherwise.
> The easiest way to port this to SQL Server is to change the COSTKEY
> column in the COSTPART table to an IDENTITY column. That way, SQL Server
> will dish out a new, continuously increasing number. Without the need
> for a seperate one-row "last number used" table, and without the locking
> issues, deadlock opportunities and scalability restrictions that such a
> table incurs.
> Note however that the IDENTITY column does not guarantee that there are
> no gaps. If a transaction aborts and is rolled back, the IDENTITY values
> used in that transaction won't be reused. The Oracle code you posted
> will reuse those values, since the change to the ZEQ_120 table will also
> be rolled back.
> If you can change COSTPART.COSTKEY to an identity column, the equivalent
> trigger in SQL server becomes very easy:
> CREATE TRIGGER PartCost_WO
> ON PartTrans
> AFTER INSERT
> AS
> INSERT INTO COSTPART
> (USAGE, PRTKEY, STKKEY, TOTCOST, ADDDTTM, ADDBY,
> HISTKEY, CHRGDTTM, RATE)
> SELECT PRTSISS, PRTKEY, STKKEY, TRNCST, ADDTTM, 'PartCost_WO TRIGGER',
> HISTKEY, ADDDTTM, TRNCST/PRTSISS
> FROM inserted
> go
> Note that this changes the logic from a row-based trigger to a set-based
> trigger (the only trigger type supported in SQL Server). Probably a lot
> faster too.
> Also, it might make sense to redefine COSTPART.RATE as a computed
> column. Storing a value that can be computed from two other values in
> the same row only makes sense if there's the possiblity that one of the
> values might change at a later time without the others changing along.
> But if RATE always has to be equal to TPRTKEY/USAGE, then it's better to
> use a computed column.
> Final advice: don't use all caps. Your code will be much eaasier to read
> and understand if you use ALL CAPS for keywords, and PascalCase for
> table and column names.
> CREATE TABLE CostPart
> (CostKey int NOT NULL IDENTITY,
> -- other columns,
> Rate AS PrtKey / Usage,
> PRIMARY KEY (...),
> -- other constraints,
> )
> Best, Hugo
> --
> (Remove _NO_ and _SPAM_ to get my e-mail address)|||No need for further review - thanks for the help all!
"mikeb" <mike@.nohostanywhere.com> wrote in message
news:u$%23%23s$UmFHA.1412@.TK2MSFTNGP09.phx.gbl...
> Thanks Hugo.
> I couldn't agree with you more on all points - even down to the all caps
> and confusing column names (oh wait, maybe that was my own peve).
> Unfortunately, I can't change anything about the tables other than adding
> in a trigger. This database was written for another vendors software, I'm
> only augmenting it to make it work the way it should.
> And you guys are right, the zeq_# table is used to gain a unique value for
> a specific inserted column of data in a row. Again, I'd do this
> differently, probably even without use of an identity column.
> That aside, I appreciate all the comments and continued input! I'm just
> very unfamiliar with oracle sql and only at the tip of the 'berg of t-sql
> (as I continuously humble myself when reading through this NG).
> Thanks again!
> btw, here's how I converted the trigger so far -- no database yet in my
> hands to be able to test it - but can I get some input? Would be very
> nice to be able to insert it into a test database and have it work right
> from the start!! But I don't know it enough to really test the logic
> against the oracle code.
> Create Trigger WO_PrtCost ON PRTTRNI After Insert
> As
> Declare @.aNextVal int
> Declare @.PricePer float
> Begin
> If (Inserted.HISTKEY > 1)
> Begin
> SELECT @.aNextVal = Z.NEXTVAL FROM ZEQ_120 Z
> Select @.aNextVal = @.aNextVal + 1
> Select @.PricePer = Inserted.TRNCST/Inserted.PRTSISS
> Insert COSTPART
> (
> USAGE,
> PRTKEY,
> STKKEY,
> TOTCOST,
> ADDDTTM,
> ADDBY,
> COSTKEY,
> HISTKEY,
> CHRGDTTM,
> RATE
> )
> Select
> PRTSISS,
> PRTKEY,
> STKKEY,
> TRNCST,
> ADDDTTM,
> 'WO_PRTCOST TRIGGER',
> @.aNextVal,
> HISTKEY,
> ADDDTTM,
> PricePer
> From
> Inserted
> Update ZEQ_120
> Set NEXTVAL = NEXTVAL + 1
> End
> End
> --RaiseError('trigger WO_PRTCOST', 1, 1)
>
> "Hugo Kornelis" <hugo@.pe_NO_rFact.in_SPAM_fo> wrote in message
> news:fgv4f1tf6q28otl42arm8q488m24ht2so2@.
4ax.com...
>
No comments:
Post a Comment