The Daily WTF: Curious Perversions in Information Technology

2006-09-22 Reply Admin

Anonymous:
I have a crazy question here ... did Sir Robert the Great ever consider that data modifications may require a COMMIT?

Only with real databases. This is probably MySQL. It's such a toy.

You see? Robert was right!

2006-09-22 Reply Admin

Anonymous:
IF EXISTS (SELECT COUNT(s_user_id) from sessions WHERE s_user_id = input_id)

Since there will exists either a count of 0 or a count of 1 the expression is always true.

Anyway, PL/SQL is worse.

select count(*)

into li_count

from dual

where exists ( select * from sessions where session_id = id );

if count(*) = then ...

2006-09-22 Reply Admin

Man, the wear and tear on that database engine has to be nasty. I bet they have to change oil at least once a month not to mention replacing broken tables.

2006-09-22 Reply Admin

Anonymous:
Anonymous:

IF EXISTS (SELECT COUNT(s_user_id) from sessions WHERE s_user_id = input_id)

Since there will exists either a count of 0 or a count of 1 the expression is always true.

Anyway, PL/SQL is worse.
select count(*)
into li_count
from dual
where exists ( select * from sessions where session_id = id );
if count(*) = then ...

are you sure it's not:

select count(*)

into li_count

from sessions

where session_id = id;

;-))

2006-09-22 Reply Admin

The real WTF are :

FIRST : why does he need to check if record is here (select...), you can directly perform the delete.

SECOND and biggest :
Why does he use database stored session ? does he never try $_SESSION[], session_start() and stuff like that. it's made for session management, you can setup time out without any need to garbage collector in case user forgot to "disconnect"... to use a database for that is a waste of time to develop, risk (sql injection), a wast of performance, and an imcomplete user made implementation of session. that's stupid !

devdas · 2006-09-22 Reply Admin

random_garbage:
Alexis de Torquemada:

I don't know what RDBMSes you are using, but most of those that I work with (e.g. PostgreSQL, Firebird SQL) don't even have commands "outside of a transaction".

Postgres doesn't have commands outside of transaction?
It looks to me like it is just fine with commands outside transactions... Did you mean that protocols like JDBC don't allow commands outside transactions?

Uhm, no. There is an implied transaction if you don't explicitly start one. You can change this by setting autocommit to off (It defaults to being on, as per SQL specification).

http://www.postgresql.org/docs/8.1/static/populate.html

http://www.postgresql.org/docs/8.1/static/tutorial-transactions.html

From the second link

PostgreSQL actually treats every SQL statement as being executed within a transaction. If you do not issue a BEGIN command, then each individual statement has an implicit BEGIN and (if successful) COMMIT wrapped around it. A group of statements surrounded by BEGIN and COMMIT is sometimes called a transaction block.

2006-09-22 Reply Admin

Anonymous:
How did the code go from being...
[ED: ... snip ...]
To...
[ED:  ... snip ...] 
WTF is going on WTF?

[ED: The submission was kept mostly intact with the following changes: simplified for ease of understanding, superflous logging code removed, variables renamed, and comments added. The simplification led to an infinte loop, which the original did not contain. I should also note that the comment at the top ("this is stupid") was not added by me. See New Editorial Guidelines]

Err.. what use are WTF code snippets that don't resemble original code anymore? If there wasn't an infinite loop originally, why make one?

2006-09-22 Reply Admin

Anonymous:
themagni:

Doing the same thing over and over again and expecting different results is the definition of insanity.

So, what is it called when the different results finally do transpire?

IT

Howi · 2006-09-22 Reply Admin

Maybe the Exec ends up executing the database command asyncronously, and returns without having blocked on the database execution? The delay before the database callback or thread execution may be enough to warrant his 5000 retries.

Then again he'd have to be an idiot to realise the error if that were the case. Surely he's not an idiot...

2006-09-22 Reply Admin

With PHP and Postgres, if the table is locked, PHP will wait (atleast until it timesout).

2006-09-22 Reply Admin

