Re: Help with simple PL/SQL

From: William Robertson <william.robertson_at_bigfoot.com>
Date: 13 Oct 2005 11:19:54 -0700
Message-ID: <1129227594.598397.309180@g43g2000cwa.googlegroups.com>

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. Received on Thu Oct 13 2005 - 13:19:54 CDT