Hi
I'm refactoring an old procedure that calls a function for determining whether passed in values consist of only characters allowed in the front end app on top of the database.
The procedure has a cursor that gathers all records it needs to check, loops through each cursor record, runs the function for each value, and if an illegal character is found in a value then it inserts that value, the column it came from, and its ID into a logging table.
But since the cursor pulls multiple columns, the call to the function and subsequent insert are written out over and over for each column in the cursor row (in my simplified example below, I narrowed it down to 3 times/columns, but there are many more).
In the interest of D.R.Y., I'd love to find a way to only have one function call and insert in there, and, but am getting stuck finding away to "cycle through the columns". Briefly thought about making the cursor a bunch of union alls to make it return only "ID", "Value", and "Field" columns and looping through those instead, but there must be better ways.
Any suggestions on best approaches to solving the problem?
(procedure below, full test case in LiveSQL link)
create or replace procedure p_log_invalid_chars
is
cursor c_people
is
select id,
NVL(fname, 'Q') FNAME,
NVL(lname, 'Q') LNAME,
NVL(addr, 'Q') ADDR
from people;
v_id pls_integer;
v_fname varchar2(60);
v_lname varchar2(60);
v_addr varchar2(120);
BEGIN
FOR rec IN c_people LOOP
v_id := rec.id;
v_fname := rec.FNAME;
v_lname := rec.LNAME;
v_addr := rec.ADDR;
/* Here we go with the repeated function calls and inserts */
if not f_valid_chars(v_fname) then
insert into invalid_values(message) values
('invalid chars in record: '||v_id||': fname: '||v_fname);
end if;
if not f_valid_chars(v_lname) then
insert into invalid_values(message) values
('invalid chars in record: '||v_id||': lname: '||v_lname);
end if;
if not f_valid_chars(v_addr) then
insert into invalid_values(message) values
('invalid chars in record: '||v_id||': addr: '||v_addr);
end if;
END LOOP;
END p_log_invalid_chars;
/Thanks in advance,
Phil
OK, I'm going to suggest a fairly significant refactoring here :-)
We're going to:
- drop the function
- drop the cursor
- drop the procedure
But I think you'll be pleased with the result.
1) I can pivot the data to get every column as the same row
SQL> select * from people
2 unpivot ( txt for tag in (fname as 'FNAME',lname as 'LNAME', addr as 'ADDR') )
3 /
ID TAG TXT
---------- ------------------------------ ------------------------------
1 FNAME Mary
1 LNAME Marison
1 ADDR 5 Smith St
2 FNAME Jöhn
2 LNAME Johnson
2 ADDR 23 Court St
3 FNAME James
3 LNAME Jameson
3 ADDR 78 23rd St
4 FNAME Irene
4 LNAME Irenberg
4 ADDR 74 Douglass St
5 FNAME Gary
5 LNAME Garriful
5 ADDR 23 Buènker Ave
2) Plucking out invalid strings based on ascii ranges can be done with a regex
SQL> with data as
2 (
3 select * from people
4 unpivot ( txt for tag in (fname as 'FNAME',lname as 'LNAME', addr as 'ADDR') )
5 )
6 select 'id:'||id||' '||tag||' is invalid: '||txt
7 from data
8 where regexp_replace(txt,'[A-Za-z0-9\,\-\.\/\ \#\''\(\)]') is not null;
'ID:'||ID||''||TAG||'ISINVALID:'||TXT
---------------------------------------------------------------------------------------
id:2 FNAME is invalid: Jöhn
id:5 ADDR is invalid: 23 Buènker Ave
So your routine is now just an insert statement
SQL> insert into invalid_values(message)
2 with data as
3 (
4 select * from people
5 unpivot ( txt for tag in (fname as 'FNAME',lname as 'LNAME', addr as 'ADDR') )
6 )
7 select 'id:'||id||' '||tag||' is invalid: '||txt
8 from data
9 where regexp_replace(txt,'[A-Za-z0-9\,\-\.\/\ \#\''\(\)]') is not null;
2 rows created.
and we're done! It will spank the existing one for performance as well