Error when executing stored procedure in Oracle PL/SQL loop index variable 'I' use is invalid

0
votes

I'm fairly new to Oracle PL/SQL and Im trying to execute a stored procedure which inserts into the orders table and loops through to insert a collection of items into the order_item table. I've created a type which has been defined, but when I try to execute my procedure I get this loop index variable 'I' use is invalid.

SQL> -- Execute procedure
SQL> DECLARE
  2  order_id_ NUMBER;
  3
  4  BEGIN
  5      insert_order(p_order_id => 4, p_order_num => 'O223PS56', p_name => 'Test Test', p_email => '[email protected]', p_address => '123 Test Street', p_city => 'Newcastle Upon Tyne', p_province => 'Tyne and Wear', p_postcode => 'NE98 4TN', p_telephone => '123456789', p_total => 7.97, p_order_date => to_date('11-apr-2021', 'DD-mon-YYYY'));
  6      FOR i IN i..order_items
  7      LOOP
  8        insert_order_items(order_id_, order_items(i) => 5, order_items(i) => 2, order_items(i) => 2, order_items(i) => 3073748221, order_items(i) => 2, order_items(i) => 'Brand New', order_items(i) => 1.99, order_items(i) => 1.99);
  9       COMMIT;
 10     END LOOP;
 11  END;
 12  /
    FOR i IN i..order_items
             *
ERROR at line 6:
ORA-06550: line 6, column 14:
PLS-00364: loop index variable 'I' use is invalid
ORA-06550: line 6, column 5:
PL/SQL: Statement ignored
SQL>

CREATE OR REPLACE PROCEDURE insert_order
(
    p_order_id IN INT, p_order_num IN CHAR,
    p_name IN CHAR, p_email IN CHAR,
    p_address IN VARCHAR2, p_city IN VARCHAR2,
    p_province IN VARCHAR2, p_postcode IN VARCHAR2,
    p_telephone IN NUMBER, p_total IN NUMBER,
    p_order_date IN DATE
)
AS
BEGIN
    INSERT INTO orders(order_id, order_number, billing_name, billing_email, billing_address, billing_city, billing_province, billing_postcode, billing_telephone, billing_total, order_date)
    values(p_order_id, p_order_num, p_name, p_email, p_address, p_city, p_province, p_postcode, p_telephone, p_total, p_order_date);

    COMMIT;
END;
/

CREATE OR REPLACE PROCEDURE insert_order_items
(
    p_order_item_id IN INT, p_order_id IN INT, 
    p_product_id IN INT, p_seller_id IN INT,
    p_sub_order_number IN CHAR, p_quantity IN INT,
    p_condition IN CHAR, p_unit_price IN NUMBER, 
    p_cost_charge IN NUMBER
)
AS
BEGIN
    INSERT INTO order_item(order_item_id, order_id, product_id, seller_id, sub_order_number, quantity, condition, unit_price, cost_charge)
    values(p_order_item_id, p_order_id, p_product_id, p_seller_id, p_sub_order_number, p_quantity, p_condition, p_unit_price, p_cost_charge);
    COMMIT;
END;
/

CREATE OR REPLACE TYPE order_items_collection as object(
    order_id int, product_id int,
    seller_id int, sub_order_number char(10),
    quantity int, condition char(100),
    unit_price number, cost_charge number
);
/

CREATE OR REPLACE TYPE order_items is table of order_items_collection;
/
2
oracleplsqloracle11g
FOR i IN i..order_items doesn't make sense; FOR I IN 1..order_items might, if order_items was a number; but it is being treated later as a collection so you probably want FOR I IN order_items.FIRST..order_items.LAST. Except order_items doesn't seem to be defined. Where is that collection supposed to come from? - Alex Poole
@AlexPoole when changed to FOR I IN 1..order_items I get invalid use of type name or subtype name but order_items has been defined as a type unless it can't find it. Using FOR I IN order_items.FIRST..order_items.LAST say component 'FIRST' must be declared - Jal
But you need an instance of that type - i.e. a variable of that type, which you have populated with the data you want to to refer to in the loop. What you are doing inside the loop doesn't make sense though. Why do you have that type and collection at all - the procedures don't use it? It isn't really clear what you are trying to do. - Alex Poole
So what I'm trying to do is execute the insert_order procedure which populates data into the orders table, then executing another procedure insert_order_items, however an order may have more than one order_item. So I’m trying create a procedure that creates an order called insert_order and another procedure that is called in a loop to insert the order_item rows. - Jal
You have hard-coded values for the order and a single item. Maybe you want to pass in a collection of items, and maybe an object for the order itself; but that isn't what you're doing at the moment. - Alex Poole

