Re: Please check this syntax

From: DA Morgan <damorgan_at_x.washington.edu>
Date: Thu, 31 Mar 2005 17:52:59 -0800
Message-ID: <1112320165.522237@yasure>

Josh White wrote:

> whitegoose_at_inorbit.com (Josh White) wrote in message news:<aafea0a8.0503301942.5ac37083_at_posting.google.com>...
>

>>Hi,
>>
>>I'm new to PL/SQL and am currently writing some triggers for work -
>>even though I won't actually have anything on my machine that can
>>check the syntax until late next week - it's a long story.
>>
>>So someone please check the syntax of this simple trigger for me?
>>
>>-----------------------------------------------
>>CREATE OR REPLACE TRIGGER hia_iu_Comp
>>AFTER INSERT OR UPDATE
>>ON COMP
>>REFERENCING NEW AS new_Row OLD AS oldRow
>>FOR EACH ROW
>>
>>DECLARE
>>v_compType VARCHAR2(4);
>>v_sql VARCHAR2(500);
>>v_bgtNo VARCHAR2(24);
>>
>>BEGIN
>>	-- Get the type of asset
>>	SELECT v_compType = CT.COMPCODE
>>	FROM COMP C, COMPTYPE CT
>>	WHERE C.COMPTYPE = CT.COMPTYPE
>>	AND C.COMPKEY = :newRow.COMPKEY
>>	
>>	-- 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;
>>END
>>-----------------------------------------------
>>
>>Thanks,
>>Josh.

>
>
>
> 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.

I don't see how you've fixed the code. It still has EXECUTE IMMEDIATE and still does nothing. I'm serious. It does not one bit of real work. What is the business case this is supposed to address?

-- 
Daniel A. Morgan
University of Washington
damorgan_at_x.washington.edu
(replace 'x' with 'u' to respond)

Received on Thu Mar 31 2005 - 19:52:59 CST