• Hanzito (unregistered)

    Frist step of solving performance problems: remove code. Everybody knows that. The "WHERE" clause in a query, pesky flags in shell scripts and conditions comparing pointers to nil are great starting points.

  • The Dave G (unregistered)

    Why did Clara bother with YYYYMM in the first place? Why not simply "" ?

    The whole thing has a bigger code smell. Downstream processes require a non-null character field. The database allows it. There's your problem.

  • I'm not a robot (unregistered)
    And then there's the stray case where they enforce a collation on the input string, which I have no idea why they deed to do it there but nowhere else, and I don't think they knew either.
    That's probably to force the check to be case-sensitive, so it can distinguish between MON and Mon. It may or may not be a bug that it doesn't do the same in the REPLACE call (I don't know enough about T-SQL to say), although it's unlikely that a single format string would use both.
  • Darren (unregistered)

    If the code isn't there then it can't be run. Code that doesn't run can't cause any performance issues. Therefore the perfect solution to performance issues is to remove all the code. Simple.

    That'll cause functionality issue, you say? Well, this is all about performance - you'll have a raise a fresh ticket for functionality problems.

    /s

  • (nodebb)

    If they went with a SQL CLR function which made an API call to a cloud hosted web service, the performance would go up dramatically. Especially if the cloud web service is set to the maximum performance tier.

  • Kenn Humborg (unregistered)

    Did they ever run this on dates in _M_arch or _D_ecember?

  • Steve (unregistered) in reply to I'm not a robot

    I do see what they're doing there... The collation they're setting it to is a case sensitive (and accent sensitive) collation. It implies that the database is in a case insensitive collation, meaning normally MON and Mon would be treated the same and only the first line encountered would be followed. The key is that in one branch they are upper casing the output, and in the other they are not touching it. So this means they put this hack in so they could output their month name all in caps or not based on whether their format string was all in caps or not...

  • (nodebb)

    I hope Clara improved her performance - with a new job.

  • Dr Pepper (unregistered)

    The requirement is "something, just not null". So why bring in dates at all? Just "InvalidEmailAddress" should work. No need for a function at all: in the queury, use nullif to replace null value with the string.

  • Barry Margolin (github)

    The part that bugs me is all the CHARINDEX calls. There's no need to do this before calling REPLACE -- if the thing being replaced isn't in the string, it's a no-op. If you're trying to improve performance, doing every search twice is hardly the way to go.

  • ZZartin (unregistered)

    Or just CAST(NEWID() AS varchar(50)) + 'yoursystemsucks.com'

  • Jonathan (unregistered)

    I know that it used to be the case (Oracle, years ago), that SYSDATE required jumping into the kernel, possibly with a context switch, to get the exact, to the fractional second, system time. So using it in a SQL query that had to loop over a lot of data was considered performance killing - just call it once outside the the "loop", stick it in a DATETIME variable, and use that in the SQL.

    So maybe that's what the guy was remembering. But he sure didn't solve that.

  • Jeremy (unregistered)

    So for those applications that do not want to accept null you'd have to adapt the applications that do care about email adresses to derive null from 'blank_email_xxx'. That sounds like a great idea. Or we can wonder why certain users never seem to get their emails...

  • WTFGuy (unregistered)

    The next thing I'm expecting is one of the downstream systems, or a system downstream of there, will choke on an email address that's not in proper email address form. Probably checked by a regex that we all know is wrong, but at least recognizes the base cases well enough to have sat there running quietly for years.

    So now Clara's (or the senior dev's) email addresses that aren't really email addresses will choke something else later. And whoever is tasked to fix that will have no idea why this code is spamming weird date-junk into what "everybody knows" is an email address. IOW, why not just return "[email protected]"?

    It's WTFs all the way down.

  • (nodebb)

    I bet the downstream application treats an empty string as a null, meaning a lack of field value. And in fact it probably keys on the email field, expecting it to be different for every user.

    So really the issue is the database schema is not aligned with the application, and that email field simply should not be nullable. But that's probably too much redesign work that some manager doesn't want to see, so better to task the lower level dev to work around a crappy "solution", which at the end is more work for everyone.

  • Labasta (unregistered)
    Comment held for moderation.
  • Officer Johnny Holzkopf (unregistered) in reply to Ralf

    Why not use "[email protected]" isntead? Or "-" (which is not null and also not emptystring, and doesn't look lika a valid email address either) and could be rejected further down (or up?) by any part of the software trying to send email to someone who does not have an email address.

  • (nodebb)

    Okay, someone has his presentation layer in the database. I think there's nothing more to add here, just burn it with fire.

  • mantra (unregistered)
    Comment held for moderation.
  • ichbinkeinroboter (unregistered) in reply to Dr Pepper

    downstream code that assumes an email address for a user record might also assume it is unique... which is all kinds of amusing with all the code shown here :-)

Leave a comment on “Fast Conversion”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article