"Josh White" <whitegoose_at_inorbit.com> wrote in message
news:aafea0a8.0504012011.44fb4143_at_posting.google.com...
> ken_at_kendenny.com wrote in message
news:<1112378024.501494.259330_at_o13g2000cwo.googlegroups.com>...
> > Josh White wrote:
> > >
> > > OK thanks for the feedback. I am new to PL/SQL (haven't even come in
> > > to contact with Oracle since university - and that was to learn basic
> > > SQL) as I have been working with SQL Server for the last year, and
> > DB2
> > > before that. To make matters worse, like I said, at the moment I do
> > > not even have access to an Oracle database so I am writing this code
> > > blind.
> > >
> > > Anyway, after your feedback and discovering 'mutating table' errors,
> > I
> > > have fixed the code somewhat:
> > >
> > > --------------------------------------------------
> > > CREATE OR REPLACE TRIGGER hia_iu_Comp
> > > BEFORE INSERT OR UPDATE
> > > ON COMP
> > > REFERENCING NEW AS newRow OLD AS oldRow
> > > FOR EACH ROW
> > >
> > > DECLARE
> > > Err_Num NUMBER;
> > > Err_Desc VARCHAR2(500);
> > > v_compType VARCHAR2(4);
> > > v_sql VARCHAR2(500);
> > > v_bgtNo VARCHAR2(24);
> > >
> > > BEGIN
> > > -- Get the type of asset
> > > SELECT CT.COMPCODE INTO v_compType
> > > FROM COMPTYPE CT
> > > WHERE CT.COMPTYPE = :newRow.COMPTYPE;
> > >
> > > -- Building Assets must have a budget number
> > > IF v_compType = 'BLDG' THEN
> > > -- Build SQL to get budget number
> > > v_sql := 'SELECT C.BGTNO FROM COMP' || v_compType || ' C WHERE
> > > C.COMPKEY = :newRowCompKey';
> > >
> > > EXECUTE IMMEDIATE v_sql INTO v_bgtNo USING :newRow.COMPKEY;
> > >
> > > IF v_bgtNo IS NULL THEN
> > > RAISE_APPLICATION_ERROR(-20100, '##Budget Number field must
> > > be populated##');
> > > END IF;
> > > END IF;
> > >
> > > EXCEPTION
> > > WHEN OTHERS THEN
> > > Err_Num := SQLCODE;
> > > Err_Desc := '##Error (' || Err_Num || ' - ' ||
> > > SUBSTR(SQLERRM,1,99) || ') Occured in Trigger hia_iu_Comp. Please
> > > contact support.##';
> > > RAISE_APPLICATION_ERROR (-20100, Err_Desc);
> > >
> > > END;
> > > --------------------------------------------------
> > >
> > > Is this looking a bit more competant?
> > >
> > > Thanks again,
> > > Josh.
> >
> > What Daniel said. The purpose of a trigger is to do an insert, update,
> > or delete from some other table or to change the value of some :NEW
> > column. If nothing in the database changes as a result of the trigger,
> > then why bother having a trigger.
> >
> > And FWIW here's something else in your trigger that seems unnecessary.
> > The dynamic SQL. I can see you using dynamic SQL because you
> > constructing the table name as COMP' || v_compType but this only
> > happens inside the "IF v_CompType = 'BLDG'" so the table name can't be
> > anything other than COMPBLDG, so why use dynamic SQL, just select from
> > COMPBLDG.
> >
> > Maybe if you told us what the requirements were for this trigger, we
> > could help you out more.
> >
> > Ken Denny
>
> OK yes maybe my requirements are a little bit odd. Firstly this
> trigger is not supposed to update, insert or delete from another
> table. I am configuring an out-of-the-box product. The product's code
> cannot be altered so the standard procedure is to use database
> triggers to enforce specific business rules. In this case the trigger
> will raise an application error warning the user that a budget number
> must be entered if COMPxxxx.BGTNO is null. This may not seem like a
> very good idea, but it is what I have to do.
>
> You (and DA Morgan) are right - there is real no need in this case for
> the execute immediate. But I have written it this way becuae I am sure
> that in the next week there will be new requirements to have this
> trigger work for many tables other than COMPBLDG - there are
> approximately 30 COMPxxx tables in the database. Writing it this way I
> am not hard-coding table names or IF statements.
>
> Does my code make more sense now - and more importantly for me - does
> it look syntactically correct?
>
> Thanks for your help,
> Josh.
Don't write "generic" code it is going to bite you in the butt. (not to
mention possibly be very inefficient) Instead call a procedure and handle
the thing in the procedure. And raise an error if you need to.
Jim
Received on Fri Apr 01 2005 - 22:23:40 CST
