The Daily WTF: Curious Perversions in Information Technology

2011-03-24 Reply Admin

Hey, what happened to my name?

2011-03-24 Reply Admin

Jake OBrien:
Hey, what happened to my name?

It became Frist.

2011-03-24 Reply Admin

DELETE FROM tblCommentsMaster INSERT INTO tblCommentsMaster (comment) VALUES ('WTF?')

2011-03-24 Reply Admin

Conn.open Seems very safe without transactions and so Conn.close

Conn.open First? Conn.close

Conn.open Captcha: valetudo Conn.close

2011-03-24 Reply Admin

Isn't this normal for ASP.NET?

2011-03-24 Reply Admin

Just stating the obvious: There are millions of addresses and names which may have apostrophes for a very good reason, and just omiting them may even change their meaning ...

But apart from that - what about the good ol' TRUNC command? Ah, let me guess: not supported by ASP.Net. OK, I get it.

2011-03-24 Reply Admin

TRWTF is VB, amirite? What's that? This isn't VB? Fine, TRWTF is VB.NET, amirite? ASP.NET? Come on guys, work with me here, we all know <platform> is TRWTF. I'm just doing what I can with sub-standard tools.

2011-03-24 Reply Admin

non-tea:
Jake OBrien:
Hey, what happened to my name?

It became Frist.

Hey, leave me outta this!

2011-03-24 Reply Admin

Thanks a bunch Gary, there's a good reason why I have an apostrophe in my surname you know.

2011-03-24 Reply Admin

Why does Brian have so much time to post to tdwtf, anyway? Is it because he's too uptight and insecure about his own coding?

Nagesh · 2011-03-24 Reply Admin

Gary not heard of parameters collections?

I prefer to code using non-microsoft langauges because they are generally considered better for linux.

Do you know linux user group is called L-USER group? Are you a L-USER?

2011-03-24 Reply Admin

Gary:
TRWTF is VB, amirite? What's that? This isn't VB? Fine, TRWTF is VB.NET, amirite? ASP.NET? Come on guys, work with me here, we all know <platform> is TRWTF. I'm just doing what I can with sub-standard tools.

Man, that was sooo gonna be my comment. You stole it pre-emptively. Bastard.

2011-03-24 Reply Admin

Um, this code, it's not very good.

2011-03-24 Reply Admin

That's not a WTF... that's merely sad. Alternatively, I'm at the end of a long working day and have lost all capability to be surprised.

2011-03-24 Reply Admin

