-1

I have a cursor to return record to be used in EXECUTE IMMEDIATE

 CURSOR c1
 IS
 SELECT crs_cust.CUSTOMER_ID AS CUSTOMER_ID, subset.NEW_CUSTOMER_REFERENCE_ID AS 
 CUSTOMER_REF_ID FROM CRS_CUSTOMERS crs_cust INNER JOIN 
 DAY0_SUBSET subset ON 
 crs_cust.CUSTOMER_ID=subset.CURRENT_CUSTOMER_ID;

The EXECUTE IMMEDIATE queries in below block are not executing.

OPEN c1;
 LOOP
  EXIT WHEN c1%NOTFOUND;
  EXIT WHEN (c1%ROWCOUNT <> p_SCBCount);
   FOR i in c1 LOOP
     EXECUTE IMMEDIATE 'UPDATE CRS_CUSTOMERS SET REF_ID = ' || i.CUSTOMER_REF_ID ||'WHERE CUSTOMER_ID = ' || i.CUSTOMER_ID; 
     p_TotalUpdatedCRS := p_TotalUpdatedCRS + 1;

     EXECUTE IMMEDIATE 'UPDATE CRS_REVIEWS SET 
     REF_ID = ' || i.CUSTOMER_REF_ID || 'WHERE CUSTOMER_ID = ' || i.CUSTOMER_ID; 
     EXECUTE IMMEDIATE 'UPDATE CRS_EVENT SET REF_ID = ' || i.CUSTOMER_REF_ID || 'WHERE UNIQUE_ID = ' || i.CUSTOMER_ID;
     EXECUTE IMMEDIATE 'UPDATE ALERT_HEADER SET CUSTOMER_SOURCE_REF_ID = ' || i.CUSTOMER_REF_ID || 'WHERE CUSTOMER_ID = ' || i.CUSTOMER_ID; 
 END LOOP;
    DBMS_OUTPUT.PUT_LINE ('The total updates to CRS table = ' || p_TotalUpdatedCRS); 
END LOOP;      
 CLOSE c1; 

The DBMS output is also not printed out when I execute procedure using SQL developer.

user2102665
  • 429
  • 2
  • 11
  • 26

3 Answers3

4

The reason why your code does nothing is this:

OPEN c1;
 LOOP
  EXIT WHEN c1%NOTFOUND;   
  EXIT WHEN (c1%ROWCOUNT <> p_SCBCount);

You are testing for c1%ROWCOUNT before you have executed a fetch. So its value is 0; I'm guessing p_SCBCount is not zero at that point (because you initialised it to some value in the DECLARE block) so that test evaluates to true and the program exits.

Alternatively the problem is this:

OPEN c1;
 LOOP
   ...
   FOR i in c1 LOOP

We can't use the FOR ... IN with an explicit cursor. You have opened the cursor. Then the FOR tries to open it again which hurls ORA-06511: PL/SQL: cursor already open. If you're not seeing this error you must have an exception handler which suppresses it (e.g. WHEN others then null;).

Basically the outer loop is completely unnecessary and you should discard it.

Explicit loop control is rarely necessary: just use the FOR ... IN construct and let Oracle control the flow.

Also unnecessary is all the dynamic SQL. SQL works with variables so you just need to write static SQL which references the cursor attributes:

 FOR i in (SELECT crs_cust.CUSTOMER_ID AS CUSTOMER_ID
                 , subset.NEW_CUSTOMER_REFERENCE_ID AS CUSTOMER_REF_ID 
           FROM CRS_CUSTOMERS crs_cust 
           INNER JOIN  DAY0_SUBSET subset
           ON crs_cust.CUSTOMER_ID=subset.CURRENT_CUSTOMER_ID )
 LOOP
     UPDATE CRS_CUSTOMERS 
     SET REF_ID = i.CUSTOMER_REF_ID
     WHERE CUSTOMER_ID = i.CUSTOMER_ID; 
     p_TotalUpdatedCRS := p_TotalUpdatedCRS + 1;

     UPDATE CRS_REVIEWS
     SET REF_ID =  i.CUSTOMER_REF_ID
     WHERE CUSTOMER_ID =  i.CUSTOMER_ID; 

     UPDATE CRS_EVENT 
     SET REF_ID = i.CUSTOMER_REF_ID 
     WHERE UNIQUE_ID = i.CUSTOMER_ID;

     UPDATE ALERT_HEADER 
     SET CUSTOMER_SOURCE_REF_ID = i.CUSTOMER_REF_ID 
     WHERE CUSTOMER_ID = i.CUSTOMER_ID; 
END LOOP;
DBMS_OUTPUT.PUT_LINE ('The total updates to CRS table = ' || p_TotalUpdatedCRS); 

I'm not sure of the purpose of the c1%ROWCOUNT <> p_SCBCount. My hunch is it's superfluous, because the FOR LOOP controls the fetches precisely. In fact I suspect you added it to avoid the side-effects of the nested loops; and I suspect you only introduced the nested loops because you're original code hurled PLS-00376: illegal EXIT/CONTINUE statement; it must appear inside a loop (just a wild guess).

