William Robertson wrote:
> DFS wrote:
>
>>Rene Nyffenegger wrote:
>>
>>>On 2005-10-13, DFS <nospam_at_dfs_.com> wrote:
>>>
>>>>All I'm doing is taking a list and renumbering a column to make sure
>>>>the values are sequential from 1 to N.
>>>>
>>>>It compiles fine, but when I run it I get:
>>>>
>>>>ORA-00936: missing expression
>>>>ORA-06512: at "DFS.SP_SETLISTORDER", line 14
>>>>
>>>>1. CREATE OR REPLACE PROCEDURE sp_SetListOrder (ProjectID Integer) IS
>>>>2. TYPE projCursor IS REF CURSOR;
>>>>3 pCur projCursor;
>>>>4 i number := 1;
>>>>5 ProjCPID number;
>>>>6 ProjItemOrder number;
>>>>7 cSQL VARCHAR2(1000);
>>>>8 BEGIN
>>>>9 CSQL := 'SELECT ProjCPID, ItemOrder FROM Table_Project WHERE
>>>>ProjID =
>>>>
>>>>>ProjectID ORDER BY ItemOrder';
>>>>
>>>>10 OPEN pCur FOR cSQL USING ProjectID;
>>>>11 LOOP
>>>>12 FETCH pCur INTO ProjCPID, ProjItemOrder;
>>>>13 EXIT WHEN pCur%NOTFOUND;
>>>>14 EXECUTE IMMEDIATE 'UPDATE Table_Project SET ItemOrder =
>>>>||i|| WHERE ProjCPID = ||ProjCPID ||';
>>>>15 i := i + 1;
>>>>16 END LOOP;
>>>>17 CLOSE pCur;
>>>>18 END;
>>>
>>>
>>>It seems as though you solved the problem with the wrong quoting in
>>>the execute immediate statement. But it remains unclear to me why
>>>you use execute immediate at all. The statement is perfectly fit for
>>>an ordinary update. Likewise, there is no reason why you quote the
>>>select statement.
>>
>>Thanks for the tips. I'm new to PL/SQL.
>>
>>You mean there's no reason to use or assign the select to variable cSQL?
>>And there's no need for EXECUTE IMMEDIATE?
>>
>>So, rewritten (remove the cSQL and EXECUTE IMMEDIATE)
>>
>>
>>>>1. CREATE OR REPLACE PROCEDURE sp_SetListOrder (ProjectID Integer) IS
>>>>2. TYPE projCursor IS REF CURSOR;
>>>>3 pCur projCursor;
>>>>4 i number := 1;
>>>>5 ProjCPID number;
>>>>6 ProjItemOrder number;
>>>>7 BEGIN
>>>>8 OPEN pCur FOR
>>
>>'SELECT ProjCPID, ItemOrder
>>FROM Table_Project
>>WHERE ProjID = ProjectID
>>ORDER BY ItemOrder'
>>USING ProjectID;
>>
>>>>9 LOOP
>>>>10 FETCH pCur INTO ProjCPID, ProjItemOrder;
>>>>11 EXIT WHEN pCur%NOTFOUND;
>>>>12 UPDATE Table_Project SET ItemOrder = :i WHERE ProjCPID =
>>
>>:ProjCPID;
>>
>>>>13 i := i + 1;
>>>>14 END LOOP;
>>>>15 CLOSE pCur;
>>>>16 END;
>>
>>That shaved off two lines, which is cool, but now creates with compilation
>>errors.
>>
>>But if I put back in the EXECUTE IMMEDIATE (and the USING clause at the
>>end), it compiles and runs fine.
>>
>>Thanks for your help, Rene
>
>
> You can simplify it even firther with something like (untested):
>
> CREATE OR REPLACE PROCEDURE setlistorder
> ( p_projectid INTEGER )
> IS
> i INTEGER := 1;
> BEGIN
> FOR r IN
> (
> SELECT projcpid, itemorder
> FROM table_project
> WHERE projid = projectid
> ORDER BY itemorder
> )
> LOOP
> UPDATE table_project
> SET itemorder = i
> WHERE projcpid = r.projcpid;
>
> i := i + 1;
> END LOOP;
> END;
>
> However I might have missed what all the variables were for. It also
> seems like something you should be able to do with a straight UPDATE
> statement.
Why would you use a cursor loop to do what could be done 100X faster
with array processing?
--
Daniel A. Morgan
http://www.psoug.org
damorgan_at_x.washington.edu
(replace x with u to respond)
Received on Thu Oct 13 2005 - 17:13:31 CDT
