It's NULL, so let's convert it to VARCHAR(12)
-
In the following stored procedure snippet (written by a recently-departed contractor), @PayeeID is an INT parameter to the stored procedure.
DECLARE @ErrorString NVARCHAR(max)
IF @PayeeID IS NULL
BEGIN
SET @ErrorString = 'Invalid Payee ID ' + CONVERT(varchar(12),@PayeeID);
RAISERROR(@ErrorString, 11, 1);
END
Really? We needed that CONVERT in there? You couldn't just write an error message that says that the parameter was null?
-
I concur. Converting the NULL to VARCHAR(25) is much safer.
-
-
Purely justifiable, IMO.
-
@dhromed said:
@corgimonster said:
written by a recently-departed contractor
You... you killed him?
I'm not generally that violent. One of the other full-time developers here, who has to deal with this guy's code artifacts a lot more than I do, might be more inclined. Some of the contractor's abuses of CURSORs that grind the database to a halt might warrant the ultimate punishment.
-
You can have payments without a payee? Well, THAT explains what happened to my bank account balance...
-
Not as bad as getting deprecated and decapitated mixed up.
-
@corgimonster said:
Really? We needed that CONVERT in there?
Hey, some people are CONVERT-happy. Just a couple weeks ago I was looking at a slow SQL (~15 seconds). Somebody was selecting on a date range:
AND CONVERT(VARCHAR(10),trans_date,121) >= @startDate AND CONVERT(VARCHAR(10),trans_date,121) <= @endDate
So it seems that by converting every value in that column to a string, it forces a full scan of the index (or table), rather than just selecting the range intended. That's my assumption, anyway, as once we removed the CONVERT, the query came back instantly.
-
@boog said:
TRWTF is storing a date as a string, amirite?@corgimonster said:
Really? We needed that CONVERT in there?
Hey, some people are CONVERT-happy. Just a couple weeks ago I was looking at a slow SQL (~15 seconds). Somebody was selecting on a date range:
AND CONVERT(VARCHAR(10),trans_date,121) >= @startDate AND CONVERT(VARCHAR(10),trans_date,121) <= @endDate
So it seems that by converting every value in that column to a string, it forces a full scan of the index (or table), rather than just selecting the range intended. That's my assumption, anyway, as once we removed the CONVERT, the query came back instantly.
-
@Sutherlands said:
TRWTF is storing a date as a string, amirite?
there's nothing indicating that they did that, it's just that DATE types are (at least in MySQL) accessible/represented through "string format", you provide 'YYYY-MM-DD HHss' string to a DATE value (probably just timestamp with flag telling SQL engine to do this), and vice versa, it just seems they didn't know about this/didn't trust the implicit "conversion".
-
@SEMI-HYBRID code said:
So you're saying it was a date, they implicitly cast it to a string, and then explicitly cast it to a date again?@Sutherlands said:
TRWTF is storing a date as a string, amirite?
there's nothing indicating that they did that, it's just that DATE types are (at least in MySQL) accessible/represented through "string format", you provide 'YYYY-MM-DD HHss' string to a DATE value (probably just timestamp with flag telling SQL engine to do this), and vice versa, it just seems they didn't know about this/didn't trust the implicit "conversion".
-
@Sutherlands said:
So you're saying it was a date, they implicitly cast it to a string, and then explicitly cast it to a date again?
No, trans_date is a date. Calling CONVERT changed it to a string. This conversion was unnecessary, and resulted in lower performance since CONVERT had to be called on all rows before the comparison could take place (forcing a full scan).Without the CONVERT, it's a simple date comparison, which uses the index to jump straight to the range we're selecting.
There were no dates stored as strings, thankfully.
-
@boog said:
FTFM@Sutherlands said:
So you're saying it was a date, they explicitly cast it to a string, and then implicitly cast it to a date again?
No, trans_date is a date. Calling CONVERT changed it to a string. This conversion was unnecessary, and resulted in lower performance since CONVERT had to be called on all rows before the comparison could take place (forcing a full scan).Without the CONVERT, it's a simple date comparison, which uses the index to jump straight to the range we're selecting.
There were no dates stored as strings, thankfully.