10 comments:

Let's see:1. p_source and p_result could be %TYPEd on the underlying table columns.2. p_primarykey should use some other prefix than p_ because it's not a parameter.3. WTF select on sequence from dual? Move it to the insert statement.4. WTF insert with append and nologging on a log table? So if the database crashes just after logging you've lost what you logged anyway. Besides for one record it's not going to yield the expected massive performance improvements :-)5. WTF ignore all exceptions?6. why is there a SUBSTR on p_result but not on p_source?7. timestamp can be DEFAULT SYSDATE.8. wherewasi, processed_count being filled with NULL: can drop these columns if they're never filled, or fill them; if they really are optional why include them in the insert statement? In general I use DEFAULTs and don't specify these columns and optional columns when inserting.9. why not fill process_id with NULL if it's not specified, or is this some business logic that this is process "1"?10. drop the surrogate key.

I got three WTF's... could be four if the use of APPEND and nologging are separate items.

Yes I was counting APPEND and NOLOGGING as two separate WTFs, but not quite for the reason you mention. There is actually nothing in the INSERT statement to prevent logging. The developer seems to have misunderstood INSERT /*+ APPEND */ and the NOLOGGING attribute, and the database will just see a regular INSERT with a meaningless hint and table alias, which it will ignore.

I agree about using "p_" for things that aren't parameters though. I wonder whether the procedure once had an OUT parameter, and this was why it had the separate step to derive errorlogging_seq.NEXTVAL (they didn't know about RETURNING either).

When a lot of log entries are created within one second, the timestamp is not enough to tell the order of creation. And it is easier to tell a coworker "please have a look at log entries 124817..124922, looks odd" if you have such a key.