2 Answers

0
votes

You are declaring a procedure, and calling it as a function. That's why it's not working.

Try this:

DECLARE
order_id_ NUMBER;

BEGIN
    -- note here that you're now assigning the return value to anything
    insert_order(p_order_id => 4, p_order_num => 'O223PS56', p_name => 'Test Test', p_email => '[email protected]', p_address => '123 Test Street', p_city => 'Newcastle Upon Tyne', p_province => 'Tyne and Wear', p_postcode => 'NE98 4TN', p_telephone => '123456789', p_total => 7.97, p_order_date => to_date('11-apr-2021', 'DD-mon-YYYY'));
    FOR i IN 1..order_items
    LOOP
      insert_order_items(order_id_, order_items(i) => 5, order_items(i) => 2, order_items(i) => 2, order_items(i) => 3073748221, order_items(i) => 2, order_items(i) => 'Brand New', order_items(i) => 1.99, order_items(i) => 1.99);
     COMMIT;
   END LOOP;
END;
/
0
votes

This:

FOR i IN i..order_items

doesn't make sense; i is the loop variable so it can't be the range as well. You could do:

for i in order_items.FIRST..order_items.LAST

or as this isn't a sparse array, maybe more simply:

for i in 1..order_items.COUNT loop

But order_items is a type, and you can't iterate over a type. You would iterate over an instance of the collection type. You haven't declare, or populated, a variable of that type. And within the loop your references to the type don't make sense, and wouldn't if it was an instance.

As far as I can tell, you're trying to do something like this:

DECLARE
  order_id_ NUMBER;
  order_items_ order_items;
BEGIN
...
  -- create and populate instance of the collection type
  order_items_ := new order_items(
    order_items_collection(5, 2, 2, 3073748221, 2, 'Brand New', 1.99, 1.99)
  );

  -- iterate over the collection type
  for i in 1..order_items_.COUNT loop
    insert_order_items(
      p_order_item_id => order_id_,            -- these two values might be reversed?
      p_order_id => order_items_(i).order_id,  -- 
      p_product_id => order_items_(i).product_id,
      p_seller_id => order_items_(i).seller_id,
      p_sub_order_number => order_items_(i).sub_order_number,
      p_quantity => order_items_(i).quantity,
      p_condition => order_items_(i).condition,
      p_unit_price => order_items_(i).unit_price,
      p_cost_charge => order_items_(i).cost_charge
    );
  end loop;
END;
/

db<>fiddle demo

That would allow you to insert multiple values without repeating the procedure call, e.g. by changing:

  -- create and populate instance of the collection type
  order_items_ := new order_items(
    order_items_collection(5, 2, 2, 3073748221, 2, 'Brand New', 1.99, 1.99)
  );

to

  -- create and populate instance of the collection type
  order_items_ := new order_items(
    order_items_collection(5, 2, 2, 3073748221, 2, 'Brand New', 1.99, 1.99),
    order_items_collection(6, 3, 4, 3073748222, 1, 'Used', 19.99, 19.99),
    order_items_collection(7, 8, 9, 3073748223, 7, 'Refurb', 9.99, 9.99)
  );

(though from that you can see your object naming is a bit confusing).

With hard-coded values that doesn't seem much easier or simpler than hard-coding those values into three procedure calls, but perhaps your goal is to pass the collection in from somewhere else, like a front-end application. Or maybe you want to pass the collection into your procedure, and have that iterate over the collection and perform the inserts. At the moment both procedures seem to just be complicating things a bit, and preventing you from doing bulk inserts.

It also isn't generally a good idea to commit inside a procedure, particularly one that you're calling in a loop for related data. If the third item insert fails, you've already committed the inserts from the order and the first two items, so you're left in an inconsistent state. It looks like all of those inserts should be inserted in the same transactions, and committed (or rolled back) as one unit.