This is just a story about a really weird bit of code. It was the type of code you look at and say, “that’s obviously wrong” and delete straight away.
One of the requirements was to rebuild an Oracle form, a data-entry form for receipts (cash, cheque, credit card, etc). Some of the cashiers use these cheque readers to read the numbers off the MICR band, and with the first release of the new form they reported a defect – “the cheque readers don’t work”.
I didn’t have access to a cheque reader when I was developing the form, but assumed that it would be a normal cheque reader – they put the cheque in, it reads it, it sends a series of digits to the computer. The form should work no different than if the operator keyed in the numbers manually (which they have to do, anyway, if the cheque reader doesn’t work for a particular cheque).
So to investigate the defect I requisitioned a cheque reader, along with some test cheques; after some difficulty (turns out these things don’t work alongside my USB keyboard, I had to get a PS2 keyboard), it was working.
It didn’t take long to discover that the cheque reader was sending the cheque number and BSB in the wrong order, as far as the form is concerned; thus why the validation was failing.
I opened up the old form again, and had a good hard look at the fields. Turns out, what I missed originally is that there is a custom KEY-NEXT-ITEM trigger on the bank code field (which is where the operator puts the focus before using the cheque reader). It looks something like this:
DECLARE
v_data VARCHAR2(50) := REPLACE(REPLACE(:rct.bak_code
,' ','')
,CHR(9),'');
BEGIN
IF LENGTH(v_data) > 4 THEN
IF LENGTH(v_data) < 14 THEN
NULL;
ELSE
:rct.cheque_no := SUBSTR(v_data,1,6);
:rct.bak_code := SUBSTR(v_data,7,3);
:rct.branch := SUBSTR(v_data,10,3);
go_field('RCT.CHEQUE_TYPE');
END IF;
ELSE
go_field('RCT.BRANCH');
END IF;
END;
It turns out that:
(a) the REPLACE(REPLACE( code to remove spaces and tab characters (CHR(9)) is redundant, since the cheque reader never sends spaces, and when it sends a TAB, Oracle Forms doesn’t put a CHR(9) into the field anyway; instead it fires the KEY-NEXT-ITEM trigger
(b) if the length of the data is between 5 and 13, the KEY-NEXT-ITEM trigger does nothing; which means the focus stays in the bak_code field
It’s (b) that is the reason this worked. The trigger fires three times when the cheque reader is used; the third time the trigger fires, it’s got enough digits lumped together in the bak_code field, which it then splits apart, and moves the relevant bits to the cheque_no and branch fields.
A normal, sane person, building this form, would have designed the form to accept the cheque number, bank code and branch in the same order that they are read from the cheque reader; that way, no special code is required – the cheque reader just tabs through the fields, filling them in as it goes.
Oh well – it’s too late to do a screen redesign now, so I’ve had to pretty much replicate the same behaviour in the new form; except that my new code is a little bit smarter – it can also read money orders, which I’ve been told will make the cashiers very happy.
There’s no difference between the effects of the following two statements, are there:
INSERT INTO mytable (col1, col2) VALUES ('hello','world');
INSERT INTO mytable (col1, col2) SELECT 'hello', 'world' FROM DUAL;
Well, as it turns out, it is possible for the first statement to succeed where the second statement would fail – in the presence of a suitably crafted Before Insert trigger, the second will raise “ORA-04091 table is mutating, trigger/function may not see it”:
http://oraclequirks.blogspot.com/2010/09/ora-04091-table-stringstring-is.html
An interesting discussion on the PL/SQL Challenge blog here has led to me changing my mind about “the best way” to loop through a sparse PL/SQL associative array.
Normally, if we know that an array has been filled, with no gaps in indices, we would use a simple FOR LOOP:
DECLARE
TYPE t IS TABLE OF NUMBER INDEX BY BINARY_INTEGER;
a t;
BEGIN
SELECT x BULK COLLECT INTO a FROM mytable;
FOR i IN a.FIRST..a.LAST LOOP
-- process a(i)
END LOOP;
END;
If, however, the array may be sparsely filled (i.e. there might be one or more gaps in the sequence), this was “the correct way” to loop through it:
Method A (First/Next)
DECLARE
TYPE t IS TABLE OF NUMBER INDEX BY BINARY_INTEGER;
a t;
i BINARY_INTEGER;
BEGIN
...
i := a.FIRST;
LOOP
EXIT WHEN i IS NULL;
-- process a(i)
i := a.NEXT(i);
END LOOP;
END;
Method A takes advantage of the fact that an associative array in Oracle is implemented internally as a linked list – the fastest way to “skip over” any gaps is to call the NEXT operator on the list for a given index.
Alternatively, one could still just loop through all the indices from the first to the last index; but the problem with this approach is that if an index is not found in the array, it will raise the NO_DATA_FOUND exception. Well, Method B simply catches the exception:
Method B (Handle NDF)
DECLARE
TYPE t IS TABLE OF NUMBER INDEX BY BINARY_INTEGER;
a t;
BEGIN
...
FOR i IN a.FIRST..a.LAST LOOP
BEGIN
-- process a(i)
EXCEPTION
WHEN NO_DATA_FOUND THEN
NULL;
END;
END LOOP;
END;
This code effectively works the same (with one important proviso*) as Method A. The difference, however, is in terms of relative performance. This method is much faster than Method A, if the array is relatively dense. If the array is relatively sparse, Method A is faster.
* It must be remembered that the NO_DATA_FOUND exception may be raised by a number of different statements in a program: if you use code like this, you must make sure that the exception was only raised by the attempt to access a(i), and not by some other code!
A third option is to loop through as in Method B, but call the EXISTS method on the array to check if the index is found, instead of relying on the NO_DATA_FOUND exception.
Method C (EXISTS)
DECLARE
TYPE t IS TABLE OF NUMBER INDEX BY BINARY_INTEGER;
a t;
BEGIN
...
FOR i IN a.FIRST..a.LAST LOOP
IF a.EXISTS(i) THEN
-- process a(i)
END IF;
END LOOP;
END;
The problem with this approach is that it effectively checks the existence of i in the array twice: once for the EXISTS check, and if found, again when actually referencing a(i). For a large array which is densely populated, depending on what processing is being done inside the loop, this could have a measurable impact on performance.
Bottom line: there is no “one right way” to loop through a sparse associative array. But there are some rules-of-thumb about performance we can take away:
- When the array is likely often very sparsely populated with a large index range, use Method A (First/Next).
- When the array is likely often very densely populated with a large number of elements, use Method B (Handle NDF). But watch how you catch the NO_DATA_FOUND exception!
- If you’re not sure, I’d tend towards Method A (First/Next) until performance problems are actually evident.
You probably noticed that I haven’t backed up any of these claims about performance with actual tests or results. You will find some in the comments to the afore-mentioned PL/SQL Challenge blog post; but I encourage you to log into a sandpit Oracle environment and test it yourself.