“This Procedure Never Raises Exceptions”

It’s a really bad thing to do, but that’s ok because “we put comments in that say it’s bad”.

PROCEDURE insert_stats IS
  PRAGMA AUTONOMOUS_TRANSACTION
BEGIN
  INSERT INTO stats_table ...
  COMMIT;
EXCEPTION
  WHEN OTHERS THEN
    NULL; --yeah this is a bad thing to do, bla bla bla...
END insert_stats;

The idea is that we want to gather some stats about user behaviour, but we are not allowed to interrupt the user’s important work with any unexpected error that might be raised due to the gathering of those stats.

This post is not about why that’s a bad thing – others have made very good points about this practice elsewhere.

What I want to write about is the exception handler here – does it really protect the caller from exceptions raised by this procedure? The answer is, no. Why?

SQL> CREATE PROCEDURE test_handler AS
  PRAGMA AUTONOMOUS_TRANSACTION;
  n NUMBER;
BEGIN
  INSERT INTO stats_table (id) VALUES (0);
  dbms_output.put_line('inserted=' || SQL%ROWCOUNT);
  n := 1 / 0; -- fail...
  COMMIT;
EXCEPTION
  WHEN OTHERS THEN
    dbms_output.put_line('handled: ' || SUBSTR(SQLERRM,1,4000));
    --"silly thing to do but at least we're safe, right?"
END test_handler;
/

Procedure created.

SQL> BEGIN test_handler; END;
/
inserted=1
handled: ORA-01476: divisor is equal to zero
BEGIN test_handler; END;
*
ERROR at line 1:
ORA-06519: active autonomous transaction detected and rolled back

The divide-by-zero is just there to simulate an exception that is raised after the insert succeeds, but before (or during) the COMMIT. As soon as the transaction is started, the procedure must raise ORA-06519 unless a COMMIT or ROLLBACK succeeds. Lesson to learn? An autonomous transaction will raise ORA-06519 to the caller, and it will not be caught by “WHEN OTHERS”. To get around this we could put a ROLLBACK in the exception handler.


TOO_MANY_ROWS side effect

I used to assume that whenever a TOO_MANY_ROWS exception is raised, the target bind variables would be left untouched. Until today I’ve never written any code that relies on the bind variables being in any particular state when a TMR exception is raised, so was surprised.

For example, given the code below, I would expect the dbms_output to indicate that v is null:

CREATE PROCEDURE proc (v OUT NUMBER) IS
BEGIN
   SELECT 1 INTO v FROM all_objects;
EXCEPTION
   WHEN TOO_MANY_ROWS THEN
      dbms_output.put_line
         ('TOO MANY ROWS: v='
          || v);
END
/
 

DECLARE
   v NUMBER;
BEGIN
   proc(v);
   dbms_output.put_line('AFTER: v=' || v);
END
/

TOO MANY ROWS: v=1
AFTER: v=1

What appears to happen is that the out bind variables will be assigned values from the first row returned from the query; then when a second row is found, the TOO_MANY_ROWS exception is raised.

According to the documentation (emphasis added):

“By default, a SELECT INTO statement must return only one row. Otherwise, PL/SQL raises the predefined exception TOO_MANY_ROWS and the values of the variables in the INTO clause are undefined. Make sure your WHERE clause is specific enough to only match one row.”

(Oracle Database PL/SQL User’s Guide and Reference (10gR2): SELECT INTO Statement)

So it appears my original stance (don’t assume anything about the variables’ state after TOO_MANY_ROWS is raised) was correct. Lesson learned: beware of performing a SELECT INTO directly on the OUT parameters of your procedure!


Un-riching Rich Text Format

Let’s just call it Legacy because I’m not going to say what the source is. It is a single-user desktop application that after a little investigation (i.e. searching the online forum for the app) was found to be storing its data in tables readable by MS Access. I wanted to get at this data, analyze it, maybe do some smart things to it, and then present it via Apex.

