Dec
13
Written by:
StevenFeuersteinTW
Tuesday, December 13, 2011 8:55 AM
This is something I say in my best practice trainings:
Consider every VARCHAR2(N) declaration to be a bug – unless it's a SPOD.
What, you might ask, is a SPOD? It's the single point of definition - the one place in your code where the thing you are working with (datatype, formula, magic value, etc.) is defined or, to put it another way, hard-coded.
You don't want to repeat magic values in your application and you don't want to repeat formulas...but all too many of us are happy to write code like this:
PROCEDURE process_employee (department_id IN NUMBER)
IS
l_id employees.employee_id%TYPE;
l_fullname VARCHAR2 (100);
CURSOR emps_in_dept_cur
IS
SELECT employee_id, last_name || ',' || first_name lname
FROM employees
WHERE department_id = department_id;
Notice in particular that VARCHAR2(100) declaration. Now, sure, I know that I should use %TYPE to anchor to a database column, as I did with l_id. But I cannot; the "full name" is derived from the first and last names. So what should I do?
Well, what I will most likely do is be lazy: look up the structure of the employees table, note that the maximum length of first name is 20, last name is 25, so that's 46 with the comma. Hey, I'll double that, round up to 100 - the full name will never get that big!
Yeah, well, unless the DBA expands the maximum length of those columns.
Seems fairly hypothetical, right? Sadly, I ran into just such a scenario yesterday in the Application Express application for the PL/SQL Challenge (www.plsqlchallenge.com).
A user complained of the classic " ORA-06502: PL/SQL: numeric or value error: character string buffer too small" error when trying to view a quiz from April 20, 2010 - way back at the beginning of the PL/SQL Challenge. So immediately I wondered how things had changed since then, since players are not reporting similar errors for newer quizzes. Ah, the bad assumptions we make!
Well, fortunately, I could reproduce the error without difficulty. And APEX 4.1 gives us a lot of error information so I saw this:
So I opened up the page in Application Express and found this code in the Region Source:
DECLARE
CV SYS_REFCURSOR;
r qdb_news%ROWTYPE;
l_after_first BOOLEAN := FALSE;
BEGIN
CV :=
qdb_competition_mgr.news_for_quiz (
quiz_id_in => :p651_quiz_id);
LOOP
FETCH CV INTO r;
EXIT WHEN CV%NOTFOUND;
HTP.bold ('<h4>' || r.title || '</h4>');
HTP.p (r.full_text);
IF r.discussion_url IS NOT NULL
THEN
HTP.p (
'Click <a href="'
|| r.discussion_url
|| '" onClick="window.open('''
|| r.discussion_url
|| '''); return false; var x__ = parseInt'
|| '">here</a> for more information and discussion on this point.');
HTP.p ('<br/>');
END IF;
END LOOP;
CLOSE CV;
END;
I couldn't see any reason in my code to cause a VALUE_ERROR exception - but maybe there was a problem passing a large string to the HTP subprograms? So I pasted it into Toad and ran it - and got a VALUE_ERROR exception. I added some trace statement, ran it again - and again - but no longer got the VALUE_ERROR exception. I did notice, however, that the full_text string was about 1200 characters long.
Very frustrating! I still do not know why I got such an error the first time I ran it. A mystery without a resolution. Ever have any of those?
Eventually, I gave up on this code - it really didn't seem to have a problem. And I looked further down on the page, to the Condition code. And what do I find there, but the following:
DECLARE
l_news_title VARCHAR2(1000);
l_news_text VARCHAR2(1000);
BEGIN
qdb_competition_mgr.get_news_for_quiz (
quiz_id_in =>:p651_quiz_id
, news_title_out => l_news_title
, news_text_out => l_news_text);
IF l_news_title ISNOTNULL
AND:p651_hide_answers ='show'
THEN
RETURNTRUE;
ELSE
RETURNFALSE;
ENDIF;
END;
Oh my. Someone (can't imagine who J No seriously I don't think it was me!) took a "short cut" and decided that the title and text of the news item couldn't ever possibly exceed 1000 characters.
Oh, no, of course not.
But for this particular quiz, way back on April 20, 2010, it did.
Well, the fix was then quite straightforward. I changed the declaration section to:
DECLARE
l_news_title qdb_news.title%TYPE;
l_news_text qdb_news.full_text%TYPE;
and the problem disappeared.
An excellent reminder to me, and now to all my readers, that you should:
Consider every VARCHAR2(N) declaration to be a bug – unless it's a SPOD.
You are probably wondering, like me, why the error wasn't reported sooner - like on April 21, when players would be reading the news. The only explanation I have for that is this problematic code wasn't introduced until much later, as we revised the way we displayed the news.
Like I say: the mysteries of programming!
3 comment(s) so far...
Re: Every VARCHAR2(N) Declaration a Bug!
Steven - when I started reading this article I thought you were going to give a solution to typing to a derived field ..."Notice in particular that VARCHAR2(100) declaration. Now, sure, I know that I should use %TYPE to anchor to a database column, as I did with l_id. But I cannot; the "full name" is derived from the first and last names. So what should I do?"
This article didn't answer this question at all. Is there a way to declare a %TYPE for a devired field? One that will ebb-and-flow with changes in the database? Or is a static VARCHAR2 declaration the only solution?
By sacgal on
Wednesday, December 14, 2011 12:41 PM
|
Re: Every VARCHAR2(N) Declaration a Bug!
Beware of cross-site scripting in this code;
HTP.bold ('' || r.title || '');
HTP.p (r.full_text);
both these DB variables need wrapping in htf.escape_sc().
r.discussion_url needs to be escaped for the href and must never be coerced into containing a single quote.
Is the code open source at all?
www.recx.co.uk
By NathanCatlow on
Wednesday, December 14, 2011 12:41 PM
|
Re: Every VARCHAR2(N) Declaration a Bug!
Using %TYPE with a Derived Field
There is no way to do that. You could, however, create a view and then %type from that column - or in 11g you could create a virtual column and %type from that.
Escape_SC
Thanks for the advice! No, this is not open source code. And those values are controlled completely by admins on the site. That is, "normal" users do not enter data into the news table.
By StevenFeuersteinTW on
Wednesday, December 14, 2011 12:44 PM
|