Mixing Positional & Named Params Can Lead to Trouble

A user of the APEX application I built for a local non-profit sent me a screenshot over WhatsApp today saying that she was getting an error trying to save a change to a student record in the system.

I could reproduce the problem, and quickly enabled APEX debug trace to level 9 which gave me a rich amount of information on what was happening.

The trigger on my STUDENTS table contained lines that were conditionally logging audit information for particular column values like the example below and the call stack for the ORA-6502 visible in the APEX debug trace was pointing to the line handling the audit for change in value of the YEAR_OF_BIRTH column.

if (updating) then
    -- etc.
    bambini.handle_audit('STUDENTS','FIRST_NAME',:old.id,
                         p_old_text_value => :old.first_name,
                         p_new_text_value => :new.first_name);
    bambini.handle_audit('STUDENTS','CLASS_YEAR',:old.id,
                         p_old_number_value => :old.class_year,
                         p_new_number_value => :new.class_year);
    bambini.handle_audit('STUDENTS','YEAR_OF_BIRTH',lv_user,:old.id,
                         p_old_number_value => :old.year_of_birth,
                         p_new_number_value => :new.year_of_birth);
    bambini.handle_audit('STUDENTS','DATE_OF_BIRTH',:old.id,
                         p_old_date_value => :old.date_of_birth,
                         p_new_date_value => :new.date_of_birth);
    -- etc.
end if;

Nestled in there among many similar lines of code, my eye wasn’t seeing what could be wrong, but luckily my colleague Patrick spotted an odd disturbance in the repetitive code. What was that extra lv_user variable doing in the argument list? None of the other lines had it! The declaration of the handle_audit() procedure being invoked looked like this:

    PROCEDURE handle_audit(
       p_table_name       VARCHAR2,
       p_column_name      VARCHAR2,
       p_id               NUMBER,
       p_old_text_value   VARCHAR2 := null,
       p_new_text_value   VARCHAR2 := null,
       p_old_date_value   DATE     := null,
       p_new_date_value   DATE     := null,
       p_old_number_value NUMBER   := null,
       p_new_number_value NUMBER   := null    
    );

An accidental copy/paste inserted lv_user at a position in the text that, due to my mixing positional arguments with named arguments, did not cause a compilation error. It resulted in passing the value of lv_user to the numeric p_id parameter and causing PL/SQL to convert the value of the numeric :old.id field to a string as the value of the next p_old_text_value parameter. Since lv_user contained a string with the currently logged-in user name, it was getting an ORA-6502 trying to convert that to a number to pass into p_id.

As a result of this exercise, I learned never to mix positional and named parameter notation when invoking PL/SQL program units, and it reminded me how useful it would be if PL/SQL supported an ANYRECORD datatype so I could have written a single, generic handle_audit() procedure that could accept the :new and :old records from the trigger without stamping out line after line of similar code in all of my table’s triggers where I want value-change auditing. So I also filed an enhancement request in our internal bug database requesting that PL/SQL someday add an ANYRECORD type to make writing generic record-processing code like this less tedious. If that were a thing, my trigger could have been a one-liner:

-- NOTE: Dream code, not currently supported!
if (updating) then
    bambini.handle_audit('STUDENTS',:new,:old);
end if;

Thanks, Patrick, for the eagle-eye code spotting and the lesson learned on PL/SQL best practice.