Re: Please check this syntax

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 Received on Fri Apr 01 2005 - 11:53:44 CST