Step 1: Get the data into Oracle.

Simple matter of exporting from MS Access via ODBC. At least, it was simple once I replaced the Oracle ODBC drivers with the latest download from OTN. Before that I was getting a number of annoying TNS errors.

Step 2: Transform the data.

Most of the tables are easy-to-understand normalized relational tables. One of them, however, has a column that came through as a CLOB containing strange values like this:

{\rtf1\ansi\deff0\deftab254
{\fonttbl{\f0\fnil\fcharset0 Arial;}
{\f1\fnil\fcharset0 Verdana;}}{\colortbl\red0\green0\blue0;\red255\green0\blue0;\red0\green128\blue0;\red0\green0\blue255;\red255\green255\blue0;\red255\green0\blue255;\red128\green0\blue128;\red128\green0\blue0;\red0\green255\blue0;\red0\green255\blue255;\red0\green128\blue128;\red0\green0\blue128;\red255\green255\blue255;\red192\green192\blue192;\red128\green128\blue128;\red255\green255\blue255;}
\paperw12240\paperh15840\margl1880\margr1880\margt1440\margb1440
{\*\pnseclvl1\pnucrm\pnstart1\pnhang\pnindent720
{\pntxtb}{\pntxta{.}}}
...

Now based on my knowledge of the application I knew that this column was used to store small pieces of text (typically 8 to 30 short lines), with some amount of formatting (e.g. fonts, alignment, etc.). Again the online forum came in useful in that a side comment from one of the developers (regarding a small bug undocumented anti-feature) revealed that they stored the formatted text as RTF – Rich Text Format. Should have known from the opening 6 bytes in the data.

Somewhere in these oceans of rtf codes were swimming the plain text I craved. So Googled RTF, skimmed this old RTF specification, and ended up with this admittedly poorly-performing PL/SQL, which for the 651 rows in this table, each with an RTF of average 3KB, works just well enough for my purposes. As it turned out the only RTF codes I was interested in were \fcharset and \*, both of which I used to ignore bits of text I didn’t want in my output. Oh and \par, which denotes the end of a paragraph. I can run this script once a month on the freshly exported data and apply the full weight of Oracle’s analytic capabilities on it.

The code below exemplifies the use of a pipelined function. This is not a good idea, by the way, if you want to use it in regular queries, e.g. a view. In my case, however, I only wanted to call this from within PL/SQL, and then only once a month. Because of the way parameters work with functions like this, I had to call it with dynamic SQL (execute immediate).

That was kind of fun, but I’d rather not have to deal with RTF ever again, thank you.

create or replace package myutil_rtf is
  type t_v4000_table is table of varchar2(4000);
  function extract_text (p_recid in number)
  return t_v4000_table pipelined deterministic;
end;
/

create or replace package body myutil_rtf is
  function extract_text (p_recid in number)
    return t_v4000_table pipelined deterministic is

    l_ch varchar2(1);
    l_ctrl varchar2(4000);
    l_line varchar2(4000);
    l_rtf clob;

    --don't output any text between
    --\fcharset and closing }
    l_fcharset boolean := false;

    --increments for each enclosed pair of { }
    --within a discard section
    l_discard number;

  begin

    select rtf_clob into l_rtf
    from rtf_table where recid = p_recid;

    for i in 1..dbms_lob.getlength(l_rtf) loop

      l_ch := substr(l_rtf,i,1);

      if l_ch = '}' then

        if l_fcharset then
          --closing } found; re-enable output
          l_fcharset := false;
          l_line := null;
        end if;

        if l_discard > 0 then
          l_discard := l_discard - 1;
          if l_discard = 0 then
            l_discard := null;
          end if;
        end if;

      elsif l_ch = '{' then

        if l_discard is not null then
          l_discard := l_discard + 1;
        end if;

      elsif l_ch = '\' then

        --controls start with a backslash
        l_ctrl := '\';

      elsif l_ctrl is not null then

        --controls are always ended by some
        --non-alphanumeric character
        if instr('abcdefghijklmnopqrstuvwxyz'
        || '0123456789',lower(l_ch)) > 0 then
          l_ctrl := l_ctrl || lower(l_ch);
        else
          if l_ctrl = '\par' then
            pipe row (l_line);
            l_line := null;
          elsif substr(l_ctrl,1,9) = '\fcharset' then
            l_fcharset := true;
          elsif l_ctrl || l_ch = '\*' then
            --{\* ... } means you can ignore
            --anything between the { }
            if l_discard is null then
              l_discard := 1;
            end if;
          end if;
          l_ctrl := null;
        end if;

      elsif l_ch not in (chr(10), chr(13), '{')
        and not l_fcharset and l_discard is null then

        l_line := l_line || l_ch;

      end if;

    end loop;

    if l_line is not null and not l_fcharset then
      pipe row (l_line);
    end if;

    return;
  end extract_text;
