PL/SQL Cursor whose query is based on a variable

Question

I've got a PL/SQL block that's basically

DECLARE
    PIDM            NUMBER(8);
    CLM_TEST_SCORE  NUMBER(5);
    CURSOR C_STUDENT IS
        select PIDM
        from   SOSC.DW_ALL_COLLECTOR
        order by PIDM;

    CURSOR C_CLM_SCORES IS
        select max(to_number(SORTEST_TEST_SCORE))
        from   SATURN.SORTEST
        where  SORTEST_PIDM   = pidm;
BEGIN
    OPEN C_STUDENT;
    LOOP
        CLM_TEST_SCORE := '';

        FETCH c_Student INTO pidm;
        EXIT WHEN c_Student%notfound;

        OPEN C_CLM_SCORES;
        FETCH C_CLM_SCORES INTO CLM_TEST_SCORE;
        CLOSE C_CLM_SCORES;
        
        insert into some_table (CLM_TEST_SCORE)
        values (CLM_TEST_SCORE);

    END LOOP
END

As far as I'm aware, the pidm referred to in C_CLM_SCORES is the PIDM NUMBER(8) declared in line 2. That would mean that the query the cursor refers to mutates every iteration of the LOOP, depending on the current value of pidm. That doesn't jive with my understanding of cursors as a query-in-progress, as the underlying query changes every LOOP. Maybe it's the original author taking advantage of a clever DB algorithm?

This code works. I just have absolutely no idea why. What the heck is going on here?

Looks like this might be an example of dynamic-sql? Not clear enough on what that is to tell. — Jakob Lovern
– Jakob Lovern, Commented Sep 7, 2022 at 22:28
I am not sure what "This code works" means. What does it do exactly, and what part is not behaving as expected? Is it the part where C_CLM_SCORES has binding applied each time it is opened? — William Robertson
– William Robertson, Commented Sep 7, 2022 at 22:31
I'm trying to learn more about SQL by diving into my org's production codebase, so the code is known good code. The part I don't understand is how the DB knows to re-apply pidm inside C_CLM_SCORES every time. — Jakob Lovern
– Jakob Lovern, Commented Sep 7, 2022 at 22:33
The query is the same at each iteration, because pidm is effectively a bind variable: cursor gets opened at each loop pass and accepts new input. Though I personally do not like functions with shared variables, and this code may be parameterized explicitly by adding parameter to the cursor definition — astentx
– astentx, Commented Sep 7, 2022 at 22:51
As I see, the answer was updated with the same statement and code sample, so it is a better way to express this relationship (not to perform the task) and it contains a better way to performinsert also — astentx
– astentx, Commented Sep 7, 2022 at 22:55

MT0 · Accepted Answer · 2022-09-07 22:53:14Z

3

You have an overly confusing block of code that is a nightmare to debug as you have:

SQL statements that refer to column name and local variables with the same identifier (PIDM and CLM_TEST_SCORE).
Cursors that change every iteration because they contain a bind variable referring to local variables (PIDM).
Highly inefficient use of loops.

If you want to make it clearer, you can rewrite the PL/SQL block so that you do not have duplicate identifiers and use a parameterised cursor:

DECLARE
  v_PIDM            SOSC.DW_ALL_COLLECTOR.PIDM%TYPE;
  v_CLM_TEST_SCORE  some_table.CLM_TEST_SCORE%TYPE;

  CURSOR C_STUDENT IS
    select PIDM
    from   SOSC.DW_ALL_COLLECTOR
    order by PIDM;

  CURSOR C_CLM_SCORES(p_pidm NUMBER) IS
    select max(to_number(SORTEST_TEST_SCORE))
    from   SATURN.SORTEST
    where  SORTEST_PIDM = p_pidm;
BEGIN
  OPEN C_STUDENT;
  LOOP
    FETCH c_Student INTO v_pidm;
    EXIT WHEN c_Student%notfound;

    OPEN C_CLM_SCORES(v_pidm);
    FETCH C_CLM_SCORES INTO v_CLM_TEST_SCORE;
    CLOSE C_CLM_SCORES;
        
    insert into some_table (CLM_TEST_SCORE)
    values (v_CLM_TEST_SCORE);
  END LOOP;
