IF....ELSE Statement Problem - SQL Server Forums

Please start any new threads on our new site at https://forums.sqlteam.com. We've got lots of great SQL Server experts to answer whatever question you can come up with.

All Forums

SQL Server 2000 Forums

Transact-SQL (2000)

IF....ELSE Statement Problem

Author

Topic

cwsrad
Starting Member

4 Posts

Posted - 2005-09-19 : 20:30:57

I have the following stored procedure:
CREATE PROCEDURE update_target @status varchar(1)=NULL,@trg_id numeric(10,0)=NULL, @mseq numeric(5,0)=NULL, @trg_type_code char(1)=NULL,@trg_sub_code varchar(2)=NULL, @alat numeric(6,4)=NULL, @alng numeric(7,4)=NULL, @tlat numeric(6,4)=NULL, @tlng numeric(7,4)=NULL, @trg_date varchar(12)=NULL, @trg_time varchar(2)=NULL, @range numeric(7,2)=NULL,@bearing numeric(5,2)=NULL, @comments varchar(800)=NULL, @altitude numeric(12,12)=NULL, @code as numeric(5,0)=NULL, @identified_code as numeric(5,0)=NULL,@photo as bit=NULL,@video as bit=NULL,@name varchar(35)=NULL, @side varchar(35)=Null, @callsign varchar(35)=Null, @activity varchar(35)=Null, @nation varchar(35)=Null, @vess_type varchar(35)=Null, @fishery_type varchar(35)=Null,@course varchar(35)=Null, @speed varchar(35)=Null AS
SET NOCOUNT ONSET ANSI_NULLS ON
IF @status IS NULL or @status = '' BEGIN --No target status code specified for INSERT, UPDATE, DELETE RETURN (1) END
ELSE --Check to ensure the target code does not already exist in SIS -- Code to ADD the new target
IF @status = 'A' SET @target_datetime = @trg_date + @trg_time IF EXISTS(select * from target where trg_id=@trg_id and mseq=@mseq) RETURN(2) ELSE BEGIN IF EXISTS(select code from target where mseq=@mseq) BEGIN SET @code = (select max(code) from target where mseq=@mseq) + 1 SET @trg_id = (select max(trg_id) from target where mseq=@mseq) + 1 SET @identified_code = @code END ELSE BEGIN SET @code = 1 SET @identified_code = 1 SET @trg_id = (select max(trg_id) from target where mseq=@mseq) + 1 END
INSERT INTO target (code, mseq,trg_id,trg_type_code,trg_sub_code,alat,alng,tlat,tlng,trg_date,range,bearing,comments,altitude) VALUES (@code, @mseq, @trg_id, @trg_type_code, @trg_sub_code,@alat,@alng,@tlat,@tlng,@target_datetime,@range,@bearing,@comments,@altitude)
INSERT INTO target_identified (mseq, trg_id, identified_code, code, detail1, detail2, detail3, detail4, detail5, detail6, detail7, detail8, detail9, photo, video, comments) VALUES (@mseq, @trg_id, @identified_code, @code, @name, @side, @callsign, @activity, @nation, @vess_type, @fishery_type, @course, @speed, @photo,@video, @comments) RETURN (0) END
-- Code to UPDATE a specified target BEGIN IF @status = 'U' IF EXISTS(select * from target where trg_id=@trg_id and mseq=@mseq) UPDATE target set code=@code, mseq=@mseq, trg_id=@trg_id, trg_type_code=@trg_type_code, trg_sub_code=@trg_sub_code, alat=@alat, alng=alng, tlat=@tlat, tlng=@tlng, trg_date=@target_datetime, range=@range, bearing=@bearing, comments=@comments, altitude=@altitude UPDATE target_identified set trg_id=@trg_id, mseq=@mseq, identified_code=@identified_code, code=@code, detail1=@name, detail2=@side, detail3=@callsign, detail4=@activity, detail5=@nation, detail6=@vess_type, detail7=@fishery_type, detail8=@course, detail9=@speed, photo=@photo, video=@video, comments=@comments RETURN (0) END
-- Code to DELETE a specified target BEGIN IF @status = 'D' IF EXISTS(select * from target where trg_id=@trg_id and mseq=@mseq) DELETE FROM photos where trg_id=@trg_id and mseq=@mseq DELETE FROM target_identified where trg_id=@trg_id and mseq=@mseq DELETE FROM target where trg_id=@trg_id and mseq=@mseq RETURN (0) END
--Check for SQL Server ErrorsIF @@ERROR <> 0 BEGIN RETURN (4) END
ELSE BEGIN --Successful delete RETURN (0) ENDGO
When I pass in a status (@status) code of 'U', then the first IF statement after "IF @status = 'A'"is evaluated where I would like the code to jump to next part of codeto evaulate the next status (IF @status = 'U') and so on until the proper conditionis met. However, no matter what value I pass in for @status, it stilltries to evaluate the same code everytime and ignores the other conditional statements.What am I doing wrong? I am using the debugger to pass in the parameters.