end myutil_rtf;
/

To extract the text from the table with recid=1:

select column_value line_of_text
      ,rownum line_number
from table(myutil_rtf.extract_text(1));

ORA-06502: PL/SQL: numeric or value error: Bulk bind: Error in define

I came across an inexplicable error when bulk collecting into a PL/SQL table with the NOT NULL constraint the other day. What was confusing was that the code had been passing tests for quite some time.

In the end the only thing that had changed was that a VARCHAR2 which should have been non-null happened to be NULL for one particular row in the table.

Thanks to Connor for the simple test case, listed below.

If you know what might be the cause or reason behind this error, and why it doesn’t occur for dates, I’d be interested.

This was reproduced on Oracle 10.2.0.1.0.

SQL> declare
      type t is table of number not null index by pls_integer;
      r t;
     begin
      select case when rownum < 20 then rownum else null end
      bulk collect into r from all_Objects
      where rownum <= 20;
     end;
     /
declare
*
ERROR at line 1:
ORA-06502: PL/SQL: numeric or value error: Bulk bind: Error in define
ORA-06512: at line 5

SQL> declare
      type t is table of varchar2(80) not null index by pls_integer;
      r t;
     begin
      select case when rownum < 20 then rownum else null end
      bulk collect into r from all_Objects
      where rownum <= 20;
     end;
     /
declare
*
ERROR at line 1:
ORA-06502: PL/SQL: numeric or value error: Bulk bind: Error in define
ORA-06512: at line 5

SQL> declare
      type t is table of date not null index by pls_integer;
      r t;
     begin
      select case when rownum < 20 then sysdate else null end
      bulk collect into r from all_Objects
      where rownum <= 20;
     end;
     /
PL/SQL procedure successfully completed.

Table Types Supplied by Oracle

This is a list of all the table types I’ve found in Oracle-supplied packages, e.g. OWA_UTIL in 10g supplies the type:
TYPE datetype IS TABLE OF varchar2(10) INDEX BY binary_integer;

I find them handy for quick one-off scripts.

Oracle_supplied_table_types


PL/SQL Maintenance Nightmares (Learning to avoid…)

I’ve worked on some new PL/SQL packages and Forms which work quite well. They are efficient in their use of resources, and are easy for me to debug and maintain. After some other developers have had to do some changes (some from changes to requirements, some from bugs I didn’t find), I’ve learned that the highly modular style of code I use is difficult to modify without introducing new bugs.

The modular style is great while developing; it reduces duplication and provides useful abstractions. When it comes to maintaining that code, however, a new developer has to read all that code and follow its tortuous logic around in order to debug it. It’s much easier to just patch the existing code to force it to work.

This is obviously not the best outcome; more new code is added, instead of the existing code being fixed. The module thus becomes more complex and unmaintainable. After some time, a new developer confronts a hodge-podge of duplicated functionality and cut-and-paste monstrosities, and exclaims, “Who wrote this rubbish?” – perhaps with justification.

