• skicow (unregistered)

    Dear God, that's brilliant!

    Spec: "Retrieve an order from the Orders table using the most system resources as possible."

  • cablito (unregistered)

    not to mention the dammed "SP" prefix.

  • Mark Heimonen (unregistered)

    Wow! I'm dumbfounded.


    That's one way to keep a support contract with your client...

    I'm going to spend the next couple weeks working on a "performance upgrade" for your system. Every couple weeks, replace one of these queries with a "select * from orders where orderId = @orderId", and show your client how you've been able to tweak the system with a "performance upgrade".

  • Tim Cartwright (unregistered)

    stop!stop!stop!stop!stop!stop! brain aneurism kicking in, heart stopping, eyes melting......

  • Eduardo (unregistered)

    Sorry, but cant believe this is true

  • Douglas Reilly (unregistered)

    Eduardo,

    I have no first hand knowledge of this, but I assure you I have seen equally inane code in the wild. Someone sees a sample, reqorks it so it does what he wants (even if that was not the original intent) and just lets it be.

  • Jeff (unregistered)

    You've got to be kidding! If the programmer is smart enough to create a temp table, delete all records NOT matching the criteria, reselect the remaining records and drop the temp table, W(hy)TF didn't he just select the correct records to begin with? Was this the entire stored procedure? Most of the posts on here seem "real", because I've seen some boneheaded code in my day (and I've probably written some, a long, long time ago), but this just doesn't seem "real". They use a where clause and you can't tell me that if they are aware of <>, they aren't aware of =

  • Alex Papadimoulis (unregistered)

    I have a feeling that the coder believed that a Temp table is stored in RAM, and hence it would be faster to copy an entire table in ram, delete the rows, then return them. That's wwhat I'm guessing atleast ...

  • Bonjo (unregistered)

    LMAO!!!

    That reminds me legacy code been converted to a RDBMS...

    Just a copy and paste!!!

  • Jason Langston (unregistered)

    I can't buy the "maybe he believed Temp tables are stored in RAM" theory. See Jeff's comment above.

    I think Mark was on to something - this was intentional. Either revenge, a joke or milking a contract, something.

  • Nugget (unregistered)

    Worse? Sure --

    Replace the transactionless "SELECT INTO" with an "INSERT INTO ... SELECT".

    See? Some degree of optimization has been performed, even if it was accidental.

  • Steve (unregistered)

    "They use a where clause and you can't tell me that if they are aware of <>, they aren't aware of = "

    lol... thats a good point

  • Matthew W. Jackson (unregistered)

    Perhaps its somebody trying to degrade the performance on one RDBMS by making its stored procedures stupid so they could get the client to buy the database software they like.

  • Lynn (unregistered)

    Maybe they want to look sophiscated by writing a few lines of redundant code rather than reducing it to 1 line of code. Trying to justify to the client that it's money well-spent hiring a sophiscated programmer.

  • Huiyong (unregistered)

    Thank God the proc creator didnt know about cursor at that point, else the code will look like this instead.

    alter procedure sp_get_order (
    @OrderID int
    ) AS

    select * into #temp_order from Orders
    declare @tmp_orderid int
    declare cur cursor for select OrderID from #temp_order order by OrderID
    open cur
    fetch from cur into @tmp_orderid
    while @@fetch_status = 0
    begin
    if @tmp_orderid <> @OrderID
    delete from #temp_order where OrderID=@tmp_orderid
    fetch next from cur into @tmp_orderid
    end
    close cur
    deallocate cur

    select * from #temp_order
    drop table #temp_order

  • Ecoist (unregistered)

    I could think of a worse solution, but I would have to consider all other options and forget the ones that were identical to this.

  • Peter (unregistered)

    Maybe the Order table had not set indexes correctly. Then this solution would be quicker than select ... where ...

  • Geert (unregistered)

    No not a better solution, but considering the fact that were i work all the applications are written like that ... Well, just until i started that is ...
    My best ever record for code improvement was from 3.5 days to 40 seconds ...
    The boss was really impressed, the only thing i did was change 4 pieces of nested code like this into 4 simple queries ...
    Damn, i did forget to ask for a raise !

  • Ovidiu (unregistered)

    I'm speechless...

  • Simon (unregistered)

    I once was told to put sleeps into a program as the customer was getting a massive discount, this way they could pay for speed enhancements, perhaps this is along the same lines, or maybe the guys just an arse.

  • icelava (unregistered)

    he's trying to find ways to convince management to up the budget for a meaner server. :)

  • Hannes (unregistered)

    Is this a April fool joke?

  • af (unregistered)

    I bet the guy who wrote that got provisions of the hardware sold to run that.

  • wha? (unregistered)

    Peter, are you on crack? Even without an index on OrderID it would be faster. Let's compare:
    select ... where ...
    1 table scan
    select into, delete, select
    2 table scans

    For all our sake, I just hope you aren't a database developer.

  • scott (unregistered)

    Actually, if you want to be pedantic it's 3 table scans (one being a one row table scan). Also, when you create a temp table in MS Sql server it has to be written out to disk and I doubt anyone who writes code like this uses any other database.

  • Bernhard (unregistered)

    The stuff dreams are made of... I return to the client saying that I'll be able to provide a huge performance improvement if I can review the code in complete isolation at home. I take a week holiday and return. LOOK AT IT RUN NOW!!

  • johan (unregistered)

    I feel bad for some junior programmer who is going to look at this as an example of how to do it... see that it says 'the fastest way to get a record' and write code like this... hopefully the kid picks up a sql book first.

    scary...

  • Cptn WTF (unregistered)

    This HAS to be a joke.

  • Jeff (unregistered)

    I don't know ... kinda makes sense to me. When I go shopping for an item, say a lightbulb, I usually buy everything in the store and then go back and return everything except for the lightbulb.


    Mission accomplished.

    The code looks good to me. Send it to production!

  • RavenBlack (unregistered)

    Without context, one possible explanation is that the programmer was asked to make it slow so that it looked like something important is happening. A friend and I have both had this sort of thing requested on separate projects. I did mine with an actual timer delay (so that it would always take about 7 seconds no matter how fast the computer), but I don't think you can do that sort of thing in SQL.

  • Tim Cartwright (unregistered)

    RavenBlack, take a look at T-SQL WAITFOR.... That way you can propogate the insanity into your SQL as well.

  • Jim (unregistered)

    Lynn has a good point. A PHB looking at the code will be impressed at how smart the programmer is. For a prime example, read "A Tale of Two Programmers":

    http://www.linux.ie/pipermail/social/1999-October/000483.html

  • Nigel Rivett (unregistered)

    Can think of a way it came about.
    No index on OrderID
    Ran it with the single select and discovered it was very slow. Ran this query and found it a lot faster.

    Didn't realise it was because the first query got all the data in memory and never tested in the reverse order or the singleton select twice.

    Or maybe I'm being too generous here.

  • Rajah Donalt (unregistered)

    Sorry didn't realize my submission was posted. This is not an April Fool's joke. The only alterations to the original code is the removal of the comments (this is in a client's system and I didn't feel comfortable posting potentially identifiable info) and substituting Northwind table/column names (again, to prevent anything from being itentifiable back to the source). It is the entire stored proc, not a snippet of a larger proc. The code is around 7 years old and was done by a programmer (not a DBA thank God). The comments from the author claim (as Jeff theorized above) that moving the data into a temp table and then scanning the temp table was ultimately faster than directly selecting the row. Hope you found it as funny and dumbfounding as I did!

  • DJ (unregistered)

    Hem... 7 years ago, circa SQLServer 6.0 when selecting from a single row in the table put a lock on the table. but doing a select into the tempdb didn't use any locks..

    Guess he found a way around having to wait for the page locks to be released from other queries.. hence "under load" a faster select.
    Evil and should have been changed after 6.5 sp 1 :D .... pour database administration..

    DJ

  • Nigel Rivett (unregistered)

    In 6.0 the select into tempdb would have locked the system tables in tempdb effectively stopping much else happenning on the server. Maybe that's why it was faster - had the erver to himself.

  • Richard P (unregistered)

    I've toyed with the idea of using a temp table to return rows 100-120 of a given query, for example.

    I could see something like this evolving out of that idea.

  • moondogg (unregistered)

    Even better, here's what I ran into a few times at my last job...

    Instead of:
    select * into #temp_order from Orders
    delete from #temp_order where OrderID<>@OrderID
    select * from #temp_order
    drop table #temp_order

    We brilliantly used:
    select * into temp_order from Orders
    delete from temp_order where OrderID<>@OrderID
    select * from temp_order
    drop table temp_order

    Oh, this was a web application with 100 concurrent users, if that makes any difference...


  • skab (unregistered)

    >reqorks it

    I know this was a typo, but it really makes for a great verb for what was done here...

  • curry684 / Niels (unregistered)

    Actually I've seen worse than this :)

    I took over a project that was into production, and was looking from some SP's. One of them was about ~50 lines, and I honestly had to spend half an hour analyzing what the hell it was doing in all that spaghetti (layout wasn't the original coder's strongest point either).

    What he did was create a temporary table, then do 2 select-into queries, select the whole table and drop it. This could possibly make sense, except for the fact that I couldn't spot the difference between the 2 select statements. Eventually I noticed the small difference, that one of them did an 'is null' comparison and the other one an inner join on the same field.

    Indeed I replaced the whole SP immediately with 4 lines using a magical thing called 'left outer join', stood up from my chair and parked my head in the wall 3 times.

    Stupidity is everywhere :-|

  • george (unregistered)

    This can't be real! I bet that the stored procedure kept on evolving by many people that don't read all the code before changing it... and then the code was "stipped down" to those lines by somebody who saw the bigger picture...

  • new to forum (unregistered)

    Oh my... that's sad :-/

    I think it could be real. I’ve seen a lot of odd code written by front-end developers. They have a different way of thinking.


    Anyway, for the few who were trying to figure out reasons why the delete method might be preferable to a single select, I ran a couple of tests using a million row table (no index).

    The SP's loony method took anywhere from 12593 to 13500 milliseconds.
    The single "select where =" statement took between 153 and 250 ms.
    (The numbers differed depending on whether the batch was executed first or second.)

    And, of course, the query plans were worlds apart.

    Unreal.

  • Jon Baggaley (unregistered)

    One of the clients I visited a few years back used a seperate table for every row (and then got upset at my incredulous reaction)....

  • Mike South (unregistered) in reply to Jon Baggaley

    This is waking up a really old thread, but I've seen a few similar things in the forum.

    Evil code is:

    select * into #temp_order from Orders
    delete from #temp_order where OrderID<>@OrderID
    select * from #temp_order
    drop table #temp_order

    Rather than intentional sabotage, it's probably a bit of poorly understood wisdom passed down from pre-relational, or early stupid SQL, databases. If you were going to retrieve a sufficient fraction of the rows from Orders, and the OrderID links were sufficiently disorganized, then it could be faster to do three serial passes over the data than it would be to bounce around following the index links. Of course, it would be even faster to skip the delete:

    select * into #temp_order from Orders
    select * from #temp_order where OrderID=@OrderID
    drop table #temp_order

    For example, if Orders was a holding table for unfulfilled orders, with rows constantly being added and deleted and stuff scattered all over, and you could expect to retrieve, say, 5% of the rows, then it might actually work. On the other hand, if Orders was pretty much a permanent repository, with most of the rows for an order layed down physically near each other, than it would suck big time.

  • malaprop (unregistered)

    Ok, so I'm replying 3 years+ later, but I'm pretty sure the original coder was trying to retrieve all rows matching the criteria AND that may be null.

    where field = @value or field is null

    (is "is null" available outside of T-SQL?)

  • Pete (unregistered) in reply to malaprop

    The whole table would be locked during execution of the stored procedure - if that is what the programmer intended!

Leave a comment on “Can you think of a worse solution than this?”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article