Close encounter
-
I had this close encounter of the third kind some days ago.
The system that our company supports processes applications for micro-credits (among other things). The client bank requested that if the applicant meets some criteria, some checks in the approving process to be skipped. One of the requirements were the pending exposition (the sum of all requested money in the candidate's applications of last seven days, that was not imporved) to be less than 4000 (some currency).
This particular task was an easy one, or so I thinked. There already were a function, which calculated that same pending exposition. This function was in production for years and was proven correct. So I used it. But when the testing team tested the changes, they rejected them, saying that the pending exposition rule does not work.
I was surprised, so I checked the code. The function, contained in a Oracle package, is written by a fellow teammate, let's name her Alice (which, btw, has a pretty high academic degree), and looks something like that:Function CalcPendingExp(aPersonID in applications.person_id%TYPE) return number isI got the select and ran it, replacing vPersonID with the ID from the test case. It returned a value prety much above the threshold (4000). Then I ran debuger. After execution of the first select, the value of vExp became 0. I was shocked. Then removed nvl. vExp became NULL. Then I ran the select again, replacing vPersonID with the problematic one and replacing nvl and sum with *. The select returned about 10 rows, each with amount > 0. I was shocked again.
vExp number;
begin
select nvl(sum(requested_amount), 0)
into vExp
from applications a
where a.person_id = vPersonID
and a.application_date >= trunc(sysdate, 'DD') - 6
and a.application_status not in (<some status codes>);
-- snip some additional code, which adds to vExp various amounts, stored elsewhere
return vExp;
end CalcPendingExp;
I started to think that maybe the magnetic poles are switching, or the laws of physics had expired, or maybe Oracle database has some very obscure bug, which demonstrates itself in very rare conditions. I spent following 20-30 minutes tweaking the select in the procedure, attempting to achieve something different than 0. Without any success.
Then I incidently noticed, that the argument that I replaced in my tests with a constant value, is called vPersonID, while the argument of the function is called aPersonID. But why this function compiles at all? It turns out, that the value passed to aPersonID is completely ignored and that vPersonID is a global package variable. When I use the function directly it is uninitialized (null), so the select did not find nothing. There is another procedure (GetData), which fills values of all the global package variables (about 30 of them) with the values of a given application.
I have some VERY bitter words to say to Alice, when I see her.
-
@partizanin said:
I have some VERY bitter words to say to Alice, when I see her.
Why would you do that? It's only cost you at most a day or two of work.
-
Yes, if we never count the effort of the eventual rewrite of the entire package (which is flooded with refernces to these global variables), and the front-end code that uses it... (and the testing effort)
-
@partizanin said:
Yes, if we never count the effort of the eventual rewrite of the entire package and rhe front-end code that uses it.
Yeah, but why is that your problem? Sure you put an issue in to get it fixed (and that might eventually come back to you to do) but at least it's found now. Deal with the immediate problem instead of trying to make things perfect in passing.I've seen worse.
-
Oh, I see. I didn't explain the situation in all his glory. The given project is off-focus for the management, because it is working the way it is. The client bank is trying to cut the cost of the support, and all this sh*t is droping directly over my head. So I try to make all that mess working (and I perfectly know that I am doomed), but I think that it is worthy to share all funny (?) stories of that journey with you, people.
-
@partizanin said:
The given project is off-focus for the management, because it is working the way it is. The client bank is trying to cut the cost of the support, and all this sh*t is droping directly over my head.
Oh well, if you're out on a limb there then you might as well just try to minimise what you touch and get back to a more supported area of business. It'll leave a mess for the future, but sometimes that's the best option overall. If you're feeling kind, put comments in the code to say what the horrible nasties are that you've spotted. Maybe it'll at least make someone else who sees them look twice before digging in worse.You've got an issue tracker open? (If not, much bigger WTF!) Put an issue in there, fairly low priority since it is currently working in its original intended use case, to say that there's a problem with excessive technical debt (especially in relation to variable scope management) in the area. For added giggles, assign it to the original author of the code.
-
@partizanin said:
It turns out, that the value passed to aPersonID is completely ignored and that vPersonID is a global package variable.
Oracle has global variables?! Yikes! This thread arguably belongs in the "I Hate Oracle" forum, but thank you for the cautionary tale about why people hate it.
-
@pjt33 said:
Oracle has global variables?!
They prefer to be called "World Citizen Variables", thank you. Now be a dear and toss me that rigging line.
-
@pjt33 said:
@partizanin said:
If you don't know why people hate it, you've never used it, or installed it, or talked to anyone about it.It turns out, that the value passed to aPersonID is completely ignored and that vPersonID is a global package variable.
Oracle has global variables?! Yikes! This thread arguably belongs in the "I Hate Oracle" forum, but thank you for the cautionary tale about why people hate it.
-
Oh Rake Ul. Whose solution to every problem is that you should upgrade to the new version, it will definitely fix that bug that we never knew existed.
A few it-could-only-be-oracle stories:
A 2 billion row partitioned table corrupted because the primary UNIQUE key index started allowing duplicates in a few partitions. -Oracle official solution: Upgrade to the next version! (we were already on 11g, and the next version had barely even just come out). Real solution... create a new table with the key, extract the 2 billion rows and insert/update them back to uniqueness in the new table. Then drop the old table and rename the new one.
One very specific query we had with a group by rollup statement that caused the ever descriptive "Space Leak" server error (not a syntax error), but only if your results were a single row. Group by date with rollup, query a full year, works great. Query for 1 date, server implodes. Plenty of other queries we had were perfectly happy doing a group by with rollup on 1 row, but not this one.
Temp tables. Ugh, don't even want to think about it.
Empty strings = NULL? Screw you Oh Rake Ul. Your garbage requires my code to translate an input of an empty string to some other value just so I can tell it was an effing blank and not actually NULL.
Storing a Midnight Date (or DateTime in any sane database) = Storing a Date part and a NULL Time part... Seriously, how the hell did they come up with this shit? The Time is NOT EFFING NULL. IT IS GODDAMN MIDNIGHT YOU JACKASSES.