However, if it does serve to implement some genuine business logic you can add it into the loop somehow.

APC
  • 144,005
  • 19
  • 170
  • 281
  • I think it's the `c1%rowcount` check, as that requires `p_SCBCount` to be `0` which I'm guessing it isn't. The `c1%found`/`%notfound` cursor attributes reflect fetch results and so will be null at this point. – William Robertson Apr 17 '18 at 06:49
  • @WilliamRobertson - thanks for making me test it. You're right that the problem is `c1%rowcount`. – APC Apr 17 '18 at 07:07
  • @WilliamRobertson, @APC, `c1%rowcount` is to compare against with `p_SCBCount`, if both are not same it should exit without running sp – user2102665 Apr 17 '18 at 07:22
  • @user2102665 - so what is the starting value of `p_SCBCount`? – APC Apr 17 '18 at 07:26
  • ok , i added `EXIT WHEN (c1%ROWCOUNT <>p_SCBCount`) before `END LOOP`, make sure fetch is executed before that point. – user2102665 Apr 17 '18 at 07:31
  • @APC- the record count of another table `SELECT COUNT(*) INTO p_SCBCount FROM DAY0_SUBSET;` – user2102665 Apr 17 '18 at 07:33
  • @user2102665 `c1%rowcount` reflects fetches, so it will be 0 initially and then increment by 1 each time you fetch a row. But cursor FOR loops open and fetch as well so your code is messed up. Run it through a debugger to see what’s happening. – William Robertson Apr 17 '18 at 08:01
  • i need to dynamic, as the table name in update statement has a prefix 'T_' . Example: ` l_prefix := 'T_';` ` EXECUTE IMMEDIATE 'UPDATE '||l_prefix||'CRS_CUSTOMERS SET REF_ID = '||i.CUSTOMER_REF_ID||' WHERE CUSTOMER_ID = '||i.CUSTOMER_ID; ` – user2102665 Apr 17 '18 at 08:06
  • 1
    So the posted code doesn't reflect the actual code you're running??? Why are you wasting our time like this? – APC Apr 17 '18 at 08:32
  • sorry, it was some last minutes requirement i need to apply the Prefix as table name – user2102665 Apr 17 '18 at 09:16
  • The thing is, you're asking us to explain why your code is doing a weird thing but you haven't posted the exact code you're running. Without the whole code we're just guessing. – APC Apr 17 '18 at 09:49
  • @APC, i repost again, please refer below site,Error coming from first `EXECUTE IMMEDIATE` when trying to update the first fetched record. https://stackoverflow.com/questions/49873744/sp-run-without-error-but-execute-immediate-and-dmbs-output-not-executing?noredirect=1#comment86769070_49873744 – user2102665 Apr 17 '18 at 12:21
1

There's nothing dynamic in your code; why bother with it?

