- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
Most databases will optimise the query and execute exactly the same in both cases....
Admin
there's nothing necessarily wrong with a loop rather than one insert statement. Advantages of using a loop including the ability to log results, audit, trap errors etc (although none of those are done here). The WTF's are:
1) The loop isn't optimized to be a fast-forward read-only cursor which would shoot through the records quickly. It also returns too many fields rather than those needed.
2) The real error - the SQL command is recreated every time through the loop. It should be created and prepared once, and then the parameters populated each time through the loop. Ideally it should be some sort of builtin SQL append command structure, rather than a standalone SQL Insert command.
Oliver TOwnshend
Admin
Long-winded example: my first real job out of college was working on a simulator for the Navy. My company was responsible for the acoustic modeling code (an unholy marriage of Ada, C, and Fortran, but that's another story). Another contracter was responsible for the 3D graphical display, which looked really cool but wasn't quite fast enough. We were asked to see if we could speed it up.
This was absolutely the worst C code I have ever seen someone get paid for. The entire system was a single main() function, around 5000 lines in length, with several hundred distinct variables, half declared within main(), the other half at file scope. Files containing fragments of C code were #included in random places throughout the source. The <strike>primary</strike> only control structure was goto (about 13 of them, branching pretty much at random). It took a coworker of mine over 2 weeks to figure out the flow of control. It was also brittle as hell; trying to fix a problem for one execution path introduced bugs everywhere else.
It was clear from the outset that the only viable answer was to scrap the whole thing and build a new one from the keel up, which we had neither the time nor budget to do. So we simply tried to build it with optimization turned on (-O2, I think). The compiler ate up all the physical RAM, then all the swap, and finally panicked the system.
We told the customer that we couldn't make it go any faster without doing it over from scratch. So they just bought faster hardware.
Admin
Well, due to the different normalization constraints (see my previous comments), you'd still want to pick some values from across the data model and insert them into a single record - though those values should be limited to e.g. "user U1's ID" rather than "user U1's ID, SMS number, first and last name"; the extra info should instead be picked up by joining the user table to the queue table.
Admin
I've seen one once. Honestly.
Admin
The first version is old-school MS SQL Server. There was a time that MS SQL Server (and Sybase SQL Server) did not support joins directly. All tables to be joined where listed in the from clause and the where clause contained any logic for joining tables. Today, MS still supports the first version, but tries to optimize it as an inner join in its execution plan. That's why the consultants had to throw in all the [table].* crap, so that it would slow it down and hide the fact that the old-school join is just as fast as doing it the right way. ;)
The consultants were clearly afraid of SQL and tried to move as much logic into the code as possible. I could write this spamware in about an hour using a SQL agent and writing a multithreaded client service to poll a queue table, taking the top 10 or 20 items to be processed by a thread. The slowest part of this whole program should be the actual SMS text sending routine.
Admin
Puh-leese... databases were born to JOIN :)
Admin
Yep, and I saw Elvis walking down the street just last week!
Admin
WTF? Have you never heard the startup folder in the start menu?
:D
Rich
Admin
Admin
Admin
I appreciate the replies. What I am missing here is that with the original WTF code, and the optimized solution, nowhere do you see the 'unprocessed messages' deleted or modified in any way to indicate that "this message has been placed in queue". The one line SQL may be great at populating the queue, but what keeps it from populating the messages over and over?
If the point of the queue is to make the bulk send of messages as simultaneous as possible, I'd probably do the big select query, writing each message to a flat file, and then mark each record as "queued". Then, notify a second process to grab that flat file, and blast the messages as fast as possible. If flat files aren't fun, use a queue table, I guess, but still, something needs to be done to mark the messages in the first query as sent, queued, whatever.
Admin
ACID? (Specifically the A.)
Admin
The real WTF is that the client has that much money, and yet JT is making a lowly contractor's wage. I don't see stupid queries like that where I work (we have a good review process), but if I did, I would certainly be vocal about it. Don't take no for an answer. If somebody tells you to shut up, take it over their head. There is SOMEBODY in the company who will appreciate saving money on hardware by writing better code.
Admin
You are only seeing a single transaction of a single client taking place here. Normal gateways handle thousands of transactions all the time for thousands of different users. It's all about scaling; when the count of concurrent transactions increases, *any* system slows down. When there are other transactions taking place, there are some that need to "wait their turn"; more transactions, more waiting.
Furthermore, as has been stated earlier, fetching a set of rows from server to client only to return them one by one back to the server is a non-scalable solution (the original one written by the consultant). However, if we can make one query to perform a bulk insert of the rows we want *all within the server*, we decrease the job required ... from 2001 executed queries (each of which, in the worst case, need to wait for some time before they actually get a chance to execute themselves) to just one query. The best way to optimise the job to be done is not to do the job at all :) and I don't mean that the actual work should be left undone; with one query we do just as much work as the original 2001 queries, but we had much less job to do.
Because we had much less job to do, there was much more time on the database end to do other things. This meant that we were able to serve many more clients at the same amount of time. If all the clients behave the same way, the performance gain is exponential rather than linear.
Still, as a conclusion; having seen how some operator end SMS gateways have been built, I think it's rather amazing SMS works as well as it does these days. There are plenty of similar WTFs to be found on those systems :D
Admin
ACID, are you on it? Yes, we insert a bunch of records into QUEUE by running that query. Someday, oh someday, we may want to to populate that queue again with more unsent messages. When we run that query again, it's going to pull up those same records again (in addition to any new ones). Unless we do something to remove them or mark them as sent in the tables of the first query, which I don't see happining here.
Admin
So what is the difference between LEFT JOIN and INNER JOIN? Or are they the same, and it's just a dialect thing?
Admin
No ambiguity, at least not in Oracle...you can use the (+) operator to make outer joins:
SELECT t1.col1, t2.col1 FROM table1 t1 table2 t2 WHERE t1.pk (+) = t2.pk (+)
This is a full outer join..You can make a left join by removing the (+) on the right hand side I believe.
Still, ANSI syntax is clearer and preferred by most developers I know. As somebody else stated, this syntax is an easy way to miss a join key and get a Cartesian join between two tables.
Admin
However MSSQL 2005 does not permit the use of the first one by default.
Admin
But ... but ... but... if it's not a GUI program, how would the consultants get the PFK program to work with it???
Admin
gotta hate it when you have a superior solution, but not a superior enough position to get it put into production
Admin
Where "all else fails" is defined as "we have a good solution but we don't like where it came from". This attitude alone is sufficient to keep mankind from ever reaching the stars.
Admin
But only the insert was optimized in the fixed version; the cartesion product problem would still be there. Also, only 2000 inserts were performed (or so it's stated), so this probably wasn't the issue. It still doesn't add up.
FWIW, I think the ANSI syntax is far easier to read, and makes your intentions clear.
Admin
I think the implication from the post was that the application was downloading a crap-ton of data from the database server only to re-force it through the limited-when-compared-to-that-of-the-fibre-channel-array-on-the-database-server bandwidth of the local network. I chalk the fuzziness in the numbers ("2000 SMS messages shouldn't take that long") up to anonymization and WTF code else where in the system.
Admin
So if you have a table with prisoners:
Admin
One is ANSI standard SQL, and the other is ANSI standard SQL from a later version of the standard. In the Before Time there was only inner join, later the various kinds of outer join were added and "INNER JOIN" was added for symmetry. The word "INNER" is a noiseword that is mostly ignored, so "JOIN" without qualification and "INNER JOIN" are identical.
Both are supposed to be semantically identical, although some servers will treat one of the two forms as a hint that the joins should be done as nested loops with iteration over one table or the other on the outer loop.
Admin
No, they are very different. LEFT JOIN is synonymous with LEFT OUTER JOIN... basically, all items in the LEFT table will be present, even if there's nothing in the RIGHT table for them to JOIN to. RIGHT OUTER JOIN is just the opposite, but since left and right are arbitrary (you can always swap the order you specify the tables in), usually only LEFT OUTER JOIN is ever used.
INNER JOIN, on the other hand, will return only results that have a non-null counterpart.
Say you want a list of all customers and their most recent order. An INNER JOIN would do exactly that, and would leave out customers with no orders. A LEFT OUTER JOIN will return customers with no orders, too.
Admin
The non-WTF query still has to do the SELECT. Maybe there's millions of rows in the SELECT that have to be scanned, filtered, and boiled down to 2000 output records. I wouldn't be surprised if there e.g. wasn't an index on one of the join columns in one of the smaller tables.
I would expect that the 2000 inserted records take maybe 1-10ms to execute depending on server load, the other 299,999ms would be spent doing the SELECT.
That does imply that the 2000 INSERTs take 175 minutes to execute though.
Another possibility is that there's some nasty locking issue going on with concurrent transactions. I've seen systems where some client somewhere grabs hold of a lock on some row in some table, and *everything* grinds to a halt until that client (which for some reason is always the slowest client ;-) finishes whatever it was doing. Maybe the big SELECT has to wait until a moment where nobody is sending any SMS messages before it gets to start executing.
Admin
Thank you.
Admin
I'm just glad to know that the people selling those idiotic joke SMS services are just as dumb as their clients. Of coruse, I suppose that means that they really understand their market... :-)
Admin
Sorry, that should have been PFB (Press the Freakin Button) instead of PFK.
Admin
I think programs in the startup folder (and registry Run* entries) are not processed until someone actually logs in. While a service can be run in the background even without logged users.
Admin
Obviously the original system must have had some sort of "don't queue things more than once" functionality, we just weren't shown it because
Admin
That often turns out to be "the other clients also grind things to a halt, just not for long enough to be noticeable".
Admin
My former roommate once went to a conference where he had them put "Supreme Hosehead" as his title on his name tag. I don't think anyone said anything.
--RA
Admin
It requires three lines, but its pretty easy to do using atomicitiy and transactions:
1. Select out the messages destined for the queue out of the input join into a temporary table.
2. Delete or flag those rows from the input table as being queued.
3. Insert from the temp table into the queue table.
Or without using a temp table:
1. INSERT ... SELECT .... into the queue table, use a column in the queue table to flag a row to indicate that it is newly added.
2. Flag or delete rows in the input tables if they're in the queue marked with the newly added flag.
3. Remove the newly added flag from the rows in the queue table.
Admin
Unfortunately, this problem of inefficient sql algorithm, if you will, is all too common for anyone's comfort. I had once seen code just like the example provided today where invoice and billing generation took well over 6 hours for just roughly 20,000 records. Now, initially, the business thought it was due to the volume of records being processed.
I , however, felt something was wrong and when I looked under the hood, I saw code that resulted in n*n processing. The 20,000 records were obtained into 1 dataset. A loop is done for each of these rows and within that loop, a call is made to the database to find out if the record was suitable for billing and invoicing.
This in effect meant 20,001 calls to the database just for the pre-process phase. The process phase was equally interesting. The rows that qualified for processing was looped through individually, and then business logic with database calls were made for each and every one of these rows.
Bottom line is that I charted and deduced that there were roughly 150,000 separate calls to the database for the 20,000 rows of sample data. This resulted in the process taking 6 hours. I re-designed the code to take the most direct route to obtain data, plus the use of Readers (both SqlDataReader and the XmlReader) to generate the invoices in under 30 mins for the same 20,000 rows.
Problem was that the business has no budget to FIX this flaw and had no unit testing in place to ensure that after the changes were made, everything else checked out. Plus the programmer that was responsible for this big WTF was very touchy about enhancements to his work since that would put him in a very bad light.
Well, the business unsurprisingly ran into financial difficulties and had to let go all of us consultants.
Moral of the story: When you see WTFs like today, the cause of them is often POOR MANAGEMENT
Admin
Where I work we have successfully used outsourcing for some jobs (not programming). It only works when you have someone dedicated to Q&A demanding quality work and providing the company what they need for that quality work.
Admin
Yes. In MySQL(as of 4.X) the second statement is almost always faster than the first one at some degree.
Haven't studied if there's optimization in 5.X yet.
Admin
It's not uncommon that experienced nurses know more than new grad. (preheps even moderate experienced) doctors, yet those nurses won't become doctors.
It's educational background that counts...
Admin
Why be so circuitous? Just drop the queue table and recreate it every time. Make it temp, if you have the memory. When the SMS engine starts, it'll drop and populate the queue, then start pumping them out and marking them sent in the main database, and when done (or nearly done) can start the process over. (The SMS engine need not be the SMS server, it should be a load-balancing distributor if there's more than one sending server.)
Also, a database discussion without Jeff S? What has the world come to?
Admin
I don't think the loop is necessarily so crazy, depending on what has been elided in the for block. For example, what if 1 insert in 2000 throws a FK constraint error or otherwise fails? Would you rollback everything in that batch and sit on it while you wait for someone to answer his page and figure out where he fscked up the database design, presumably leaving 2000 undelivered messages, or would you log the specifics of the error and try to deliver the other 1999?
Admin
To Anonymous (and for the record):
The "send" table contains the individual SMS messages to be sent; the "outbox" table, OTOH, contains just the text to be sent. The JOINs are necessary to connect the "address book" -- which contains the mobile number to send to. And, the 3 hours it took? It was the fact that they were doing an INSERT for every row to be placed in the "send" table, instead of doing a simpler (and more amenable to optimization by the SQL server) INSERT INTO... SELECT FROM. Why would it be slower?
Thus the WTF.
PS : I long left that company. As for the figures, I don't have the actual/exact ones at hand at the moment, but I do recall the difference was several order of magnitudes -- a difference between minutes/hours to some seconds or minutes. And the 2000 records refers to 2000 rows in the "outbox" table -- which would populate several thousand rows on the send table. (I think I forgot to add that clarification)
Admin
Both the "outbox" and "send" tables in fact had a status column; I just didn't find it necessary to include it in the submission (as there wasn't anything WTF about it-- just update the column at the end, after doing all sends, or update a row's send status etc.).
BTW, the original code didn't even wrap all the INSERTs in a single transaction. Go figure.
Admin
Stories like this make Lankiveil angry. It's got the trifecta: inept consultants, awful code, and lazy management.
Admin
Common sense (and my PostgreSQL background) dictates that the second form should run faster. I was quite surprised when MySQL (4.1, as it happens) was faster on the first version by a few miliseconds. Then I remembered the kind of application MySQL is optimized for...
Admin
When they are definitely NOT identical is whhen they are outer joins...
SELECT *
FROM table1, table2
WHERE table1.key *= table2.key
and
SELECT *
FROM table1
Left JOIN table2
ON table1.key = table2.key
Then they can in certaincircumstances actually return different results...
Admin
I'm sorry but you're all on completely the wrong track with this one.
Yes, the original SQL/design wasn't very good, but all good developers were once beginners, and you can't have every development team consist of only experienced people.
Yes, it probably should have been caught in a code review, but we've all been on projects where unimportant things like technical design documents and code reviews are the first thing to go when time starts running out.
Yes, the better solution should have seen the light of day but without knowing the politics of why there was this caste structure in place it's hard to know whether there was any better way to arrange things. Politics is a part of life for almost all projects, deal with it, you'll be a happier person.
No, boys and girls, The Real WTF™ is that this "solution" made it into production without ever having been load tested with real-world data. Had there been anything other than rudimentary unit testing done someone would have noticed that the system will be overrun soon after it goes live. At which point, since it would be a show stopper, some of the more experienced developers or (gasp) a DBA would be brought in to help investigate.
So The Real Culprit™ was the Project Manager, who should have made sure that the testing regime was geared towards proving that the system will do what it is supposed to. We are not told whether he/she was a permanent, a Consultant, a Contractor or Paula herself, and it doesn't really matter because there are good and bad PMs everywhere.
Admin
Seen it several times. Happens more frequently if the consulting firm is also the hardware supplier (hint: IBM).
Admin
Seen it several times. Happens more frequently if the consulting firm is also the hardware supplier (hint: IBM).