im very surpised that noone has commented on the inherent danger in deleting all the records prior to making the update, then opening a new connection to insert everything (except 's) back in again.

i mean, do you think a table called 'tblCustomerMaster' might contain important stuff?

captcha 'dolor' yeah ive got plenty of that.

Kuba · 2011-03-24 Reply Admin

TRWTF is that it should have been done in a single non-query or a stored procedure without having to push all the data through the "pipes".

I have been lukewarm towards stored procedures. Then I had to prototype something quickly using OO.org Base as the front-end. I ended up creating a bunch of writeable materialized views that worked around OO.org's forms deficiencies, and my 'deficiency' of not bothering to code things in OO Basic. In the end, I'm accessing the same functionality from web2py, so no work wasted, and I'm a big fan of triggers and stored procs (without going overboard, I hope).

2011-03-24 Reply Admin

yeah , he should have used a "truncate table" clause ....

Kuba · 2011-03-24 Reply Admin

James:
im very surpised that noone has commented on the inherent danger in _deleting_ all the records prior to making the update, then opening a new connection to insert everything (except 's) back in again.
i mean, do you think a table called 'tblCustomerMaster' might contain important stuff?

That's so funny since Alex has been warning precisely about this scenario in his <a rel="nofollow" href=""http://thedailywtf.com/Articles/Testing-Done-Right.aspx"" target="_blank" title=""http://thedailywtf.com/Articles/Testing-Done-Right.aspx"">yesterday's rant.

2011-03-24 Reply Admin

First comment!

2011-03-24 Reply Admin

Truncate only works in sql server when the table does not have any tables referencing it's primary key. If you do, you have to use delete from...le sigh.

2011-03-24 Reply Admin

This actually looks pretty straight-forward. Granted you should probably put the whole thing in a transaction so that when something fails (like it running out of memory - which of course will never happen because they'll never have so many customers that they can't store them all with their address, etc. in memory) that they will still have something in the web database to serve their customers, even if it is out-of-date. And granted they strip all of the single-quotes out of the string fields rather than looking up how to properly accept arbitrary strings (parametrized expressions). Or they could set up real replication, assuming they are using a database that supports it.

Aside from fixing the quotes issue (and using truncate rather than delete), the one other change I would propose would be to store only one customer entry at-a-time and insert it immediately (batching the inserts).

Not really a WTF, though.

2011-03-24 Reply Admin

Linux User Groups are more commonly refered to by 'LUG'

2011-03-24 Reply Admin

According to the description, this is supposed to be copying from one database into another. Assuming they didn't really mean copying from one table to another in the same database (because that would be a trivial SQL statement), does it bother anybody else that conn isn't declared or assigned anywhere? It looks like it's all done with the same connection and therefore to the same database.

Stripping out the single quotes is a poor attempt at preventing SQL injection. Parameter collections are much better.

2011-03-24 Reply Admin

James:
im very surpised that noone has commented on the inherent danger in _deleting_ all the records prior to making the update, then opening a new connection to insert everything (except 's) back in again.
i mean, do you think a table called 'tblCustomerMaster' might contain important stuff?

captcha 'dolor' yeah ive got plenty of that.

We are lead to believe that the table that he is deleting from is the one in the database used by the web front-end, which database shouldn't contain anything that isn't already in the company master database, then he is pulling everything from the master database and pushing it to the web database.

I suppose a WTF is that there is nothing to tell you which databases are being used without looking up how conn and CustomerData are created; it looks like they could just as well be using the same connection in which case it would look like they are deleting everything in the table and then copying from the freshly deleted table to itself.

2011-03-24 Reply Admin

Dave:
... Stripping out the single quotes is a poor attempt at preventing SQL injection. Parameter collections are much better.

I don't think they were doing it to prevent SQL injection, I think they were doing it so that the insert statement which wraps the string values in single-quotes would work.

JayC · 2011-03-24 Reply Admin

airdrik:
James:
im very surpised that noone has commented on the inherent danger in _deleting_ all the records prior to making the update, then opening a new connection to insert everything (except 's) back in again.
i mean, do you think a table called 'tblCustomerMaster' might contain important stuff?

captcha 'dolor' yeah ive got plenty of that.

We are lead to believe that the table that he is deleting from is the one in the database used by the web front-end, which database shouldn't contain anything that isn't already in the company master database, then he is pulling everything from the master database and pushing it to the web database.

I suppose a WTF is that there is nothing to tell you which databases are being used without looking up how conn and CustomerData are created; it looks like they could just as well be using the same connection in which case it would look like they are deleting everything in the table and then copying from the freshly deleted table to itself.

I'm not sure that updating the master is the issue he's referring to (as I re-read Jame's Comment).

The Update Might Fail.

If you updated some temp or "update" tables first, validated that you have all necessary records and your records are sound and only then updated your primary tables using whatever methods necessary, it'd be much more sane. (Of course I'm ignoring some of the other WTFs mentioned.)

I also wouldn't agree with your WTF as MS SQL server connection strings often include the database name.

JayC · 2011-03-24 Reply Admin

Dave:
According to the description, this is supposed to be copying from one database into another. Assuming they didn't really mean copying from one table to another in the same database (because that would be a trivial SQL statement), does it bother anybody else that conn isn't declared or assigned anywhere? It looks like it's all done with the same connection and therefore to the same database.
Stripping out the single quotes is a poor attempt at preventing SQL injection. Parameter collections are much better.

Agreed about the SQL injection bit, although likely, it wasn't even preventing SQL injection, it was a poor attempt to make things work.

But you're missing something. The source data comes from:

   'Fill Dataset
   DA.Fill(DS.CustomerMasterView)

DA isn't initialized or anything in this code. We're to assume it knows how to get it's data, however it gets it.

2011-03-24 Reply Admin

Kuba:
James:
im very surpised that noone has commented on the inherent danger in _deleting_ all the records prior to making the update, then opening a new connection to insert everything (except 's) back in again.
i mean, do you think a table called 'tblCustomerMaster' might contain important stuff?
That's so funny since Alex has been warning precisely about this scenario in his <a rel="nofollow" href=""http://thedailywtf.com/Articles/Testing-Done-Right.aspx"" target="_blank" title=""http://thedailywtf.com/Articles/Testing-Done-Right.aspx"">yesterday's rant.

If by "yesterday" you mean "two days ago," then yes, it was yesterday.

2011-03-24 Reply Admin

Hey! I program in VB.NET.

Ok, sure I'd LIKE to be programming in C#, but everything was already done in VB.NET when I started this job.

Salami · 2011-03-24 Reply Admin

Wouldn't be terrible if he

Put the whole thing in a transaction
Replaced ' with '' instead of an empty string
Didn't open a new connection every time he did an insert

2011-03-24 Reply Admin

Mark:
Hey! I program in VB.NET.
Ok, sure I'd LIKE to be programming in C#, but everything was already done in VB.NET when I started this job.

Don't let that stop you. My current place of work used VB.NET exclusively, just because they thought the familiarity of VB-like syntax would make life easier for the developers (typical dumb management decision). One month and three Powerpoint presentations later, I had convinced them that C# was the superior language and we went fowards using it for all newly developed classes. We had a couple of ex-VB coders who weren't too pleased but they picked up C# in no time and haven't looked back. If you ask them today what they would rather use, they will emphatically say "C#".

2011-03-24 Reply Admin

Salami:
Wouldn't be terrible if he <SNIP> 3) Didn't open a new connection every time he did an insert

If he's using a connection pool (which is default on most ADO.NET drivers) that doesn't make much of a difference.

SCSimmons · 2011-03-24 Reply Admin

Anonymous Coward:
Isn't this *normal* for ASP.NET?

No. It's many things, but 'normal' is not in the list.

2011-03-24 Reply Admin

airdrik:
This actually looks pretty straight-forward. Granted you should probably put the whole thing in a transaction so that when something fails (like it running out of memory - which of course will never happen because they'll never have so many customers that they can't store them all with their address, etc. in memory) that they will still have something in the web database to serve their customers, even if it is out-of-date. And granted they strip all of the single-quotes out of the string fields rather than looking up how to properly accept arbitrary strings (parametrized expressions). Or they could set up real replication, assuming they are using a database that supports it.
Aside from fixing the quotes issue (and using truncate rather than delete), the one other change I would propose would be to store only one customer entry at-a-time and insert it immediately (batching the inserts).

Not really a WTF, though.

So... you're entirely comfortable with the fact that he reads the entire table into an array in memory and then writes it out to the new table?

Assuming there is some non-trivial work to be done on the way through, what about read a record, write a record, loop till end?

2011-03-24 Reply Admin

INSERT INTO Comment ('Ive read this many times and cant find the wtf. I dont suppose some couldnt point it out to me. Id really appreciate it.')

2011-03-24 Reply Admin

Oh, I see, there's no XML. Clearly they should be using an XMLDataReader and SQLXML.

2011-03-24 Reply Admin

Kuba:
James:
im very surpised that noone has commented on the inherent danger in _deleting_ all the records prior to making the update, then opening a new connection to insert everything (except 's) back in again.
i mean, do you think a table called 'tblCustomerMaster' might contain important stuff?
That's so funny since Alex has been warning precisely about this scenario in his <a rel="nofollow" href=""http://thedailywtf.com/Articles/Testing-Done-Right.aspx"" target="_blank" title=""http://thedailywtf.com/Articles/Testing-Done-Right.aspx"">yesterday's rant.

Today's dilbert relates directly with Alex's most recent soap box...

http://dilbert.com/strips/comic/2011-03-24/?utm_source=feedburner&utm_medium=feed&utm_campaign=Feed%3A+DilbertDailyStrip+%28Dilbert+Daily+Strip%29&utm_content=Google+Feedfetcher

Zylon · 2011-03-24 Reply Admin

The title of today's WTF gave me Fallout 3 flashbacks.

Too... many... Garys...

2011-03-24 Reply Admin

I dunno about ASP.net, but it looks like there's a database view where the data is fetched from, so for me to reproduce the same behavior, I'd run:

START TRANSACTION; TRUNCATE TABLE tblCustomerMaster; INSERT INTO tblCustomerMaster(CustomerID, IndustryID, CustomerName, Address1, Address2, City, State, Zip, Salesperson, Allocation, Supervisor, Manager) SELECT * FROM tblCustomerMasterView; COMMIT;

The database servers are generally designed to do this kind of a job pretty well (again, dunno about M$ technologies), so why not let them?

Also, why would you ever want to empty the customer table?

Lorne Kates · 2011-03-24 Reply Admin

Man, my favorite band is "Insert Spaceholder". But not their latest stuff, since they were signed. You have to experience their back catalog (though I won't blame you if you don't "get" them).

D-Coder · 2011-03-24 Reply Admin

OldCoder:
airdrik:
This actually looks pretty straight-forward. Granted you should probably put the whole thing in a transaction so that when something fails (like it running out of memory - which of course will never happen because they'll never have so many customers that they can't store them all with their address, etc. in memory) that they will still have something in the web database to serve their customers, even if it is out-of-date. And granted they strip all of the single-quotes out of the string fields rather than looking up how to properly accept arbitrary strings (parametrized expressions). Or they could set up real replication, assuming they are using a database that supports it.
Aside from fixing the quotes issue (and using truncate rather than delete), the one other change I would propose would be to store only one customer entry at-a-time and insert it immediately (batching the inserts).

Not really a WTF, though.
So... you're entirely comfortable with the fact that he reads the entire table into an array in memory and then writes it out to the new table?

Assuming there is some non-trivial work to be done on the way through, what about read a record, write a record, loop till end?

That would be a whole lot of individual hits on the database server instead of a single "read-'em-all" hit, yes? Although I doubt this coder could figure it out, it might be a lot more efficient.

2011-03-24 Reply Admin

Nagesh:
Gary not heard of parameters collections?

Worse, Gary's never heard of attaching a table from a remote database. The whole thing could be one line of SQL, possibly even wrapped in a transaction.

2011-03-24 Reply Admin

Web? Web? Did someone say web? Oooh, I know this one, let me!

Send the entire database to the browser. Hack together some script to extract the stuff I want, blithely ignoring those who have scripts shut off. Use AJAX, SOAP, REST, XML and whatever other acronyms I can think of (except ATOM) to pull the selected record from the database again even though it is already in memory. As the user tabs from input to input, post the whole thing back to the database even though nothing has changed, leading to 8 second response time (during the demo; longer as the data gradually populates). Hope our site never has more than one concurrent user making updates.

Did I get it right?

2011-03-24 Reply Admin

Gary:
TRWTF is VB, amirite? What's that? This isn't VB? Fine, TRWTF is VB.NET, amirite? ASP.NET? Come on guys, work with me here, we all know <platform> is TRWTF. I'm just doing what I can with sub-standard tools.

Let's see:

DB interface has stupidly complicated SQL parameter handling leading to this WTF, check.

No language feature to properly handle resource release during exceptions leads to error prone Open and Close paradigm, check.

Stupidly complicated to create a quick anonymous map, leading to error-prone parallel arrays, check.

Half-assed attempt at standardizing iterators, leading to fencepost bugs, check.

Uniquely stupid "Dim" statement that confuses lexical and dynamic memory allocation, check.

Yup, TRWTF is VB. You can add .NET all you want, it's still VB.

2011-03-24 Reply Admin

Brompot:
Salami:
Wouldn't be terrible if he <SNIP> 3) Didn't open a new connection every time he did an insert

If he's using a connection pool (which is default on most ADO.NET drivers) that doesn't make much of a difference.

Unfortunately the first thing that really got me upset was this opening/closing the connection INSIDE of the loop. Yes, it is not as bad as ADO.NET will reuse the connection through connection pooling, but it is enough of a hit to know to immediately move the open/close outside of the loop. It is a noticeable difference, depending on how many records you have.

2011-03-24 Reply Admin

[quote user="Sobriquet"][quote user="Gary"] No language feature to properly handle resource release during exceptions leads to error prone Open and Close paradigm, check. [/quote]

You may want to read up on the Using statement...

http://msdn.microsoft.com/en-us/library/htd05whh%28v=vs.80%29.aspx

2011-03-24 Reply Admin

airdrik:
This actually looks pretty straight-forward. Granted you should probably put the whole thing in a transaction so that when something fails (like it running out of memory - which of course will never happen because they'll never have so many customers that they can't store them all with their address, etc. in memory)

What makes you so sure that they won't be storing anything else in memory?

frits · 2011-03-24 Reply Admin

Lorne Kates:
Man, my favorite band is "Insert Spaceholder". But not their latest stuff, since they were signed. You have to experience their back catalog (though I won't blame you if you don't "get" them).

I have trouble connecting with their music. It doesn't drive me. It might be a compatibility issue.

2011-03-24 Reply Admin

Gary is Alex's code word for Nagesh.

2011-03-24 Reply Admin

[quote user="Not Gary"][quote user="Sobriquet"][quote user="Gary"] No language feature to properly handle resource release during exceptions leads to error prone Open and Close paradigm, check. [/quote]

You may want to read up on the Using statement...

http://msdn.microsoft.com/en-us/library/htd05whh%28v=vs.80%29.aspx[/quote]

"The code inside the Using block should not assign the object in resourcename to another variable. When you exit the Using block, the resource is disposed, and the other variable cannot access the resource to which it points."

So much for closures. I ran into this (implicitly, of course) when calling VB objects from JScript or vice versa.

Disclaimer: I am not the original Gary, though I probably have made similar mistakes.

Gary Strikes Again

Leave a comment on “Gary Strikes Again”