tags:

views:

5285

answers:

9

Not a SQL person at all. Have the following code a consultant wrote.

First, it makes sure only an elementary school has been chosen - then, after the BEGIN, if the variable @Term equals a 3 we want to do the stuff under that IF statement. Here's the problem. When @Term is not = 3 we still want to drop down and do the SECOND INSERT INTO @Classes part. FYI - the Term is = 3 when this is being run, but it's not doing both INSERT's - should there be an END IF at the end of that "IF @Term = 3" section instead of just a plain END?

IF @SchoolCategoryCode = 'Elem' 

--- We now have determined we are processing an elementary school...

BEGIN

---- Only do the following if the variable @Term equals a 3 - if it does not, skip just this first part

    IF @Term = 3

    BEGIN

    INSERT INTO @Classes

    SELECT   
     XXXXXX 
    FROM XXXX blah blah blah

    END   <----(Should this be ENDIF?)

---- **always** "fall thru" to here, no matter what @Term is equal to - always do the following INSERT for all elementary schools

    INSERT INTO @Classes    
    SELECT
    XXXXXXXX 
    FROM XXXXXX (more code) 

END
+1  A: 

If this is MS Sql Server then what you have should work fine... In fact, technically, you don;t need the Begin & End at all, snce there's only one statement in the begin-End Block... (I assume @Classes is a table variable ?)

If @Term = 3
   INSERT INTO @Classes
    SELECT                  XXXXXX  
     FROM XXXX blah blah blah
-- -----------------------------

 -- This next should always run, if the first code did not throw an exception... 
 INSERT INTO @Classes    
 SELECT XXXXXXXX        
 FROM XXXXXX (more code)
Charles Bretana
Can i ask what is offensive about this answer?? WHo did this ?
Charles Bretana
A: 

If I remember correctly, and more often then not I do ... there is no END IF support in Transact-Sql. The BEGIN and END should do the job. Are you getting errors?

mattruma
A: 

Based on your description of what you want to do, the code seems to be correct as it is. ENDIF isn't a valid SQL loop control keyword. Are you sure that the INSERTS are actually pulling data to put into @Classes? In fact, if it was bad it just wouldn't run.

What you might want to try is to put a few PRINT statements in there. Put a PRINT above each of the INSERTS just outputting some silly text to show that that line is executing. If you get both outputs, then your SELECT...INSERT... is suspect. You could also just do the SELECT in place of the PRINT (that is, without the INSERT) and see exactly what data is being pulled.

Michael Bray
+2  A: 

There is no ENDIF in SQL.

The statement directly followig an IF is execute only when the if expression is true.

The BEGIN ... END construct is separate from the IF. It binds multiple statements together as a block that can be treated as if they were a single statement. Hence BEGIN ... END can be used directly after an IF and thus the whole block of code in the BEGIN .... END sequence will be either executed or skipped.

In your case I suspect the "(more code)" following FROM XXXXX is where your problem is.

AnthonyWJones
+1  A: 

It has to do with the Normal Form for the SQL language. IF statements can, by definition, only take a single SQL statement. However, there is a special kind of SQL statement which can contain multiple SQL statements, the BEGIN-END block.

If you omit the begin-end block, your SQL will run fine, but it will only execute the first statement as part of the IF.

Basically, this:

IF @Term = 3
    INSERT INTO @Classes
    SELECT              
        XXXXXX  
    FROM XXXX blah blah blah

is equivalent to the same thing with the BEGIN-END block, because you're only executing a single statement. However, for the same reason that not including the curly-braces on an if-statement in a C-like language is a bad idea, it is always preferable to use BEGIN and END.

foxxtrot
A: 

Off hand the code looks right. What if you try using an 'Else' and see what happens?

IF @SchoolCategoryCode = 'Elem' 

--- We now have determined we are processing an elementary school...

BEGIN

---- Only do the following if the variable @Term equals a 3 - if it does not, skip just this first part

    IF @Term = 3
    BEGIN
     INSERT INTO @Classes

     SELECT              
      XXXXXX  
     FROM XXXX blah blah blah

     INSERT INTO @Classes    
     SELECT
     XXXXXXXX    
     FROM XXXXXX (more code) 
    END   <----(Should this be ENDIF?)
    ELSE
    BEGIN


     INSERT INTO @Classes    
     SELECT
     XXXXXXXX    
     FROM XXXXXX (more code) 
    END
END
SquidScareMe
Me again. I can't add an "ELSE". Because, I always want to do the 2nd INSERT statement whether or not @Term = 3
+1  A: 

The only time the second insert into @clases should fail to fire is if an error occurred in the first insert statement.

If that's the case, then you need to decide if the second statement should run prior to the first OR if you need a transaction in order to perform a rollback.

Chris Lively
A: 

You could also rewrite the code to remove the nested 'If' statement completely.

INSERT INTO @Classes    
SELECT XXXXXX      
FROM XXXX 
Where @Term = 3   

---- **always** "fall thru" to here, no matter what @Term is equal to - always do
---- the following INSERT for all elementary schools    
INSERT INTO @Classes        
SELECT    XXXXXXXX        
FROM XXXXXX (more code)
Good thinking. The next logic step would be to reduce this to one insert with a UNION ALL select.
G Mastros
A: 

Frankly this seems like something that should be on the application layer, not the database layer...

Andrew G. Johnson