Using Cursor to update table - error

[AlStl]

Guys: follwing is my curros and I am getting this error. This select statment runs just fine if i run it seprately.

declare
commit_interval number := 1;
count number := 0;

cursor c1 is

select Y.LPT_TRKS "VLPT_TRKS",Y.TGSN "VTGSN"
FROM LTFS_FCST_YR_TMP X, LTFS_FCST_YR_TMP Y
WHERE X.TGSN=Y.TGSN AND X.FCST_YR=Y.FCST_YR
AND X.INVERT=Y.INVERT
AND X.LPT_TRKS <> Y.LPT_TRKS
AND Y.COMP_KEY='P'
AND X.TGSN='AM008985'
AND X.COMP_KEY='D'
FOR UPDATE OF X.VLPT_TRKS;

begin

For row in c1
Loop
UPDATE LTFS_fcst_yr_tmp SET LPT_TRKS=(rec_c1.VLPT_TRKS WHERE TGSN=rec_c1.VTGSN);
count := count + 1;
IF count >= commit_interval THEN
COMMIT;
count := 0;
END IF;
END LOOP
COMMIT;

Error Message:

FROM LTFS_FCST_YR_TMP X, LTFS_FCST_YR_TMP Y
*
ERROR at line 7:
ORA-06550: line 20, column 62:
PL/SQL: ORA-00907: missing right parenthesis
ORA-06550: line 20, column 7:
PL/SQL: SQL Statement ignored
ORA-06550: line 27, column 8:
PLS-00103: Encountered the symbol "end-of-file" when expecting one of the following:
begin case declare end exception exit for goto if loop mod
null pragma raise return select update while with
<an identifier> <a double-quoted d


Al

 

[carp]

A couple of things:
Your syntax is off. rec_c1 is undefined, so you can't use it. "count" is a reserved word, so it is a bad choice for a variable name. Likewise, "row" is not a good choice either.
And is there any reason you are using the FOR UPDATE mechanism? Your statement will fail if it can't obtain all of its locks right away, and your commit within the loop releases all of the locks and leaves you vulnerable to a "fetch out of sequence" error.

Try the following:

declare
   commit_interval number := 1;
   l_count number := 0;    
   cursor c1 is 
      select Y.LPT_TRKS "VLPT_TRKS",Y.TGSN "VTGSN"
        FROM LTFS_FCST_YR_TMP X, LTFS_FCST_YR_TMP Y
       WHERE X.TGSN=Y.TGSN AND X.FCST_YR=Y.FCST_YR 
         AND X.INVERT=Y.INVERT
         AND X.LPT_TRKS <> Y.LPT_TRKS 
         AND Y.COMP_KEY='P'
         AND X.TGSN='AM008985' 
         AND X.COMP_KEY='D';
      
begin
    For r in c1 Loop
       UPDATE LTFS_fcst_yr_tmp 
          SET LPT_TRKS= r.VLPT_TRKS 
        WHERE TGSN = r.VTGSN;
      l_count := l_count + 1;
      IF (l_count >= commit_interval) THEN
        COMMIT;
        l_count := 0;
      END IF; 
   END LOOP
 COMMIT;
END;

 

[LKBrwnDBA]

From the SQL Reference manual:

Restrictions on the FOR UPDATE Clause
* You cannot specify this clause with the following other constructs: the DISTINCT operator, [red]CURSOR[/red] expression, set operators, group_by_clause, or aggregate functions.
* The tables locked by this clause must all be located on the same database, and on the same database as any LONG columns and sequences referenced in the same statement.

----------------------------------------------------------------------------
The person who says it can't be done should not interrupt the person doing it. -- Chinese proverb

 

[carp]

Yet it DOES work within a cursor; in fact, that is where it is most commonly found:

12:59:39 SQL> DECLARE
13:01:17   2     CURSOR my_cursor IS SELECT * FROM xx_taskings
13:01:17   3                         FOR UPDATE;
13:01:17   4  BEGIN
13:01:17   5     FOR t IN my_cursor LOOP
13:01:17   6        UPDATE xx_taskings
13:01:17   7           SET suspense = suspense
13:01:17   8         WHERE CURRENT OF my_cursor;
13:01:17   9     END LOOP;
13:01:17  10     COMMIT;
13:01:17  11  END;
13:01:17  12  /

PL/SQL procedure successfully completed.

What the SQL reference manual is referring to is the CURSOR operator - which is a function that returns a nested cursor (similar to a REF CURSOR in PL/SQL) <see Chapter 4 of the manual - Page 4-7 in my copy>. This is a PL/SQL cursor declaration, so the FOR UPDATE clause is quite usable here.

 

[LKBrwnDBA]

Ooops, thanks for the clarification.



----------------------------------------------------------------------------
The person who says it can't be done should not interrupt the person doing it. -- Chinese proverb

 

[Beilstwh]

You should never commit inside a cursor loop. It will sometimes cause the dreded
"Snapshot too old" error. That being said, Why don't you write it as a simple update
and avoid the entire loop. A loop is the slowest pobbible way to do a mass update.

UPDATE LTFS_fcst_yr_tmp a
SET a.LPT_TRKS=  (select Y.LPT_TRKS
                  FROM LTFS_FCST_YR_TMP X, LTFS_FCST_YR_TMP Y
                  WHERE X.TGSN=Y.TGSN AND X.FCST_YR=Y.FCST_YR
                  AND X.INVERT=Y.INVERT
                  AND X.LPT_TRKS <> Y.LPT_TRKS
                  AND Y.COMP_KEY='P'
                  AND X.TGSN='AM008985'
                  AND X.COMP_KEY='D'
                  and a.tgsn = y.tgsn)
where exists
 (select null
  FROM LTFS_FCST_YR_TMP X, LTFS_FCST_YR_TMP Y
  WHERE X.TGSN=Y.TGSN AND X.FCST_YR=Y.FCST_YR
  AND X.INVERT=Y.INVERT
  AND X.LPT_TRKS <> Y.LPT_TRKS
  AND Y.COMP_KEY='P'
  AND X.TGSN='AM008985'
  AND X.COMP_KEY='D'
  and a.tgsn = y.tgsn);

Bill
Oracle DBA/Developer
New York State, USA