Room for improvement from where I stand:
- One does not read a table to find out there's nothing to read.
Instead, define what is most common: adding new, or updating stories.
If the latter, *assume* you are going to update, catch the exception,
and add in the exception. The exception would be a no-data-found.
If the first, *assume* you're inserting, catch the exception (dup-value
on-index) and update. This makes a change in your model necessary,
you'll need a unique index, other than you surrogate key (sequence).
- Surrogate keys are populated by triggers. What if someone inserts
without using your procedure?
- You ask the correct question, but use the wrong code; "select
count(*)" is the answer to 'how many stories do I have', not the
code to go along with 'do I have any story', or 'do I have This
story'. "select 1 into lcount where exists..." would be more
efficient, and closer to the question involved. Alternatively,
consider a cursor, open it, and read. If cursor%notfound, there
is no such story. If cursor%found, there is.
In modern versions of Oracle, count(*), *combined with rownum=1*
is as efficient as an old fashioned cursor. Your code inefficiency
is resolved by the 10G R2 optimizer.
- No need for an execute immediate at all. What's wrong with
update story set STO_HEADLINE = pSTO_HEADLINE,
STO_BYLINE = pSTO_BYLINE,
STO_AUTHOR=... etc, etc?
You pass the quotes already... If, in some stage, you need to
quote the quotes (and you will - that's the way Oracle works!),
try the replace function.
If you insist on execute immediate, where is the 'using' clause,
you referred to? Also, execute immediate can return values, like
the value of your sequence - saves a trip to the db, and is thus
more efficient and scales better.
Context switching (pl/sql -> sql -> pl/sql) takes time, and does
not scale well - a reason to avoid exec imm!
5) Reraising exceptions is good practice. No one will be notified
if you have concurrent inserts, unless you read the debug log.
A trigger with sequence prevents this situation by the way.
Nevertheless - raise the error again, same for your no-data-found
At least you do reraise in your catch all (when others) - good point!
--
Regards,
Frank van Bortel
Top-posting is one way to shut me up...
Received on Sun May 14 2006 - 09:47:48 CDT