surre5l
surre5l

Reputation: 27

uninitialized collection shown when procedure is called in pl/sql

I have seen the other similar questions but I cannot understand what is the problem in my code. The following is the procedure definition

Proc.sql

set serveroutput on

create or replace type myarray is varray(1000) of number;
/ 
create or replace procedure Bill(cid in number , bill out number) is
md myarray;
q myarray;
pr myarray;
rs myarray;
begin
bill:=0;
select model,quantity BULK COLLECT into md,q from transactions where customer=cid;

for i in 1..md.COUNT loop
select price BULK COLLECT into pr from computers where model_no=md(i);
END LOOP;

for i in 1..md.COUNT loop
rs(i):=q(i)*pr(i);
end loop;



for i in 1..md.COUNT loop
bill:=bill+rs(i);
end loop;

end;
/
show errors;

and this is my file for calling the procedure

CallProc.sql

set serveroutput on
declare
bll number(2):=0;
begin
Bill(1,bll);
DBMS_OUTPUT.PUT_LINE('T = '|| bll);
end;
/

Upvotes: 0

Views: 218

Answers (1)

Alex Poole
Alex Poole

Reputation: 191235

If you initialise the array as an empty collection:

declare
  ...
  rs myarray := myarray();
begin

then you have to increase its capacity, either as you go around the loop:

for i in 1..md.COUNT loop
  rs.extend;
  rs(i):=q(i)*pr(i);
end loop;

or in one go (which should be more efficient):

rs.extend(md.COUNT);
for i in 1..md.COUNT loop
  rs(i):=q(i)*pr(i);
end loop;

You still have to initialise the empty collection before you extend it.


You have other issues though. You are doing:

rs(i):=q(i)*pr(i);

but the rs and pr collections are completely independent, so their indices have no relationship. I think you are trying to add records to pr each time round this loop:

for i in 1..md.COUNT loop
select price BULK COLLECT into pr from computers where model_no=md(i);
END LOOP;

but what you are actually doing is replacing the entire contents of the collection with a single value, not appending; and at the end of that loop you will only have the price for the last row in the md collection. The only i value that is valid for that collection is 1, as only pr(1) exists (assuming model_no is unique); and unless the customer only has one transaction that won't be the right price, except for the last model (where i != 1).

You could just get the relevant price inside the loop, without a collection for that:

create or replace procedure Bill(cid in number , bill out number) is
  md myarray;
  q myarray;
  pr number; -- scalar variable
  rs myarray := myarray();
begin
  select model, quantity bulk collect into md, q from transactions where customer=cid;

  rs.extend(md.COUNT);
  for i in 1..md.COUNT loop
    select price into pr from computers where model_no=md(i); -- not bulk collect
    rs(i):=q(i)*pr; -- pr no longer indexed
  end loop;

  bill:=0;
  for i in 1..md.COUNT loop
    bill:=bill+rs(i);
  end loop;
end;
/

It would be more efficient to get the prices aligned with the rest of the data by joining the tables and doing a single bulk collect into all three collections:

create or replace procedure Bill(cid in number , bill out number) is
  md myarray;
  q myarray;
  pr myarray;
  rs myarray := myarray();
begin
  select t.model, t.quantity, c.price
  bulk collect into md, q, pr
  from transactions t
  join computers c on c.model_no = t.model
  where t.customer = cid;

  rs.extend(md.COUNT);
  for i in 1..md.COUNT loop
    rs(i):=q(i)*pr(i);
  end loop;

  bill:=0;
  for i in 1..md.COUNT loop
    bill:=bill+rs(i);
  end loop;
end;
/

But then you can simplify further by doing both calculations in one loop:

  bill:=0;
  rs.extend(md.COUNT);
  for i in 1..md.COUNT loop
    rs(i):=q(i)*pr(i);
    bill:=bill+rs(i);
  end loop;

and from that you can see you don't really need rs at all, you can just calculate the bill change directly:

create or replace procedure Bill(cid in number , bill out number) is
  md myarray;
  q myarray;
  pr myarray;
begin
  select t.model, t.quantity, c.price
  bulk collect into md, q, pr
  from transactions t
  join computers c on c.model_no = t.model
  where t.customer = cid;

  bill:=0;
  for i in 1..md.COUNT loop
    bill := bill + q(i)*pr(i);
  end loop;
end;
/

But you don't need loops or collections at all:

create or replace procedure Bill(cid in number , bill out number) is
begin
  select sum(t.quantity * c.price)
  into bill
  from transactions t
  join computers c on c.model_no = t.model
  where t.customer = cid;
end;
/

and obviously you could just run that query as plain SQL without involving PL/SQL or a procedure at all.

Depends what your assignment is telling you to use...

Upvotes: 1

Related Questions