← All posts

Reading a 4,000 line PL/SQL package without losing a week

TopicPL/SQL
Published
Reading time7 minutes

The job was small. Add one hold reason to an invoice hold routine. The package was 4,100 lines, last changed in any real way in 2013, and the two people who wrote it left years ago. No tests, no design note. Just the spec, the body, and a wrapper concurrent program.

The first instinct is to paste the whole body in and ask for an explanation. We did that. What came back was fluent, well organised, and wrong about the one cursor that mattered, because it read the cursor declaration and never followed the two places the cursor variable was reassigned.

A confident summary of a package you have not verified is worse than no summary, because you stop reading.

What worked was asking in a fixed order, and refusing to move on until each answer had been checked against the source.

1. Map the entry points from the spec only

Give it the spec file and nothing else. Ask what the package exposes, what each public routine appears to do from its name and parameters, and which ones the concurrent program calls. You get a list of six or seven procedures instead of a wall of prose, and it is easy to check because the spec is 90 lines.

This also tells you how much of the body you can ignore. Of 4,100 lines, the change touched two public procedures and one private helper. The other 3,000 lines were never in scope.

2. Ask what data it touches, with line numbers

Next question: every table and view this package reads or writes, which routine does it, and the line number. Insist on line numbers. They are the cheapest verification you will get, because you can jump to each one and confirm in seconds.

This is where the first real finding turned up. The package wrote to a custom staging table that nobody on the current team knew existed, and that table had a trigger on it. The trigger was why a previous attempt at this change had been rolled back.

3. Narrow to the branch that matters

Only now ask about behaviour, and only for the path you care about. Something like: trace what happens when p_hold_type is 'QTY_REC' and the invoice already has an open hold, from the entry point to the commit. Say what you want in the answer: the sequence of calls, the conditions on each branch, and any place the flow can exit early.

A trace of one path is checkable. An explanation of the whole package is not.

4. Make it write the test first

Before any change, ask for a script that proves current behaviour. Set up an invoice, call the procedure, assert on what lands in the holds table, roll back. It does not need a framework. One anonymous block that raises on a bad result is enough.

DECLARE
  l_count  PLS_INTEGER;
BEGIN
  xx_ap_invoice_holds_pkg.apply_hold(p_invoice_id => 90210,
                                     p_hold_type  => 'QTY_REC');

  SELECT COUNT(*) INTO l_count
    FROM ap_holds_all
   WHERE invoice_id = 90210
     AND release_lookup_code IS NULL;

  IF l_count <> 1 THEN
    RAISE_APPLICATION_ERROR(-20001, 'Expected 1 open hold, got ' || l_count);
  END IF;

  ROLLBACK;
END;
/

Run it before you touch anything. If it fails on the unchanged package, your understanding is wrong, and it is far cheaper to find that out now.

5. Ask what it is unsure about

The last question in every session: what in this package do you not have enough information to be confident about? The answers were useful. It flagged two dynamic SQL blocks it could not resolve statically, and a call into another package that was not among the files we had given it. Both were real gaps. One of them changed the fix.

Where it still went wrong

It invented a column. ap_holds_all.hold_reason_desc does not exist on our instance, and it wrote code against it twice before we noticed. The fix is boring: keep the DESCRIBE output for every table in scope in the working directory, and tell it those files are the only source of truth for column names. The invented columns stopped after that.

Treat anything it says about your schema as a claim to check. It has read a lot of Oracle. It has not read your database.

What it cost

Two afternoons instead of the week we had budgeted. Most of the saving was not in writing code. It was in not reading 3,000 lines that had nothing to do with the change.


Join the group if you have a similar write-up to send in.