tiro:
Anonymous:
I love the "E_USER_ERROR " part. How can an inability to delete a record on the database be a "user error"?
During development, Robert didn't know that the other developers and DBA's were playing a very cruel trick on him. They would watch Robert's cubicle, and when they saw him testing the code that would end a session, they would stop the database server. Then Robert would get all angry and wave his arms about and get all flustered. Then they'd restart the server. Robert would just see that his code to end the session would work sometimes, and at other times, it wouldn't work for a while, and then start working again. Robert checked and rechecked his code and saw that there was nothing wrong with it - it was a darn random bug in the darn network/apache/php/postgres/linux stack. As you can see from the code, Robert finally gave up and just decided to repeat the whole code a huge number of times.
And hey, it worked - not because of his code, but because the other developers and DBA's were getting bored with watching Robert, and some of their deadlines where looming.
Wow, if this were true Robert would have had to do something very evil to the other Devs/DBAs.

It also means that Robert was (at least this one time) the reasonable one.

Personally, I feel the correct approach would have been to debug the network/apache/php/postgres/linux stack since this "bug" might affect the operations of other critical queries in the system. Only once the cause was explained or time ran out do we put in a documented hack explaining why we do the seemingly stupid thing. I found a kernel driver error in a SCSI controller that only failed under a certain specific set of conditions. I was able to get a patch made and it is now redistributed by several major software vendors... all from finding a random 'bug' in a single web page.

However, I've worked with other developers who would not have spent a week carefully isolating software and hardware systems running through all possible permutations and documenting their behaviors to determine the bug was not in the SCSI controller but in the driver for the controller itself. Those folks would have just done something like what Robert had done. Does that make them idiots? I don't think so. I think it makes them pragmatic. They simply aren't going to blow-off a deadline, tell the customer go whistle, take over the jobs of three system administrators and DBA's, kick them out of their own server rooms and co-opt all the hardware from three projects to isolate a SCSI driver bug.

Yeah. Stupid.

2006-09-22 Reply Admin

Raven:
My first impression is that the while loop will at most only run twice. In the first iteration it will delete the data, in the second it will figure out that there is nothing to be deleted and set $bDeleted to true then the while loop will then exit.
Right?

That's a good guess, but I think I figured it out...

The user id was "bob; insert into sessions (session_id) values ('bob');"

He started the loop with no matching rows and ended with 10,000 (or is that 9998?)

chriztofr · 2006-09-22 Reply Admin

Anonymous:

One word - Brilliant!!

Hey look... a WTF on a WTF.

2006-09-22 Reply Admin

Anonymous:
Anonymous:
All I know is that I really dig the use of Hungarian Notation; without it I doubt I would have known what the variables were. The comments were also particularly helpful.

I just wonder if Hungarians actually code that way and if so, how did that habbit start in the first place.

if (see(Ceausescu.class)) {
 Gulyásleves gGoulash = new Gulyásleves(extraMeet);
throw gGoulash;
}

Hint: Ceausescu = Romania

2006-09-22 Reply Admin

Wow, I can't believe I didn't find this comment yet. Or maybe Firefox is just a crappy toy with bad search... but:

Don't you know what he's doing? He's following the first rule that we learned... "If at first you don't succeed, try, try again... 5000 times if you need to!

Just don't ever try to learn what went wrong...

Thanny · 2006-09-22 Reply Admin

Anonymous:
Maybe they're using a really weird database where queries fail if a record or table is locked... In that case only I see the point of retrying.
The WTF's that I've found:
• if $user_id is not set or not a number, all queries fail
• where is the initialization of $iTimes? One more loop like that in the code and $iTimes may already be 5000 when he gets to this one...
• why add . "" to the end of the query?
• why call the last if to continue at the end of a loop, doing nothing there will also continue the loop...
• why not set $bDeleted to true if the delete query executes fine?
• why check if the record exists at all? You can always try to delete a record - even if it doesn't exist. "DELETE FROM sessions WHERE s_user_id = 1 AND s_user_id = 2" will even execute.
Coditor

The "" at the end is probably some kind of leftover from when the ID was a string, which would require a closing quote.

He doesn't set $bDeleted to true on successful query execution because he doesn't believe the database actually deleted the row. That's why he has the SELECT query. He wants to verify that it actually was deleted. An earlier comment from the WTF's donator indicated that Robert, by chance, found a session in the database that shortly before had been deleted. Apparently it didn't dawn on him that it was a new session with the same user ID, which prompted this abomination.

Couple notes:

