0

I believe I may have uncovered a production bug, which causes intermittent problems. Basically, I am trying to understand what the AS400 does when dealing with embedded SQL and cursors. I am thinking that the cursor does not get closed in some cases, causing the next case to fail since the cursor is still open.

Here is a snapshot of the code:

begsr checkfile;

exec sql                               
declare T1 cursor for               
select * from FILE1              
where field1 = :var1;             

exec sql                               
open T1;                            

exec sql                               
fetch next from T1 into :vrDS;      

dow SQLCOD = *zeros;            
  if a=b;
    eval found = 'Y';
    leavesr;
  endif;     
enddo;

exec sql
close T1;

endsr;

My concern is in the leavesr line. If the condition is met, it leaves the subroutine, which skips the close of the cursor T1. In the job log, there are informational messages like "Cursor T1 already open or allocated.". I assume this means that it didn't do anything, or maybe even fetched from the previous cursor? I'm also wondering if the declare statement gets executed every time, or if it just skips that part of the code after the first execution of it. I think I need to put a close T1 statement before the open statement, but I wanted to get a second opinion since it's a production issue that is nearly impossible to recreate due to security key differences between test & production.

Thanks!

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
user1420914
  • 279
  • 1
  • 4
  • 16
  • bad code bad answers. leavesr before the close. And we have a answer that doesn't recognize that. – danny117 Sep 10 '18 at 15:20
  • @danny117 if you have a better answer that can teach a new user something about how to use cursors effectively, please share it, otherwise your comments are unhelpful. – jmarkmurphy Sep 10 '18 at 17:20
  • @jmarkmurphy the error is the leavesr. its like a goto in a pinkdress use with caution. See the cursor isn't closed. Next time the code runs the cursor is still open it will fail This is something that could've been answered with a comment. – danny117 Sep 12 '18 at 15:40
  • @danny117 Yes, that is obvious to any casual observer, Charles covered it in his answer where he said "the cursor would be left open", I expanded on Charles' answer. See the part where it says "What Charles said" right up front? Do you have an issue with providing a better way? – jmarkmurphy Sep 12 '18 at 18:05
  • @danny117 in fact the question was (paraphrased) "I'm having trouble with the logic for processing this cursor, seems like the leavesr is causing it to be left open, but not quite sure how to handle it". A comment that says "leavesr is the problem" doesn't really answer the question, it just confirms the OP concerns. – jmarkmurphy Sep 12 '18 at 18:10

2 Answers2

1

a DECLARE CURSOR is actually a compile time statement.

It's never executed at run-time.

:var1 is passed to the DB when the OPEN is done at run time. Here's an answer with a detailed example Using cursor for multiple search conditions

Yes, as the code is shown, the cursor would be left open. And possibly read from rather than opening a fresh cursor during the next run. Depends on what the CLOSQLCUR option is during compile (or as set using EXEC SQL SET OPTION)

You should be checking SQLSTATE/SQLCODE after the OPEN and the FETCH

Another common practice is to do a CLOSE before the OPEN, again be sure to check SQLSTATE/SQLCODE and ignore the CURSOR NOT OPEN on the close.

Charles
  • 21,637
  • 1
  • 20
  • 44
  • 1
    Thanks, after further review I think instead of a leavesr I just need to do a leave so it ends the do group instead of the subroutine, ensuring that the cursor is closed every time. Thanks for the information. It's much clearer how the AS400 operates in this regard now. :) – user1420914 Sep 07 '18 at 15:45
1

What Charles said, and also I believe you may in some cases even create an infinite loop with this code! Maybe you aren't giving the whole code, but if the fetch is successful (SQLCOD = 0) and a <> b, then you are stuck in a loop.

I like to put the fetch in a sub procedure that returns *On if a record is successfully read. Then you can do something like this:

dcl-proc MyProc;
  dcl-pi *n;
    ... parameters ...
  end-pi;

  C1_OpenCursor(parameters);
  dow C1_FetchCursor(record);
    ... do something with the record ...
  enddo;
  C1_CloseCursor();
end-proc;

// ------------------------
// SQL Open the cursor
dcl-proc C1_OpenCursor;
  dcl-pi *n;
    ... parameters ...
  end-pi;

  exec sql
    declare C1 cursor for ...

  exec sql
    open C1;
  if SQLCOD < 0;
    .. error processing ...
  endif;
end-proc;

// ------------------------
// SQL Read the cursor
dcl-proc C1_FetchCursor;
  dcl-pi *n Ind;
    ... parameters ...
  end-pi;

  exec sql
    fetch C1 into ...
  if SQLCOD = 100;
    return *Off;
  elseif SQLCOD < 0;
    ... error handling ...
    return *Off;
  endif;

  return *On;
end-proc;

// ------------------------
// SQL Close the cursor
dcl-proc C1_CloseCursor;

  exec sql close C1;
end-proc;

This lets you keep all of your database code in one place, and just call it from your program. Database procedures just access the database and report errors in some way. Your program logic can remain uncluttered by sometimes wordy database and error handling code.

One other thing, I don't check for errors on cursor close because the only error (other than syntax errors) that can be returned here is that the cursor is not open. I don't care about that because that's what I want anyway.

jmarkmurphy
  • 11,030
  • 31
  • 59
  • Hi jmark, Thanks for the response! Yes, you are correct. I just put that line in there as an example. The actual code has checks to keep it from getting stuck in a loop. I do like the sub procedure way you mentioned though. I will give that a shot on the next development project for sure! – user1420914 Sep 10 '18 at 12:58
  • leavesr before close. Done. – danny117 Sep 10 '18 at 15:21