Re-use a cursor instead re-open

1.9k views Asked by At

I have a table with employees and for each employees have few bills. I declare 2 cursors, one for all employees(distinct) and second cursor for all bills for one employees. Now, open 1st cursor with all employees, fetch one, open second cursor (based on employee from 1st cursor) with all bills for employee. To reuse a second cursor for all employees, I open and close second cursor for each employee. This thing spend a lot of time. How to reuse a second cursor instead reopen or any good idea?

Part of code in Pro*C:

 struct sforc1 {
long nis_rad[ROWS_FETCHED_C1];
long sec_nis[ROWS_FETCHED_C1];
/*char f_fact[9];
long sec_rec;*/
}forc1;

struct sforc2 {
long nis_rad[ROWS_FETCHED_C2];
long sec_nis[ROWS_FETCHED_C2];
char f_fact[ROWS_FETCHED_C2][9];
long sec_rec[ROWS_FETCHED_C2];
char f_p_camb_est[ROWS_FETCHED_C2][9];
char op_cambest[ROWS_FETCHED_C2][9];
}forc2;

 void main (void)
{
exec sql declare c1 cursor for
        select distinct nis_rad, sec_nis
        from recibos
        where ((imp_tot_rec - imp_cta)>0) and f_p_camb_est = '29991231';

exec sql declare c2 cursor for
        select nis_rad, sec_nis, f_fact, sec_rec, f_p_camb_est, op_cambest 
        from recibos
    where ((imp_tot_rec - imp_cta)>0) and f_p_camb_est = '29991231' and nis_rad = :forc1.nis_rad[i] and sec_nis=:forc1.sec_nis[i];

exec sql open c1;

while(1){
    exec sql fetch c1 into :forc1;
    rows_this_time1 = sqlca.sqlerrd[2]-rows_before1;
    rows_before1 = sqlca.sqlerrd[2];

    if (rows_this_time1==0){
        break;
    }

    for(i=0;i<rows_this_time1;++i){

        exec sql open c2;
        rows_before2 = 0;

        while(1){

            exec sql fetch c2 into :forc2;
            rows_this_time2 = sqlca.sqlerrd[2]-rows_before2;
            rows_before2=sqlca.sqlerrd[2];
            if(rows_this_time2==0){
                break;
            }

            for(j=0;j<rows_this_time2;++j){
                strcpy(forc2.f_p_camb_est[j],  "20161212");
                strcpy(forc2.op_cambest[j], "SIMD0943");

            }

            EXEC SQL
                update recibos
                       set f_p_camb_est = :forc2.f_p_camb_est,
                           op_cambest = :forc2.op_cambest
                       where nis_rad = :forc2.nis_rad 
                       and sec_nis = :forc2.sec_nis 
                       and f_fact = :forc2.f_fact 
                       and sec_rec = :forc2.sec_rec;

        }

        exec sql close c2;

    }

    exec sql close c1;
    exec sql commit;
    exec sql open c1;
    rows_before1 = 0;
}

exec sql close c1;

}

nis_rad and sec_nis is a employee_id(primary key). Each nis_rad have few bills f_fact(bills)

For processing 10000 nis_rad's spend 30 min, and 28-29 min is for re-open second cursor(c2)

UP. Deleted previously example

3

There are 3 answers

6
Kacper On

I've edited an answer.

You can't reuse date from cursor without opening it again unless you cache all results in memory. Cursor is like pointer to data when you read record it already points to next so you can't get back.

Problem in your code is using SQL imperative way. You shouldn't pull all records to your application and then apply logic. Think about how to write a query that will return only records you need to process. It may be even faster to execute 2-3 queries that will return records for each section of your code than pull all data and then check logic in application.

1
XING On

I would say inplace of opening explicit cursor, let oracle use implicit cursor and use simple loops to achieve your requirements:

declare

begin
for rec in ( select employee_id
             from employees )
  Loop                   
          for rcrd in (Select bill from employee_bill 
                      where employee_id = rec.employee_id)
           loop

            /***Process your bills***/

           end loop;                           

  end loop;

end; 
1
Boneist On

The problem with your code is that it's doing an awful lot of context switching, so it's no wonder that it's slow.

From what I can tell, your code is doing something like:

  1. In Pro*C, go to the database
  2. In the database, open the first cursor
  3. Back in Pro*C, ask the database for a row from the cursor
  4. The database sends back a row
  5. In Pro*C, go to the database
  6. In the database, open the second cursor, passing in the values from the first cursor
  7. In Pro*C, ask the database for a row from the second cursor
  8. The database sends back a row
  9. In Pro*C, ask the database to update the row(s) based on the results from the second cursor
  10. The database updates the rows.
  11. Repeat steps 7-10 until there are no more rows to fetch from cursor 2
  12. Repeat steps 3.11 until there are no more rows to fetch from cursor 1

I think you can see that there's an awful lot of unnecessary to-ing and fro-ing going on there... and in fact, it's likely that that's where the majority of your time is being spent.

Were it not for your mentor's requirements (which I assume are for teaching purposes only), you could do this in a single update statement, like so:

update recibos
set    f_p_camb_est = '20161212',
       op_cambest = 'SIMD0943'
where  (imp_tot_rec - imp_cta) > 0
and    f_p_camb_est = '29991231'; 

(N.B. if f_p_camb_est is of DATE datatype, then the string literals need to be converted into DATEs using DATE or to_date(), e.g. DATE '29991231', to_date('20161212', 'yyyymmdd'), unless you're going to pass in a parameter that's already of DATE datatype.)

Doing it this way means that you can plug the update statement into your Pro*C app and it will do:

  1. In Pro*C, ask the database to execute the update statement
  2. The database executes the update statement
  3. Control is returned back to Pro*C.

That's 2 context switches in total - a massive reduction than from your original method!

However, your mentor has asked for (IMHO) the daft way of updating the rows by doing it one nis_rad at a time.

In that case, you are still far better off doing the bulk of the work in the database - there is *no* point in sending rows back to your app from the database only for your app to do nothing with them except pass them back into the database. The way you would do this on the database side is in PL/SQL, like so:

begin
  for rec in (select distinct nis_rad, sec_nis
              from   recibos
              where  (imp_tot_rec - imp_cta) > 0
              and    f_p_camb_est = '29991231')
  loop
    update recibos
    set    f_p_camb_est = '20161212',
           op_cambest = 'SIMD0943'
    where  nis_rad = rec.nis_rad
    and    (imp_tot_rec - imp_cta) > 0
    and    f_p_camb_est = '29991231';
  end loop;
end;
/

You could either call this directly from Pro*C as an anonymous block or you could create it as a procedure in the database (potentially with parameters), and then call the procedure from Pro*C.

Either way, the overall flow looks like:

  1. In Pro*C, ask the database to execute the PL/SQL
  2. The database executes the PL/SQL
  3. Control is returned back to Pro*C.

I know this looks like 2 context switches, but you also need to take account of the context switching within the database, when it switches between the PL/SQL and SQL engines, which looks something like:

  1. The PL/SQL engine requests a cursor
  2. The SQL engine opens the cursor
  3. The PL/SQL engine requests a row from the cursor
  4. The SQL engine passes a row back
  5. The PL/SQL engine stored the information in the rec record
  6. The PL/SQL engine asks the SQL engine to execute the update statement, based on the nis_rad value from the rec record
  7. The SQL engine runs the update statement
  8. Repeat steps 1 - 7 until no more rows are returned

which, as I'm sure you can see is a lot more than the 2 context switches we had overall if you had just run the single update statement instead.

Hopefully that clears things up a bit for you?