What I’ve learned is that after developing a new module, I must go back and refactor it to improve maintainability. This includes a number of steps (this list is not intended to be exhaustive):

  1. Delete simple wrappers for library functions/procedures, and replace all calls to them with calls to the original library functions/procedures.
  2. Don’t use constants for internal codes (e.g. names of items and blocks) – use literals instead.
  3. If a fairly simple function or procedure is only called once throughout the final code – delete it and move the code to where it was called from; the exception is if this would make the calling code more difficult to read or understand.
  4. Code that generates dynamic SQL should set the SQL in entirety in single statements if possible, instead of building it procedurally. Another developer should be able to easily find the exact SQL that will be generated just by looking at the code, instead of having to mentally build it procedurally, or having to printline the SQL and run it to see what it generates.
  5. Add comments that document the philosophy of how the code has been laid out.

Some examples to illustrate the above:

  1. We have a library procedure called lk_item.lp_show_item (pc_item, pn_visible, pn_enabled) which is used to show, hide, enable, and/or disable a form item. I wanted to write code like the following: lk_item.lp_show_item(‘my_item’, some_boolean_expression, another_boolean_expression) but I couldn’t because the parameters require PROPERTY_TRUE or PROPERTY_FALSE, which are numbers. My first cut included the following wrapper for lk_item.lp_show_item:
    PROCEDURE cp_show_hide
    (pc_item IN VARCHAR2
    ,pb_visible IN BOOLEAN
    ,pb_enabled IN BOOLEAN) IS
    FUNCTION CF_PROPERTY_TF
    (pb_value IN BOOLEAN) RETURN NUMBER IS
    BEGIN
    IF pb_value THEN
    RETURN PROPERTY_TRUE;
    ELSE
    RETURN PROPERTY_FALSE;
    END IF;
    END;
    BEGIN
    lk_item.lp_show_item(pc_item,
    CF_PROPERTY_TF(pb_visible),
    CF_PROPERTY_TF(pb_enabled));
    END;

    In the end, however, I only called this procedure two times, and within a single procedure. So I deleted the wrapper procedure, and called lk_item.lp_show_item directly. I kept my Boolean expressions, but wrapped them in a locally declared function that did the translation.
    FUNCTION CF_PROPERTY_TF
    (pb_value IN BOOLEAN) RETURN NUMBER IS
    BEGIN
    IF pb_value THEN
    RETURN PROPERTY_TRUE;
    ELSE
    RETURN PROPERTY_FALSE;
    END IF;
    END;

    lk_item.lp_show_item(‘ITEM_NAME’,
    CF_PROPERTY_TF(some expression),
    CF_PROPERTY_TF(some other expression));

  2. I had a procedure that populated a large number of items in the block in response to the user selecting a new record. Because the user can select a new record using any of several different methods (effectively, different search criteria), I used a parameter called “search mode” that is set to various constant strings, e.g. “GNL_ID”, “OFF_ID_DETAILS”, “COLLAR_NO”. Some of these strings were named after significant database table columns, some were generic – they all, however, simply differentiated slightly different conditions under which the procedure must operate.

    Initially I had a set of constants that took these values; however, because this code passed these values to a database package as well, the constants were defined in the database package as well. In the end the code was much simpler and easier to read by removing all the constants and just encoding the codes as literal strings.

  3. (no example)

  4. A first cut of the code might look like (this is a very simplified example):
    cp_lexical := ‘SELECT aaa, bee, cee ‘;
    IF (some expression) THEN
    cp_lexical := cp_lexical || ‘, dee FROM bla ‘;
    ELSE
    cp_lexical := cp_lexical || ‘FROM dee, eff ‘;
    END IF;
    IF (some other expression) THEN
    cp_lexical := cp_lexical
    || ‘, gee WHERE eee = eff AND gee = oh ‘;
    ELSE
    cp_lexical := cp_lexical
    || ‘, hat WHERE eee = oh AND ii = jay ‘;
    END IF;
    cp_lexical := cp_lexical
    || ‘ AND kay = ell AND emm = enn’;

    The final, more easily maintained code would look something like:
    IF (some expression)
    AND (some other expression) THEN
    cp_lexical := ‘SELECT aaa, bee, cee, dee ‘
    || ‘FROM bla, gee ‘
    || ‘WHERE eee = eff ‘
    || ‘AND gee = oh ‘
    || ‘AND kay = ell ‘
    || ‘AND emm = enn’;
    ELSIF (some expression) THEN
    cp_lexical := ‘SELECT aaa, bee, cee, dee ‘
    || ‘FROM bla, hat ‘
    || ‘WHERE eee = oh ‘
    || ‘AND ii = jay ‘
    || ‘AND kay = ell ‘
    || ‘AND emm = enn’;
    ELSIF (some other expression) THEN
    cp_lexical := ‘SELECT aaa, bee, cee ‘
    || ‘FROM dee, eff, gee ‘
    || ‘WHERE eee = eff ‘
    || ‘AND gee = oh ‘
    || ‘AND kay = ell ‘
    || ‘AND emm = enn’;
    ELSE
    cp_lexical := ‘SELECT aaa, bee, cee ‘
    || ‘FROM dee, eff, hat ‘
    || ‘WHERE eee = oh ‘
    || ‘AND ii = jay ‘
    || ‘AND kay = ell ‘
    || ‘AND emm = enn’;
    END IF;

    This doesn’t solve all the maintenance problems (i.e. if someone makes a change to one part of the code they might not know that they should make the same change everywhere else that code has been duplicated). However, it does make maintenance a bit easier because the code is easy to follow.

  5. I generally follow the following pattern when encapsulating logic in procedures and packages:
    • Code that populates data in items (e.g. in response to changes in other items) is encapsulated in procedures or packages named POPULATE_xxx.
    • Code that changes item properties (but not their values) is encapsulated in procedures or packages named SETUP_xxx.
    • Code that checks data entry errors is encapsulated in procedures or packages named VALIDATE_xxx.

    These conventions are documented so that another developer can more easily read the code; for example, a when-validate-item trigger might have code like this:
    cp_validate_something;
    cp_populate_something;
    cp_setup_something;

    The developer might still need to follow through all the code to debug it, but they might more easily see where the code should be. Hopefully they’ll recognise that validation code should go in cp_validate_something, rather than cp_populate_something.


