CodeSOD: Fast Conversion

Clara had a database of users. The EmailAddress field in the user table was, by design, nullable, but some downstream applications didn't like nulls and wanted something there. Now, it didn't particularly matter what the values were, just that there were some, so Clara wrote up a quick stored procedure that would return the users with the null emails converted to the format BlankEmail_YYYYMMDD, e.g., BlankEmail_20230726.

This was all well and good until one of the senior developers decided that Clara's use of the T-SQL date functions was killing performance. They made this decision based on being the senior developer, but not based on any metrics or performance monitoring. Real seniors don't need such trivialities- they know what badly performing code is just by looking at it.

So that senior developer wrote this function, guaranteed to perform better.

CREATE FUNCTION [dbo].[fnGetDate_Formatted] (@Datetime DATETIME, @FormatMask VARCHAR(32))RETURNS VARCHAR(32)ASBEGIN DECLARE @StringDate VARCHAR(32) SET @StringDate = @FormatMask IF (CHARINDEX ('YYYY',@StringDate) > 0) SET @StringDate = REPLACE(@StringDate, 'YYYY', DATENAME(YY, @Datetime)) IF (CHARINDEX ('YY',@StringDate) > 0) SET @StringDate = REPLACE(@StringDate, 'YY', RIGHT(DATENAME(YY, @Datetime),2)) IF (CHARINDEX ('Month',@StringDate) > 0) SET @StringDate = REPLACE(@StringDate, 'Month', DATENAME(MM, @Datetime)) IF (CHARINDEX ('MON',@StringDate COLLATE SQL_Latin1_General_CP1_CS_AS)>0) SET @StringDate = REPLACE(@StringDate, 'MON', LEFT(UPPER(DATENAME(MM, @Datetime)),3)) IF (CHARINDEX ('Mon',@StringDate) > 0) SET @StringDate = REPLACE(@StringDate, 'Mon', LEFT(DATENAME(MM, @Datetime),3)) IF (CHARINDEX ('MM',@StringDate) > 0) SET @StringDate = REPLACE(@StringDate, 'MM', RIGHT('0'+CONVERT(VARCHAR,DATEPART(MM, @Datetime)),2)) IF (CHARINDEX ('M',@StringDate) > 0) SET @StringDate = REPLACE(@StringDate, 'M', CONVERT(VARCHAR,DATEPART(MM, @Datetime))) IF (CHARINDEX ('DD',@StringDate) > 0) SET @StringDate = REPLACE(@StringDate, 'DD', RIGHT('0'+DATENAME(DD, @Datetime),2)) IF (CHARINDEX ('D',@StringDate) > 0) SET @StringDate = REPLACE(@StringDate, 'D', DATENAME(DD, @Datetime)) RETURN @StringDateEND

This is a very traditional case of bad code: "we replace a built in function with our own, but badly". Because they're eschewing the built in CONVERT, they have to mix DATENAME calls (which return textual representations of dates) with DATEPART calls (which return numeric representations of dates). All the DATEPART calls need to get CONVERTed to text, and they also have to get padded back out.

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.

Now, you may be shocked to learn, the addition of this function did nothing to improve performance. In fact, performance got worse. Don't worry, the senior developer who wrote this function knew the root cause: something Clara did, obviously. And the senior dev set about trying to remove as much of Clara's code as possible, so that they could continue to "improve" the performance of the database.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!