PL/SQL Cursor whose query is based on a variable-CodePudding

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?

CodePudding user response：

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

CodePudding user response：

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;