Fun with copy-and-paste code

Came across this in a form (6i) to be run on a 9i db. Not only is this code about 33 lines of code too long and issues any number of unnecessary database queries, its name is quite unrelated to its intended function. Needless to say it was easily replaced with a single call to INSTR.

PROCEDURE alpha_check
(ref_in IN VARCHAR2
,ref_out OUT VARCHAR2) IS
-- Procedure included to distinguish
-- ref_in between ID or reference.
  l_alpha_char  VARCHAR2 (1);
  l_alpha_pos   NUMBER;
  l_found_pos   NUMBER;
  l_search_string VARCHAR2 (100) := ' ';
 

  CURSOR cur_get_next_alpha(N NUMBER) IS
  SELECT SUBSTR(l_search_string,N,1)
  FROM dual;

  CURSOR cur_check_for_alpha(C VARCHAR2)IS
  SELECT INSTRB(ref_in,C, 1)
  FROM dual;

BEGIN
  IF ref_in IS NULL THEN
    ref_out := 'X';
    RETURN;
  END IF;

  FOR I IN 1..LENGTH(l_search_string) LOOP
    OPEN cur_get_next_alpha(I);
    FETCH cur_get_next_alpha
    INTO l_alpha_char;
    CLOSE cur_get_next_alpha;

    FOR J IN 1..LENGTH(ref_in) LOOP
      OPEN cur_check_for_alpha(l_alpha_char);
      FETCH cur_check_for_alpha
      INTO l_found_pos;
      CLOSE cur_check_for_alpha;

      IF l_found_pos > 0 THEN
        ref_out := 'N';
        RETURN;
      END IF;
    END LOOP;
  END LOOP;

  ref_out := 'Y';
