• Prime Mover (unregistered)

    I wonder whether we should name and shame.

  • MiserableOldGit (unregistered)

    Doesn't explain what might end up in @tmp ... it reads like it would be a table of 1 row, but Remy seems to suggest their could be more? Whether or not relying on a test using "Top 1" is wrong depends on other assumptions, such as whether the number of rows in @tmp is ever different.

    Also how could somedate end up being null anyway? If it's an "upsert" shouldn't that test be checking whether datediff(somedate, getdate) <0 or something? As it stands it looks like an "upsert" but actually the rows being updated are not the ones that would be inserted on the other branch.

    Can't see this causing a big performance issue unless it's in a real hotspot and/or the indexing is truly dreadful.

    Still, as far as ERPs and their brethren go, that's a rather good bit of code, might not work, but usually none of it does anyway. At least it doesn't trash its database when two people try and do the same thing at once .... ummmm.

  • (nodebb) in reply to Prime Mover

    At some point (years of experience, job responsibility, relative costs) perhaps we should. On the other hand, if you can show me a developer who hasn't ever committed WTFs into production, I'll show you a douche who lies like a rug.

  • (nodebb) in reply to MiserableOldGit

    Article says "multi inserts into @tmp in the hundreds."

    Poor performance can be caused by the inserts. There's a reason SQL introduced the VALUES multi row insertion.

    The article says she found this code while diagnosing performance issues, not that this snippet causes performance issues itself.

    And trying to judge whether this code is a better than average or worse than average for SMB or ERP systems is like wondering if the engine is the worst part of Soviet cars, because the suspension at least works. Nah, the whole thing is junk.

  • my name is missing (unregistered)

    If it compiles: ship it is a common practice. Also, sales people will tell you anything to get a sale, reality is an optional value.

  • Omego2K (unregistered)

    I have to admit that I didn't know about merge

  • pudin9 (unregistered)

    "Now, this code is clearly SQL Server code, so a MERGE handles that."

    Does it? When I was working with T-SQL, we only dared to used MERGE for the simplest scenarios, as it used to be full of bugs (for example, https://www.mssqltips.com/sqlservertip/3074/use-caution-with-sql-servers-merge-statement/#comments). Is that better now?

  • Champs (unregistered)

    Cassandra relayed the issue to the vendor, but level 1 support closed the ticket as “not believable.”

  • DQ (unregistered)

    Even if you don't want to MERGE (or can't) you can handle this scenario better by first UPDATE-ing the existing records and then INSERTing the missing ones. Have done that lot's of times.

  • I dunno LOL ¯\(°_o)/¯ (unregistered) in reply to my name is missing

    That's why interpreted languages are so awesome, they don't compile the code, so there aren't any compiler errors! Time for a coffee break!

  • Thomas (unregistered)

    So what's so special about code that doesn't work? If there wasn't any, all software products would be bug-free, but in practice the opposite is true, i.e. all software products above some trivial, minimal level of complexity contain multiple bugs. The real WTF might be the process that allowed a piece of code like this to enter the code base, but the article doesn't really tell anything about the process.

  • JonathanZ (unregistered) in reply to Champs

    Nice.

  • (nodebb)

    If they were really trying to make this code "database agnostic" they did a great job, as far as WTFs go. MERGE is a lot more database agnostic than this piece of T-SQL.

  • TSQL guy (unregistered) in reply to pudin9

    Can confirm that MERGE is buggy and has shown performance problems in our production environment vs. the equivalent INSERT/UPDATE logic. Avoid at all costs.

  • (nodebb) in reply to nerd4sale

    Well, it is database agnostic in the sense that it will fail equally no matter what DBMS is used.

  • MiserableOldGit (unregistered) in reply to Mr. TA

    Actually it says

    --{ Dynamic single insert statements, may be in the hundreds. }

    which i took to mean just some inserting , not a direct reference to filling the @tmp table. But, I agree, coupled with Remy's reference to cursors, that your assumption makes more sense than mine. But it still may be the case that if we already have SomeTable rows generated for a given ID we would want to keep that set of rows (whatever they meant) and just update the timestamp. Granted it could be a huge bug.

    I can't see a performance advantage to using VALUES in this context, that's a local temp table, not something likely to have read locks on it when being joined in the insert. Would have been tidier (given the thing is dynamically generated) to do it that way, though. Trying to deal with a genuine database hotspot by unrolling the source data and building a VALUEs insert often sees inexperienced people replacing an obvious performance problem with a sneaky horrible race condition. Locks are serving a purpose, after all.

    It was Remy said there were performance problems with the code, he just didn't elaborate on where and why.

    And I wasn't forgiving its crimes because it's ERP code, more pointing out it's in good company! The times I've spent debugging stuff that integrates with ERPs only to then dig into the ERP code and find horrors that made me want to vomit .... well. In fact, the one I remember had a big presence in SMB (liked to use the letter e in its branding) and this is exactly the sort of thing I'd see in there. Mind you, it wouldn't be out of place in the various Dynamics offerings either. You're being unfair to Soviet cars, on the whole they were better thought out and more reliable than most ERP stuff.

  • tbo (unregistered)

    It's been a while since I used SQL Server, but does someDate = NULL work? At all? Shouldn't that be someDate IS NULL?

  • Naomi (unregistered) in reply to tbo

    That's where ANSI_NULLS comes into the picture. someDate = NULL would work if it were off.

    Also, TRWTF is making me click through nine pages of captchas. <_<

  • marketing guy (unregistered)

    "If it compiles: ship it is a common practice."

    Because this works surprisingly well. The customer will experience some bugs, they will be ironed out by code monkeys and everyone shall be happy. This is especially true in a high competition environment - if you don't ship now, someone else will, and good luck convincing the customers that your polished product is better (in the meantime, the competing product ironed out the most visible bugs), and that they should make effort to switch to your solution. Who doesn't ship, doesn't eat - that's sad but true.

    A few months ago there was a bunch of Zoom scandals - they didn't use E2E encryption, but claimed that they do; they sent data to suspicious Chinese servers... It doesn't matter! They had one foot in the door, taken place in the market and the whole scandal ended (for them) with a jolly dancing. If they didn't ship before figuring out how to do E2E, people would probably be using some other app now, and Zoom would be "an interesting alternative".

  • Fun With Resources (unregistered)

    I would do it back to them. Promote Cassandra as a cloud enterprise consultant. She does the one thing that fixed this 4,000 times for $100 an hour.

  • (nodebb)

    People wanting to Name-Shame: This might or might not be from Intland, but I can tell ya that codeBeamer is full of horrific crap, not the least of which is poor -to-nonexistent input validation for data imports (not commands, just importing a spreadsheet CSV table).

  • (nodebb) in reply to Applied Mediocrity

    Perhaps I'm optimistic...

    If you can show me a developer who hasn't ever committed WTFs into production, I'll show you a developer whose code has never passed review.

  • MiserableOldGit (unregistered) in reply to marketing guy
    A few months ago there was a bunch of Zoom scandals - they didn't use E2E encryption, but claimed that they do;

    In that respect their offering was no different to most of their competition, they were only offering genuine encryption between their own server nodes and claiming it was secure.

    What Zoom did was prioritise ease of use over security and let people turn off various security settings very easily to get going with the meeting. The users don't know any difference, it works and they couldn't get the other one to work. They are guilty and not guilty for that, but it's nothing to do with compile and ship. Various of their other fuck-ups are, though. totally stupid and unforgivable.

  • Erwin (unregistered) in reply to Naomi

    The captcha process presents a number of images and asks you to point out images that contain certain objects. If you score better than could be expected from a machine AI, it concludes that you must be human. But at the same time your answers are used to train a machine AI to become better at recognizing objects in pictures. So it becomes harder and harder to prove that you are not a machine AI.

    Apparently we're now at the point that the machine AI gets it right about 99% of the time so you need a perfect score in about a 100 images to prove that you're better.

  • 516052 (unregistered) in reply to Applied Mediocrity

    Actually you would be showing the one guy that commits more WTFs than the rest of us. A good programmer is one who knows that he is doing wrong when he is doing it and thus tries to avoid it as much as practical. A bad one just keeps chugging along oblivious to the fallout left in his tracks.

  • z f k (unregistered) in reply to Erwin

    "Captcha 22" ?

    CYA

  • Christian (unregistered)

    But okay, maybe they’re trying to be as database agnostic as possible, and don’t want to use something that, while widely supported, has some dialect differences across databases.

    LOL. As database agnostic as possible by not using a MERGE Statement and replace it with T-SQL...

    With a fundamentally misunderstanding of why this isn't only wrong but dangerous this is a set based approach of the procedural upsert which (like here) ignores the fact that database applications are multithreaded by design and ulitmately will either lead to ominous primary key violations at best or data corruption at worst. Having seen such constructs many times I am wondering how many bad business decisions are caused by corrupt data produced by those constructs...

  • Wizofaus (unregistered) in reply to Thomas

    Code that doesn't work in a particular set of circumstances is normal (and hard to avoid, given the many billions of possible states most software can be in), code that could never possibly work ever is not, and would certainly indicate a lack of investment in any sort of testing. Having said that, it's not uncommon to find such bits of code where whatever it was supposed to be doing wasn't actually needed anyway.

  • I dunno LOL ¯\(°_o)/¯ (unregistered) in reply to Erwin

    It's even worse than that. Since a few months ago, if it decides it doesn't like your browser (even when you're logged into your Google account), answering faster causes it to reject you more. I'd rather plod along than be punished, because punishment is getting 5-ups and slow-fades.

    I'm sure there are plenty of WTFs going on in the captcha department of Google, and this is one of them.

  • (nodebb)

    Really small businesses use Excel. Medium businesses use Excel. Enterprises use Excel. But in addition to that, the large businesses also pay through the nose for a gigantic ERP system, like Oracle or SAP, that they can wire up to Excel.

    So much truth. I know Remy's made this point before, and indeed I've agreed with it before, but, well, it continues to be true.

    While data may exist temporarily in other locations (like databases and ERP applications), its true home is always in Excel.

Leave a comment on “Where to Insert This”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article