jen
Master Smack Fu Yak Hacker

4110 Posts

Posted - 2005-09-19 : 23:11:27

can you repost and format your code for easy reading? you have comments and some codes might be included there... some indentations will be nice too

--------------------
keeping it simple...

Kristen
Test

22859 Posts

Posted - 2005-09-19 : 23:22:54

Format your code, indent each BEGIN END consistently and put a BEGIN / END after EVERY IF statement. That way you will probably see that you've got


IF xxx
    UPDATE this
UPDATE that
...

where you actually meant to have


IF xxx
BEGIN
    UPDATE this
    UPDATE that
END
...

What are you expecting this to do?


IF @@ERROR <> 0 
BEGIN 
	RETURN (4) 
END
ELSE 
BEGIN --Successful delete 
	RETURN (0) 
END

There is no code path that I can see that can result in an @@ERROR being detected. All the code paths that COULD raise an @@ERROR bypass this exit route - plus @@ERROR only applies to the immediately preceding statement.

This


SET @code = (select max(code) from target where mseq=@mseq) + 1    
SET @trg_id = (select max(trg_id) from target where mseq=@mseq) + 1    
SET @identified_code = @code

can more efficiently be replaced by


SELECT	@code = max(code)  + 1,
	@identified_code = @code,
	@trg_id = max(trg_id) + 1
FROM	target
WHERE	mseq=@mseq

and I expect that you didn't mean this because it will update ALL rows in the table [target]


IF EXISTS(select * from target where trg_id=@trg_id and mseq=@mseq)  
    UPDATE target 
    set code=@code, mseq=@mseq, trg_id=@trg_id, trg_type_code=@trg_type_code, 
        trg_sub_code=@trg_sub_code, alat=@alat, alng=alng, tlat=@tlat, 
        tlng=@tlng, trg_date=@target_datetime, range=@range, bearing=@bearing, 
        comments=@comments, altitude=@altitude

but instead


UPDATE U 
set code=@code, mseq=@mseq, trg_id=@trg_id, trg_type_code=@trg_type_code, 
    trg_sub_code=@trg_sub_code, alat=@alat, alng=alng, tlat=@tlat, 
    tlng=@tlng, trg_date=@target_datetime, range=@range, bearing=@bearing, 
    comments=@comments, altitude=@altitude  
FROM target AS U
where trg_id=@trg_id and mseq=@mseq

and probably similarly for [target_identified]

You should prefix all your objects with the owner (probably "dbo.") as this will allow SQL Server to cache the query more reliably, which will improve performance.

Here's your code as I would have formatted it in case it helps you spot any logic flow that you had intended, but the indentation indicates won't happen!


CREATE PROCEDURE update_target 
    @status varchar(1)=NULL,
    @trg_id numeric(10,0)=NULL, 
    @mseq numeric(5,0)=NULL, 
    @trg_type_code char(1)=NULL,
    @trg_sub_code varchar(2)=NULL, 
    @alat numeric(6,4)=NULL, 
    @alng numeric(7,4)=NULL, 
    @tlat numeric(6,4)=NULL, 
    @tlng numeric(7,4)=NULL, 
    @trg_date varchar(12)=NULL, 
    @trg_time varchar(2)=NULL, 
    @range numeric(7,2)=NULL,
    @bearing numeric(5,2)=NULL, 
    @comments varchar(800)=NULL, 
    @altitude numeric(12,12)=NULL, 
    @code as numeric(5,0)=NULL, 
    @identified_code as numeric(5,0)=NULL,
    @photo as bit=NULL,
    @video as bit=NULL,
    @name varchar(35)=NULL, 
    @side varchar(35)=Null, 
    @callsign varchar(35)=Null, 
    @activity varchar(35)=Null, 
    @nation varchar(35)=Null, 
    @vess_type varchar(35)=Null, 
    @fishery_type varchar(35)=Null,
    @course varchar(35)=Null, 
    @speed varchar(35)=Null  
AS
SET NOCOUNT ON
SET ANSI_NULLS ON
IF @status IS NULL or @status = '' 
BEGIN
    -- No target status code specified for INSERT, UPDATE, DELETE 
    RETURN (1) 
END
ELSE  
    -- Check to ensure the target code does not already exist in SIS  
    -- Code to ADD the new target
    IF @status =  'A'   
        SET @target_datetime  = @trg_date + @trg_time   
IF EXISTS(select * from target where trg_id=@trg_id  and mseq=@mseq)  
    RETURN(2)    
