jhon smith
jhon smith

Reputation: 33

stored procedure with if else sql server 2008

I have 3 tables, I have to check if grandparent table id has records in grandchildren table. If yes, return yes, else return no. Here is my stored procedure. I got an incorrect syntax error. I am new to stored procedure. Please help me.

CREATE PROCEDURE P_Check
   @PKG_ID INT,
   @S_ID INT,
   @FLAG VCHAR(10) OUT

   DECLARE IDS CURSOR LOCAL FOR SELECT S_ID FROM T1 WHERE P_ID = @PKG_ID
   OPEN IDS
   FETCH NEXT FROM IDS into @S_ID

   WHILE @@FETCH_STATUS = 0
   BEGIN
      SELECT * FROM T2 WHERE S_ID = @S_ID

      IF @@ROWCOUNT<>0
         @FLAG = 'YES'
         RETURN 
      ELSE
         @FLAG = 'NO'

      FETCH NEXT FROM IDS into @S_ID
   END

   CLOSE IDS
   DEALLOCATE IDS

Upvotes: 3

Views: 62112

Answers (2)

marc_s
marc_s

Reputation: 754268

This is just way too complicated and using a cursor here is totally not needed and totally unnecessary.

Simplify your procedure to be:

CREATE PROCEDURE P_Check
   @PKG_ID INT,
   @S_ID INT,
   @FLAG CHAR(1) OUT
AS BEGIN
   IF EXISTS (SELECT * FROM T2 
              INNER JOIN T1 ON T2.S_ID = T1.S_ID WHERE P_ID = @PKG_ID)
      SET @FLAG = 'Y'
   ELSE
      SET @FLAG = 'N'
END

When working seriously with SQL Server, you need to get away from the procedural row-by-agonizing-row thinking using cursors and loops, and you need to start thinking in sets to be efficient and productive.

Upvotes: 3

von v.
von v.

Reputation: 17108

A few things to check:

  1. I don't think there is a vchar data type in SQL Server unless that is your custom type. So change it to varchar
  2. You forgot the AS
  3. You might want to enclose your logic inside if in between begin and end

Your code that can be compiled:

CREATE PROCEDURE P_Check
    @PKG_ID INT,
    @S_ID INT,
    @FLAG VARCHAR(10) OUT
AS
    DECLARE IDS CURSOR LOCAL FOR SELECT S_ID FROM T1 WHERE P_ID = @PKG_ID
    OPEN IDS
    FETCH NEXT FROM IDS into @S_ID
    WHILE @@FETCH_STATUS = 0

    BEGIN
      SELECT * FROM T2 WHERE S_ID = @S_ID
      IF @@ROWCOUNT<>0
      BEGIN
         SET @FLAG = 'YES'
         RETURN 
      END
      ELSE
      BEGIN
         SET @FLAG = 'NO'
         FETCH NEXT FROM IDS into @S_ID
      END
    END

    CLOSE IDS
    DEALLOCATE IDS

However, I think your cursor will not be closed as you are returning here IF @@ROWCOUNT<>0. I think what you should do is change this:

IF @@ROWCOUNT<>0
BEGIN
    SET @FLAG = 'YES'
    RETURN 
END

to this:

IF @@ROWCOUNT<>0
BEGIN
    SET @FLAG = 'YES'
    GOTO ON_EXIT
END

then end your procedure like this:

ON_EXIT:
    CLOSE IDS
    DEALLOCATE IDS    

Then to answer your question in the comment, you are "returning" it already in a sense. You can call and test your procedure like this:

declare @result varchar(10)
exec P_Check 1, 1, @result out
print @result

Upvotes: 4

Related Questions