Quick Quiz on Code Style

[LNBruno]

Before I attempt an update or delete, I always check to see if the primary key exists and, if not, return a message to that effect. For example:

IF EXISTS (SELECT PrimaryKey FROM dbo.TableName WHERE PrimaryKey = @InputParam)
   --transaction wrapper removed for brevity...
    BEGIN
        UPDATE dbo.TableName 
            SET Column1  = @NewColumn1Value
            , Column2    = @NewColumn2Value
            , AndSoForth = @NewAndSoForthValue
        WHERE PrimaryKey = @InputParam
    END
ELSE
    BEGIN
        SET @vcException = 'No record found in table dbo.TableName with a primary key value of ' + RTRIM(CONVERT(varchar(15), @InputParam)) + '.'
        RETURN
    END

Being a detail-oriented, anal-retentive with a complusion for consistency, I would like to use the same construct for the SELECTs as well, but I just can't ignore the simplicity of this (not to mention one less call):

SELECT 
    PrimaryKey 
    , Column1
    , Column2
    , AndSoForth
FROM dbo.TableName 
WHERE PrimaryKey = @InputParam

IF @@ROWCOUNT = 0
    BEGIN
        SET @vcExceptionMessage = 'No record found in table dbo.TableName with a primary key value of ' + RTRIM(CONVERT(varchar(15), @InputParam)) + '.'
        RETURN
    END

So the question is, which would you choose and why? Consistent implementation or fewer lines of code? I'm so torn! ;-)

< M!ke >

 

[AlexCuse]

I take it @vcExceptionMessage is what you will be returning to your application, and you want to just show that no record was returned so no one thinks its' broken?

I would use if exists, because if this check fails, then it actually prevents you from running the select (the select will probably be more costly than the if exists).

Hope this helps,

Alex

Ignorance of certain subjects is a great part of wisdom

 

[LNBruno]

Thanks for the fast response, Alex.

Yes, @vcExceptionMessage is returned to the app (had it as @vcException in the first example - perils of copy/paste/tweak!).

I was thinking the same thing, but if it DOES exist, then I've executed 2 SELECTS instead of one... even though the IF EXISTS doesn't have the same overhead as the full-blown SELECT.

I feel like Two-Face from Batman...always of two minds about everything!

< M!ke >

 

[SQLDenis]

If would do this

return sign(@@rowcount) -1

if -1 is returned nothing is found, if 0 or greater is returned rows have been found. the application knows what PK was passed in (@InputParam) and also what table it is supposed to hit, there is no sense in clogging your pipes with text like this

'No record found in table dbo.TableName with a primary key value of ' + RTRIM(CONVERT(varchar(15), @InputParam)) + '.'

That is about 70 bytes, if you have 1000s of hits a minute that is a lot of bytes and also unneccesary processing by checking and converting the mnessage itself



Denis The SQL Menace
--------------------
SQL Server Code,Tips and Tricks, Performance Tuning
Google Interview Questions

 

[LNBruno]

Good point, Denis! I'll have to check with the powers that be to see if display messaging can be pushed back to the UI level, but your arguement is spot on!

< M!ke >

 

[Lamprey13]

Another option is to do your update like you do your select:

Instead of:

IF EXISTS (SELECT PrimaryKey FROM dbo.TableName WHERE PrimaryKey = @InputParam)
   --transaction wrapper removed for brevity...
    BEGIN
        UPDATE dbo.TableName 
            SET Column1  = @NewColumn1Value
            , Column2    = @NewColumn2Value
            , AndSoForth = @NewAndSoForthValue
        WHERE PrimaryKey = @InputParam
    END
ELSE
    BEGIN
        SET @vcException = 'No record found in table dbo.TableName with a primary key value of ' + RTRIM(CONVERT(varchar(15), @InputParam)) + '.'
        RETURN
    END

You could do it like your select, for example:

UPDATE dbo.TableName 
	SET Column1  = @NewColumn1Value
	, Column2    = @NewColumn2Value
	, AndSoForth = @NewAndSoForthValue
WHERE PrimaryKey = @InputParam

IF @@ROWCOUNT = 0
BEGIN
	SET @vcExceptionMessage = 'No record found in table dbo.TableName with a primary key value of ' + RTRIM(CONVERT(varchar(15), @InputParam)) + '.'
	RETURN
END

Which is probably more efficient since you don't have to check the table then update it.

Cheers

-Ryan

 

[LNBruno]

Um, no.

If I tried to update a non-existent record (primary key NOT on table), I'd get an error. That's the point of checking it first.

I do have (inside the transaction) a check for @@ROWCOUNT > 1 so that if for some bizarre reason I have more than one record with the same primary key (and believe me, it HAS happened!), I can rollback the transaction without committing the UPDATE.

Thanks, though.

< M!ke >

 

[nice95gle]

I think you can chalk this one up to preference. Which ever way make you more comfortable? Both techniques listed will just do a PK scan and move on. If you where going to return multiple row I would suggest IF EXSITS for the reasons we all know (OK for the one that don't know; IF EXSITS will stop processing once the first record that meets the criteria is found, which can give a little better performance). But for the primary key this is not an issue. So you answer your question I would go with IF EXISTS because IMO I find it a little easier to debug.

Well Done is better than well said
- Ben Franklin

 

[Lamprey13]

Have you tried to update a non-existent record? Because it does NOT generate an error.

 

[LNBruno]

You're right, of course Lamprey. It just doesn't do anything -- which still isn't an acceptable processing behavior for the overall application.

Another reason to check before attempting...



< M!ke >