ELSE
BEGIN   
    IF EXISTS(select code from target where mseq=@mseq)    
    BEGIN    
        SET @code = (select max(code) from target where mseq=@mseq) + 1    
        SET @trg_id = (select max(trg_id) from target where mseq=@mseq) + 1    
        SET @identified_code = @code    
    END   
    ELSE    
    BEGIN    
        SET @code = 1    
        SET @identified_code = 1    
        SET @trg_id = (select max(trg_id) from target where mseq=@mseq) + 1    
    END   
    INSERT INTO target (code, mseq,trg_id,trg_type_code,trg_sub_code,alat,alng,
            tlat,tlng,trg_date,range,bearing,comments,altitude)     
        VALUES (@code, @mseq, @trg_id, @trg_type_code, @trg_sub_code,@alat,@alng,
            @tlat,@tlng,@target_datetime,@range,@bearing,@comments,@altitude)
    INSERT INTO target_identified (mseq, trg_id, identified_code, code, detail1, 
            detail2, detail3, detail4, detail5, detail6, detail7, detail8, 
            detail9,  photo, video, comments)     
        VALUES (@mseq, @trg_id, @identified_code, @code, @name, 
            @side, @callsign, @activity, @nation, @vess_type, @fishery_type, @course, 
            @speed, @photo,@video, @comments)         
    RETURN (0)   
END 
 -- Code to UPDATE a specified target 
BEGIN 
    IF @status = 'U'  
        IF EXISTS(select * from target where trg_id=@trg_id and mseq=@mseq)  
            UPDATE target 
            set code=@code, mseq=@mseq, trg_id=@trg_id, trg_type_code=@trg_type_code, 
                trg_sub_code=@trg_sub_code, alat=@alat, alng=alng, tlat=@tlat, 
                tlng=@tlng, trg_date=@target_datetime, range=@range, bearing=@bearing, 
                comments=@comments, altitude=@altitude  
    UPDATE target_identified 
    set trg_id=@trg_id, mseq=@mseq, identified_code=@identified_code, code=@code, detail1=@name, 
            detail2=@side, detail3=@callsign, detail4=@activity, detail5=@nation, 
            detail6=@vess_type, detail7=@fishery_type, detail8=@course, detail9=@speed, 
            photo=@photo, video=@video, comments=@comments  
    RETURN (0) 
END
 -- Code to DELETE a specified target 
BEGIN 
    IF @status = 'D'    
        IF EXISTS(select * from target where trg_id=@trg_id  and mseq=@mseq)  
            DELETE FROM photos where trg_id=@trg_id and mseq=@mseq  
    DELETE FROM target_identified where trg_id=@trg_id and mseq=@mseq  
    DELETE FROM target where trg_id=@trg_id and mseq=@mseq  
    RETURN (0) 
END
--Check for SQL Server Errors
IF @@ERROR <> 0 
BEGIN 
    RETURN (4) 
END
ELSE 
BEGIN --Successful delete 
    RETURN (0) 
END
GO

Kristen

derrickleggett
Pointy Haired Yak DBA

4184 Posts

Posted - 2005-09-21 : 00:16:21

This thing is going to recompile about 200000 times every time you run it. You're thinking procedurally, instead of set-based. If you use CASE statements and analyze your INSERT, UPDATE, and DELETE statements properly, you could probably get rid of almost all this IF logic. Give it a try. If you need help, we can guide you along after the first attempt. Your code will run faster and handle larger loads if you think and write SQL the correct way.

MeanOldDBA
derrickleggett@hotmail.com

When life gives you a lemon, fire the DBA.

cwsrad
Starting Member

4 Posts

Posted - 2005-09-22 : 08:14:11

Thanks all...especially Kristen for the formatting and suggestions. I will keep this (formatting) in mind for future posts. Thanks as well, Derrick, I see your point and I have changed my code to use the CASE statement. Could you explain what you mean by "this thing is going to recompile 200,000 times."

Kristen
Test

22859 Posts

Posted - 2005-09-22 : 08:48:36

"this thing is going to recompile 200,000 times"

He's exaggerating - but maybe not by much

A stored procedure is "examined" when first run and a "plan" produced of how it will be executed - which index will be used, and so on. (There is a cost-based approach based on guestimates of how long each available index will take, and so on).

It can take a while to create that plan, so SQL caches it for next time.

However, depending on the way the Sproc is written it may decide that it cannot deduce a Plan that is good for all executions - e.g. if you have lots of IF's or temporary tables, etc.

Clearly if SQL Server CAN store the plan then it saved time next time the Sproc is run - so its A Good Thing, and you should try to make sure that your SProcs query plans get cached.

If SQL cannot reuse a cached query plan the SProc gets "recompiled"

Kristen

Subscribe to SQLTeam.com

SQLTeam.com Articles via RSS

SQLTeam.com Weblog via RSS

- Advertisement -

Resources