END;
/

However, that is still very inefficient as each iteration performs a SELECT and an INSERT and will generate log entries. You can make it much simpler and more efficient to rewrite the whole thing as a single SQL statement:

INSERT INTO some_table (clm_test_score)
SELECT ( select max(to_number(SORTEST_TEST_SCORE))
         from   SATURN.SORTEST s
         where  s.SORTEST_PIDM = c.pidm )
FROM   SOSC.DW_ALL_COLLECTOR c;

db<>fiddle here

edited Sep 7, 2022 at 22:53

answered Sep 7, 2022 at 22:37

MT0

173k12 gold badges70 silver badges136 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Jakob Lovern Over a year ago

A lot of the nightmare factor is because I'm trying to create a minimal reproducible example by cutting up an existing script. What I'm looking for is words and concepts that I can use to further research the cursor behavior, as the pattern seems to be a favorite of my predecessor.

Jakob Lovern Over a year ago

"Parameterized cursor" was exactly the term I needed! Thanks for the help.

Jakob Lovern Over a year ago

OK, so basically since the bind parameter changes each iteration, the cursor has to re-evaluate its query and produce a result set (rather than, say, caching the result set for next loop). The engine can reuse its execution plan, but not much else.

MT0 Over a year ago

@JakobLovern With the cursor, you get one select at the start and then another select each iteration (which, yes, may be able to re-use the execution plan but may not depending on the value of the bind variable but will also result in the SQL engine performing IO to retrieve the data and may fetch blocks multiple times if it is aged out of the cache between interations, etc.) and an insert (which each writes to the table and to the undo/redo log). Using a single INSERT ... SELECT ... statement eliminates almost all of the selects and may perform less IO and only writes to the log files once.

Bob Jarvis - Слава Україні · Accepted Answer · 2022-09-08 01:37:02Z

1

The code in the question is an advertisement for "Why should implicit cursors be used?". If you rewrite your code as below it becomes much easier to understand:

BEGIN
  FOR rowStudent IN (select PIDM
                       from SOSC.DW_ALL_COLLECTOR
                       order by PIDM)
  LOOP
    FOR rowScores IN (select max(to_number(SORTEST_TEST_SCORE)) AS CLM_TEST_SCORE
                        from SATURN.SORTEST
                        where SORTEST_PIDM = rowStudent.PIDM)
    LOOP
      insert into some_table (CLM_TEST_SCORE)
        values (rowScores.CLM_TEST_SCORE);
    END LOOP;  -- rowScores
  END LOOP;  -- rowStudent
END;

This eliminates all of the variables and cursor definitions, and all the code is right in front of you where you can see it at a glance.

If you wanted to tighten it up a bit further you could use a join to get down to just one cursor:

BEGIN
  FOR rowStudent_scores IN (SELECT d.PIDM, MAX(TO_NUMBER(s.SORTEST_TEST_SCORE)) AS CLM_TEST_SCORE
                              FROM SOSC.DW_ALL_COLLECTOR d
                              INNER JOIN SATURN.SORTEST s
                                ON s.SORTEST_PIDM = d.PIDM
                              GROUP BY d.PIDM)
  LOOP
    insert into some_table (CLM_TEST_SCORE)
      values (rowStudent_scores.CLM_TEST_SCORE);
  END LOOP;  -- rowStudent_scores
END;

answered Sep 8, 2022 at 1:37

Bob Jarvis - Слава Україні

50.2k10 gold badges81 silver badges119 bronze badges

1 Comment

MT0 Over a year ago

Your final query (with the join) is subtly different from the OP's code as it: 1) aggregates duplicate PIDM values together, whereas the OP's includes every duplicate from the driving table; 2) It will not include rows where the SORTED_PIDM value is not present but the PIDM value is present (but the OP's will include them as NULL); and 3) it will not include rows where the PIDM value is NULL. The latter two you can solve with a LEFT OUTER JOIN but the first is more difficult to solve (either use a correlated sub-query as per my answer or aggregate inside a CROSS JOIN LATERAL).

Collectives™ on Stack Overflow

PL/SQL Cursor whose query is based on a variable

2 Answers 2

4 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related