1) This forum software doesn't accept the CUA standard keyboard shortcuts for copy and past (CTRL-INS and SHIFT-INS, respectively). It only recognizes the absurd Windows-specific CTRL-C and CTRL-V combo. Horrible.

2) An atomic delete never needs to be part of a transaction. Either it succeeds or it doesn't. There's nothing to roll back on failure.

2006-09-22 Reply Admin

How do you test that code? like this:

 <font color="#006600">// Try 5000 times to make sure that this stupid thing 
// was deleted. This crap is such a toy.</font>
<font color="#000099">while</font> ( !$bDeleted && $iTimes < 5000 )
{

  echo "die!! < br> \n";

   
  $iTimes++;

  <font color="#006600">//does session exist?</font>
  $sSQL = <font color="#990000">"SELECT * FROM sessions "</font> .
          <font color="#990000">"WHERE s_user_id = "</font> . $user_id . <font color="#990000">""</font>;

2006-09-22 Reply Admin

Maybe they're using a really weird database where queries fail if a record or table is locked... In that case only I see the point of retrying.

I'm not familiar with the mechanics of PHP database objects or exception handling, but shouldn't a table lock cause an exception, telling the coders why the logout is failing?

The WTF's that I've found:
• if $user_id is not set or not a number, all queries fail
• where is the initialization of $iTimes? One more loop like that in the code and $iTimes may already be 5000 when he gets to this one...

We're obviously not being shown the whole function. $user_id may well have been validated (and $iTimes initialized) earlier in the function.

• why add . "" to the end of the query?
• why call the last if to continue at the end of a loop, doing nothing there will also continue the loop...

These confuse me as well. I can only think that they may be for code consistency... If you consistently code SQL statement lines to end in strings, then perhaps you'll be less likely to forget a trailing "'" or ")" or " " after a variable concatenation. It's only a little inefficient.

• why not set $bDeleted to true if the delete query executes fine?
• why check if the record exists at all? You can always try to delete a record - even if it doesn't exist. "DELETE FROM sessions WHERE s_user_id = 1 AND s_user_id = 2" will even execute.

It makes sense only if the delete silently fails for some reason. Assuming that to be the case, he needs to do the select to check to make sure the record is deleted and, if not, repeat the query. It would be marginally more efficient to do the delete and then the check, but it works both ways.

It sounds like this is what happened:

The logout routine is originally coded with a simple, obvious single SQL delete statement.
Users report occasional logout failure, maybe due to a race condition or logout code not getting called correctly.
Coder is given the task of debugging the logout routine consisting of the delete statement, which most likely had nothing wrong with it, but appears to be failing silently because of an error somewhere else.
After failing to diagnose (or maybe even reproduce) the error, the coder assumes the problem is in the database, so he adds an excessive loop and check to make sure delete is really occurring, along with an indignant comment to vent frustration.
Either the error goes away by accidentally adding enough delay to resolve a race condition, or the error continues to occur, but since the logout routine has been "fixed", the programming team assumes the error is elsewhere and eventually resolves the real problem, leaving this superfluous code.
Maintenance programmers, scared by nonsensical code and a confusing comment assume black magic is at work, so the code remains in the program until the end of time.

If this is the case, the real problems are the coder's diagnostic skills, documentation, and attitude.

Alexis de Torquemada · 2006-09-22 Reply Admin

random_garbage:
Postgres doesn't have commands outside of transaction?
xterm:
$ createdb testing
CREATE DATABASE
$ psql testing
It looks to me like it is just fine with commands outside transactions... Did you mean that protocols like JDBC don't allow commands outside transactions?

I expected a reply like that... but devdas was faster to refute it. Anyway, psql != PostgreSQL, and just because you don't see the transaction doesn't mean it isn't there.

I noticed there are some PostgreSQL commands that do not involve transactions, though, for example VACUUM. However, none of standard DML commands work without a transaction.

Anonymous:
Hint: Ceausescu = Romania

Well, Hungary, Romania... that's all outside the US. And everything that's not in the US is kinda like the same anyway, isn't it?

2006-09-22 Reply Admin

Anonymous:
Anonymous:

How would you do it?
SELECT COUNT(s_user_id) from sessions WHERE s_user_id = input_id;
?
Your query requires scanning the whole table to count up the instances of a specific ID. It is probably more efficient to return just the first hit. I know SQL Server uses "TOP" for this, and I guess MySQL does it with "LIMIT":
SELECT s_user_id FROM sessions WHERE s_user_id = input_id LIMIT 1