This is a code which should work (unless I made a typo, as I don't have your tables):

DECLARE
   l_cnt   NUMBER := 0;
BEGIN
   FOR cur_r
      IN (SELECT crs_cust.customer_id,
                 subset.new_customer_reference_id AS customer_ref_id
            FROM crs_customers crs_cust
                 INNER JOIN day0_subset subset
                    ON crs_cust.customer_id = subset.current_customer_id)
   LOOP
      UPDATE crs_customers
         SET ref_id = cur_r.customer_ref_id
       WHERE customer_id = cur_r.customer_id;

      l_cnt := l_cnt + SQL%ROWCOUNT;

      UPDATE crs_reviews
         SET ref_id = cur_r.customer_ref_id
       WHERE customer_id = cur_r.customer_id;

      UPDATE crs_event
         SET ref_id = cur_r.customer_ref_id
       WHERE unique_id = cur_r.customer_id;

      UPDATE alert_header
         SET customer_source_ref_id = cur_r.customer_ref_id
       WHERE customer_id = cur_r.customer_id;
   END LOOP;

   DBMS_OUTPUT.put_line ('The total updates to CRS table = ' || l_cnt);
END;

As of your current problems: is there, by any chance, the WHEN OTHERS exception handler in your code (and you didn't post it)? If so, remove it.

Besides, this is wrong (just one example; you have it everywhere):

SET REF_ID = ' || i.CUSTOMER_REF_ID || 'WHERE UNIQUE_ID = ' || 
                                        ^
                                        a space missing here; should be

                                    ||' WHERE UNIQUE_ID = ' ||
Littlefoot
  • 131,892
  • 15
  • 35
  • 57
  • i need to dynamic, as the table name in update statement has a prefix 'T_' . Example: ` l_prefix := 'T_';` `EXECUTE IMMEDIATE 'UPDATE '||l_prefix||'CRS_CUSTOMERS SET REF_ID = '||i.CUSTOMER_REF_ID||' WHERE CUSTOMER_ID = '||i.CUSTOMER_ID; ` – user2102665 Apr 17 '18 at 08:02
  • the `EXECUTE IMMEDIATE` still not executing. – user2102665 Apr 17 '18 at 08:05
  • 1
    Prefix T_ still doesn't make it dynamic. Simply name the table T_CRS_CUSTOMERS. As of "still not executing" - I'm not a magician & can't see what you are doing. Please, post your SQL*Plus session which shows what you did and how Oracle responded. – Littlefoot Apr 17 '18 at 08:15
  • to make it configurable, prefix has to be parameterised – user2102665 Apr 17 '18 at 08:19
0

I think you don't need any loop at all. This code should do the same:

UPDATE CRS_CUSTOMERS 
SET REF_ID = 
    (SELECT subset.NEW_CUSTOMER_REFERENCE_ID
    FROM CRS_CUSTOMERS crs_cust 
        INNER JOIN DAY0_SUBSET subset ON crs_cust.CUSTOMER_ID = subset.CURRENT_CUSTOMER_ID
    WHERE CUSTOMER_ID = crs_cust.CUSTOMER_ID)
WHERE CUSTOMER_ID =ANY 
    (SELECT crs_cust.CUSTOMER_ID
    FROM CRS_CUSTOMERS crs_cust 
        INNER JOIN DAY0_SUBSET subset ON crs_cust.CUSTOMER_ID = subset.CURRENT_CUSTOMER_ID);    

p_TotalUpdatedCRS := SQL%ROWCOUNT;


UPDATE CRS_REVIEWS 
SET REF_ID = 
    (SELECT subset.NEW_CUSTOMER_REFERENCE_ID
    FROM CRS_CUSTOMERS crs_cust 
        INNER JOIN DAY0_SUBSET subset ON crs_cust.CUSTOMER_ID = subset.CURRENT_CUSTOMER_ID
    WHERE CUSTOMER_ID = crs_cust.CUSTOMER_ID)
WHERE CUSTOMER_ID =ANY 
    (SELECT crs_cust.CUSTOMER_ID
    FROM CRS_CUSTOMERS crs_cust 
        INNER JOIN DAY0_SUBSET subset ON crs_cust.CUSTOMER_ID = subset.CURRENT_CUSTOMER_ID);    

UPDATE CRS_EVENT
SET REF_ID = 
    (SELECT subset.NEW_CUSTOMER_REFERENCE_ID
    FROM CRS_CUSTOMERS crs_cust 
        INNER JOIN DAY0_SUBSET subset ON crs_cust.CUSTOMER_ID = subset.CURRENT_CUSTOMER_ID
    WHERE CUSTOMER_ID = crs_cust.CUSTOMER_ID)
WHERE CUSTOMER_ID =ANY 
    (SELECT crs_cust.CUSTOMER_ID
    FROM CRS_CUSTOMERS crs_cust 
        INNER JOIN DAY0_SUBSET subset ON crs_cust.CUSTOMER_ID = subset.CURRENT_CUSTOMER_ID);    

UPDATE ALERT_HEADER 
SET CUSTOMER_SOURCE_REF_ID = 
    (SELECT subset.NEW_CUSTOMER_REFERENCE_ID
    FROM CRS_CUSTOMERS crs_cust 
        INNER JOIN DAY0_SUBSET subset ON crs_cust.CUSTOMER_ID = subset.CURRENT_CUSTOMER_ID
    WHERE CUSTOMER_ID = crs_cust.CUSTOMER_ID)
WHERE CUSTOMER_ID =ANY 
    (SELECT crs_cust.CUSTOMER_ID
    FROM CRS_CUSTOMERS crs_cust 
        INNER JOIN DAY0_SUBSET subset ON crs_cust.CUSTOMER_ID = subset.CURRENT_CUSTOMER_ID);    

DBMS_OUTPUT.PUT_LINE ('The total updates to CRS table = ' || p_TotalUpdatedCRS); 

Note, you run p_TotalUpdatedCRS := p_TotalUpdatedCRS + 1; no matter whether there was an update (of one or several rows) or not. I don't think this is your intention as you like to print the number of updated rows.

Wernfried Domscheit
  • 54,457
  • 9
  • 76
  • 110
  • no, i need the loop, as `c1` will return multiple `CUSTOMER_ID` and `CUSTOMER_REF_ID`. Each of the `CUSTOMER_ID` need to be updated with the new `CUSTOMER_REF_ID`, this is the intention of those update script. As `c1` already joined the tables, I don't need to join again during update statement. – user2102665 Apr 17 '18 at 09:39
  • Agree with you the issue on `p_TotalUpdatedCRS := p_TotalUpdatedCRS + 1;` intention is to print the number of updated rows. – user2102665 Apr 17 '18 at 09:43
  • A single UPDATE statement usually much faster than doing it one-by-one in a loop. Unless I made a typo I don't see any reason why simple UPDATE does not work. – Wernfried Domscheit Apr 17 '18 at 09:44
  • the subquery in UPDATE statement failed as return more than one row. As I said multiple rows return when you joined up that 2 tables, as what `c1` is returning. – user2102665 Apr 17 '18 at 09:51
  • Then add `DISTINCT` to subquery. Apart from that, the other solutions work also. – Wernfried Domscheit Apr 17 '18 at 10:06
  • Sorry..cant get you, each of the value for return in the join is totally not same, and each of them need to be updated with new REF ID – user2102665 Apr 17 '18 at 10:14