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…

 

What did you think of this article?




Trackbacks
  • No trackbacks exist for this entry.
Comments
  • No comments exist for this entry.
Leave a comment

Submitted comments will be subject to moderation before being displayed.

 Enter the above security code (required)

 Name

 Email (will not be published)

 Website

Your comment is 0 characters limited to 3000 characters.