I prefer using exists for stuff like this

SELECT 1 WHERE EXISTS

(SELECT * FROM sessions WHERE s_user_id = input_id )

2006-09-22 Reply Admin

Alexis de Torquemada:
random_garbage:
Postgres doesn't have commands outside of transaction?
xterm:
$ createdb testing
CREATE DATABASE
$ psql testing
It looks to me like it is just fine with commands outside transactions... Did you mean that protocols like JDBC don't allow commands outside transactions?
I expected a reply like that... but devdas was faster to refute it. Anyway, psql != PostgreSQL, and just because you don't see the transaction doesn't mean it isn't there.

I noticed there are some PostgreSQL commands that do not involve transactions, though, for example VACUUM. However, none of standard DML commands work without a transaction.

Anonymous:
Hint: Ceausescu = Romania
Well, Hungary, Romania... that's all outside the US. And everything that's not in the US is kinda like the same anyway, isn't it?

I've been told taht if you live in New York, everything east of the Hudson River is kinda like the same way.

savar · 2006-09-22 Reply Admin

Anonymous:
Anonymous:
I have to say that "select * from sessions where..." is the most wasteful way to check for record existence though.

How would you do it?
SELECT COUNT(s_user_id) from sessions WHERE s_user_id = input_id;
?

If you want to check for existence of a single row, then you don't need to do a full-table scan.

Oracle:

SELECT s_user_id from sessions WHERE s_user_id = :input_id AND rownum<2;

MySQL:

SELECT s_user_id from sessions WHERE s_user_id = :input_id LIMIT 1;

2006-09-22 Reply Admin

richleick:
Alex Papadimoulis:
...Kristopher hasn't quite figured out what PHP, Postgres, Apache, or Linux bug...
Enough said.

And that is supposed to mean what? While PHP is slightly more dangerous than MS-Access in the hands of an inexperienced user - like most things - you can write incredibly scaleable apps using PHP/PG/Apache. Some companies who have used this stack (at least the PG/Linux parts) include:

-Whitepages.com

-ADP Dealer Services

-BASF

But, alas, I guess if you don't desire to offer your customers the best value, you don't need to consider using these apps... :)

Cheers,

-J

2006-09-22 Reply Admin

As someone who has done more than my share of PHP/PostgreSQL work, there is no way that code works as intended.

- If PHP cannot connect to the database server, you would have to re-try the connection inside the loop, so that has nothing to do with it.

- If a table is locked, PHP will wait on the lock as long as it can (usually 30 seconds) before terminating the script (thus no loop)

There are only two conditions I can think of that would apply to this code: 1) either there is a race condition, where another process is also in a loop trying to INSERT sessions:

<font color="#006600">// Try 5000 times to make sure that this stupid thing 
// inserts. This crap is such a toy.</font>
<font color="#000099">while</font> ( !$bInserted && $iTimes < 5000 )
{
  
  $iTimes++;

  <font color="#006600">//session started?</font>
  $sSQL = <font color="#990000">"INSERT INTO sessions "</font> .
          <font color="#990000">"VALUES</font> (DEFAULT, " . $user_id . <font color="#990000">");"</font>;

<font><font color="#006600">  //etc...</font></font>

Or, 2) the database abstraction library being used (there are many available for PHP) is so bad that it occasionally returns the wrong value for success or failure of a query.

Both of these, are of course highly unlikely, and the first could be checked simply by placing the SELECT and the DELETE inside the same transaction.

Methinks more likely our brave Sir Robert was covering for some other equally bad application design elsewhere.

2006-09-22 Reply Admin

themagni:
Doing the same thing over and over again and expecting different results is the definition of insanity. I can understand a few retries, but that's ridiculous. Of course, that leads to the philosophical question: "Given that 3 retries is acceptable and 5000 is not acceptable, what number between 3 and 4999 is the maximum acceptable number of retries?"

If you are not changing the parameters I don't think numbered retries are ever a good idea. If you are waiting for some scarce resource to be freed make your code check for that resource before making it's single attempt, and put in some kind of timeout code so your loop will give up if the resource won't be available in time. The only time multiple retries are acceptable is when whatever your code is supposed to do also happens to be the only way to verify that it can do it.

