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.



  • @corgimonster said:

    written by a recently-departed contractor
     

    You... you killed him?



  • 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:

    @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.

    TRWTF is storing a date as a string, amirite?


  • @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 HH🇲🇲ss' 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:

    @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 HH🇲🇲ss' 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".

    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:

    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:

    @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.

    FTFM

Log in to reply