EXCEPTION
  WHEN OTHERS THEN
    pc_ref_out := 'X';
END;

Looks like it may have been copied from the same source as “As bad as it gets”.


Generating test data that matches existing data

I’ve had to create test data a number of times, including data for tables that had mandatory foreign keys to existing tables. It was not feasible to just create new master rows for my test data; I wanted to refer to a random sample of existing data; but the code that generates the test data had to perform reasonably well, even though it had to pick out some random values from a very large table.

Solution? A combination of the new 10g SAMPLE operator, and DBMS_RANDOM. To illustrate:


(create a “very large table”)
SQL> create table t as
2 select rownum n, dbms_random.string(‘a’,30) v
3 from all_objects;

Table created.

SQL> select count(*) from t;

COUNT(*)
———-
40981

(get a random sample from the table)
SQL> select n, substr(v,1,30) from t sample(0.01)
2 order by dbms_random.value;

N SUBSTR(V,1,30)
———- ——————————
11852 xSsdmFtGqkymbKCFoZwUzNxpJAPwaV
8973 RGyNjqMfVayKdiKFGvLYuAFYUpIbCw
25295 eJJtoieSWtzUTIZXCbOLzmdmWHHPOy
297 hiTxUPYKzWKAjFRYTTfJSSCuOwGGmG
1924 yZucJWgkFviAIeXiSCuNeUuDjClvxt
40646 wMTumPxfBMoAcNtVMptoPchILHTXJa

6 rows selected.

SQL> set serveroutput on

(Get a single value chosen at random)
SQL> declare
2 cursor cur_t is
3 select n from t sample(0.01)
4 order by dbms_random.value;
5 l number;
6 begin
7 open cur_t;
8 fetch cur_t into l;
9 dbms_output.put_line(l);
10 close cur_t;
11* end;
SQL> /
21098

PL/SQL procedure successfully completed.


My test code would open the cursor, fetch as many values as it needed, and then close it. If the cursor ran out of values (e.g. the sample was too small for the desired amount of test data, which varied), my code just re-opened the cursor to fetch another set of random values from the large table.

The reason I sort the sample by dbms_random.value is so that if I only want one value, it is not weighted towards rows found nearer the start of the table.

Note: If I didn’t really care about the sample being picked at random from throughout the table, I could have just selected from the table “where rownum < n".



Is this code actually unreachable?

PL/SQL User’s Guide and Reference (10.2): “4 Using PL/SQL Control Structures – Using the NULL Statement”

“…Note that the use of the NULL statement might raise an unreachable code warning if warnings are enabled.”

Example 4-23 Using NULL as a Placeholder When Creating a Subprogram

CREATE OR REPLACE PROCEDURE award_bonus (emp_id NUMBER, bonus NUMBER) AS
BEGIN — executable part starts here
NULL; — use NULL as placeholder, raises “unreachable code” if warnings enabled
END award_bonus;
/

Indeed, when I compile the above in 10.2 with PL/SQL warnings on, I get PLW-06002 as expected (due to bug 3680132 I get “Message 6002 not found; No message file for product=plsql, facility=PLW” but at least I can look it up in the reference).

“PLW-06002: Unreachable code”
“Cause: Static program analysis determined that some code on the specified line would never be reached during execution.”

I agree that a PL/SQL warning would be desirable in the case where a procedure has nothing but a NULL in it (probably a stub). Correct me if I’m wrong, but if I were to call award_bonus, surely the NULL is “executed” – therefore, it is reachable! A more appropriate warning would be something like “function/procedure does nothing”, or “get back to work you silly mug, you’ve forgotten to finish the code”. Maybe they just couldn’t be bothered making up another warning code.