2006-09-22 Reply Admin

Anonymous:
themagni:
Doing the same thing over and over again and expecting different results is the definition of insanity.

So, what is it called when the different results finally do transpire?

A missed precondition for determining "sameness".

2006-09-22 Reply Admin

savar:
Anonymous:
Anonymous:

I have to say that "select * from sessions where..." is the most wasteful way to check for record existence though.
How would you do it?
SELECT COUNT(s_user_id) from sessions WHERE s_user_id = input_id;
?

If you want to check for existence of a single row, then you don't need to do a full-table scan.
Oracle:
SELECT s_user_id from sessions WHERE s_user_id = :input_id AND rownum<2;
MySQL:
SELECT s_user_id from sessions WHERE s_user_id = :input_id LIMIT 1;

This is fun. People, have you ever looked at the execution plan some query is mapped to before claiming "full table scan" or other scary stuff?

"SELECT COUNT(s_user_id) from sessions WHERE s_user_id = input_id;" is perfectly fine. It's not as clear as "SELECT 1 WHERE EXISTS(...)", but it does NOT perform a full table scan, at least not is there is *any* index that's got s_user_id as it's first column. And there bloody well better be.

Also the "SELECT TOP..." (or "SELECT ... LIMIT 1" or whatever) versions are unnecessary since we're probably limiting stuff to one session per user anyway (think "unique index"). If not, well, ok, but then the WHERE clause would be wrong ("delete all sessions of user X if he quits any one session" is surely not what's intended).

There is really really a whole lot of misinformation regarding databases out there. "IS NULL means full table-/index-scan" -> nonsense. "OR means full table-/index-scan" -> nonsense. "NOT IN (...) is slow as hell" -> nonsense. Go, get MSSQL 2005 Express and see for yourselfs - the included "SQL Server Management Studio Express" can display the estimated and actual execution plan and some nice statistics.

CAPTCHA: bedtime. not really...

2006-09-22 Reply Admin

Oh, MSSQL Server is such a toy... shamelessly optimizing queries without me asking.

Real databases don't do things behind my back like making my queries more efficient.

John Hensley · 2006-09-23 Reply Admin

Most of the Roberts of the world don't comment their code nearly this well (i.e. at all).

2006-09-23 Reply Admin

Nice education.

But why do most commentors use an existence check anyway? Either the record is there and the delete will nuke it or it isn't and nothing will happen. Without a transaction you can only shoot into your own foot, if something deletes the record in between and you don't notice and your error handling does funny things.

CAPTCHA: Pizza

2006-09-23 Reply Admin

Looks like he's trying to wait for an asynchronous operation using that loop. Normally such a loop would be infinite until some condition is met; allowing it to "fail" with a finite number of iterations will get such a client into trouble on faster processors...

2006-09-25 Reply Admin

Anonymous:
themagni:
Doing the same thing over and over again and expecting different results is the definition of insanity.

So, what is it called when the different results finally do transpire?

measurement inaccuracy

captcha: random well, ok. it might be called random too. didn't think of that.

TeeSee · 2006-09-25 Reply Admin

Anonymous:

I don't see the problem. If the database design includes a suitable index, then " select count(*) from foo where pri_key = 'bar' " should execute efficiently. Especially when you're testing if something is in the index or not. I agree that it's pointless to bring back columns that you don't need.

What everyone seems to have missed is that s_user_id is more than likely not the primary key. There shouldn't need to be an index on user ID either, except for some obscure 'see if this user is logged in' feature.

And the real WTF is that he's deleting by user ID, not session ID.

2006-09-26 Reply Admin

The back page of the Sept 4 Information Week has a full-page ad for Oracle. There's a feature matrix comparing Oracle Business Intelligence to Business Objects and Cognos.

One of the "features" that's checked for Oracle B.I. and not for the others is called "Real-time Predictive Decisioning".

What does that even mean?

2006-09-26 Reply Admin

Oops, I posted this in the wrong thread. Oh well.

2006-10-16 Reply Admin

You guys forgot hardware issues. ;)

2012-06-12 Reply Admin

TRWTF : he didn't use the build in session-handling code but instead wrote this huge pile of stinking crap ... Sessions in Database . true worse than failure

Persistence Gets the Job Done

Leave a comment on “Persistence Gets the Job Done”