SQL Dumbass

Fighting dumbasses, one query at a time…

Is Code Review Necessary?

I was reviewing procedures from one of our dev teams today when I saw this (names to protect someone and there were a bunch more parameters, and yeah, I doubt this, as typed would compile, work with me):


CREATE PROC
@PK int
@Val1 varchar(50) OUTPUT
@Val2 datetime OUTPUT
@Val3 int OUTPUT
AS


IF EXISTS (SELECT 1 FROM Table1 WHERE pk = @PK)
BEGIN
                SELECT @Val1 = Val1 FROM Table1 WHERE PK = @PK
                SELECT @Val2 = Val2 FROM Table 1 WHERE PK = @PK
                SELECT @Val3 = Val3 FROM Table 1 WHERE PK = @PK
END
ELSE
BEGIN
                RAISERROR(‘We don’t have any rows that match that pk’)
END
 
If I could guarantee a jury of DBA’s, I know I’d be acquitted… Instead, I rewrote it and tried to educate them
 
SELECT @Val1 = Val1
                ,@val2 = Val2
                ,@val3 = Val3
FROM Table1
                WHERE PK = @PK
                And Val3< > 42
 
IF @@ROWCOUNT = 0
                RAISERROR(‘We don’t have any rows that match that pk’)
END
 
The real query went from more than a second to less than 50ms and the reads against the table went from several hundred to 2. All that and I still have to justify to developers why they send their code through for review…