New Rule: use SAVE EXCEPTIONS in FORALL statement

[silviomarghitola]

Language Usage / Exception Handling
Always use the SAVE EXCEPTIONS clause in a FORALL statement

[PhilippSalvisberg]

from SonarSource:

"FORALL" statements should use the "SAVE EXCEPTIONS" clause

When the FORALL statement is used without the SAVE EXCEPTIONS clause and an exception is raised by a DML query, the whole operation is rolled back and the exception goes unhandled. Instead of relying on this default behavior, it is better to always use the SAVE EXCEPTIONS clause and explicitly handle exceptions in a ORA-24381 handler.

Noncompliant Code Example

CREATE TABLE my_table(
  id NUMBER(10) NOT NULL
);

DECLARE
  TYPE my_table_id_type IS TABLE OF my_table.id%TYPE;
  my_table_ids my_table_id_type := my_table_id_type();
BEGIN
  FOR i IN 1 .. 10 LOOP
    my_table_ids.EXTEND;
    my_table_ids(my_table_ids.LAST) := i;
  END LOOP;

  -- Cause the failure
  my_table_ids(10) := NULL;

  FORALL i IN my_table_ids.FIRST .. my_table_ids.LAST  -- Noncompliant
    INSERT INTO my_table
    VALUES (my_table_ids(i));
END;
/

SELECT COUNT(*) FROM my_table;

DROP TABLE my_table;

Compliant Solution

-- ...

DECLARE
  TYPE my_table_id_type IS TABLE OF my_table.id%TYPE;
  my_table_ids my_table_id_type := my_table_id_type();

  bulk_errors EXCEPTION;
  PRAGMA EXCEPTION_INIT(bulk_errors, -24381);
BEGIN
  FOR i IN 1 .. 10 LOOP
    my_table_ids.EXTEND;
    my_table_ids(my_table_ids.LAST) := i;
  END LOOP;

  -- Cause the failure
  my_table_ids(10) := NULL;

  FORALL i IN my_table_ids.FIRST .. my_table_ids.LAST SAVE EXCEPTIONS
    INSERT INTO my_table
    VALUES (my_table_ids(i));
EXCEPTION
  WHEN bulk_errors THEN
    -- Explicitly rollback the whole transaction,
    -- or handle each exception individually by looping over SQL%BULK_EXCEPTIONS
    ROLLBACK;
END;
/

[PhilippSalvisberg]

Using the SAVE EXCEPTIONS clause and catching an ORA-24381: error(s) in array DML make sense. But always?

However, using SAVE EXCEPTIONS without handling the exceptions does never make sense. Maybe this could be an alternative rule.

[silviomarghitola]

If I allow every step within a FORALL loop to throw an exception and therefore make this and every previous step of the loop to roll back, then I only know that this was the first step that failed.
If I use the SAVE EXCEPTIONS clause, I can identify every step of the loop that causes problems.
In both cases the whole FORALL-loop is considered one single transaction.

[PhilippSalvisberg]

My point is. Do we really want to force everyone to use SAVE EXCEPTIONS in their FORALL loops? Even if they do not want to handle the saved exceptions, maybe because it is not of interest in this case and the errors can be handled in globo.

But if you use the SAVE EXCEPTIONS clause and you do nothing with the saved exceptions, then this is for sure a bad thing.

[rogertroller]

forall is not a loop.

if i want to have a all or nothing processing of all elements of the base-collection of the forall, then i see no reason for a save exception usage.

[PhilippSalvisberg]

Ok, thanks @rogertroller. Based on that I suggest to change the rule to

Always process saved exceptions

Saving exceptions but not processing them is similar to keeping unused variables.