CODEWITHSUNDEEP
mysql
user1844205
user1844205

Reputation: 141

Stored procedure go to infinite loop

I am making a stored procedure in order to check that whether a user exists or not. The problem is whenever I pass it the correct username, it executes successfully but when I pass wrong username, it go through an infinite loop.

Where am I going wrong? Here is my Stored Procedure:

CREATE PROCEDURE `VerifyUserNPass`(userParam varchar(50), out result int)
BEGIN
    DECLARE done INT DEFAULT FALSE;
    DECLARE tempUser varchar(50) default '';
    DECLARE count int default 0;
    DECLARE noRows int;

    DECLARE userList cursor for select userName from users;
    DECLARE CONTINUE HANDLER FOR NOT FOUND SET done = TRUE;

    select count(*) into noRows from users;

    OPEN userList;
    read_loop: LOOP
        FETCH userList into tempUser;
        IF tempUser = userParam THEN
                SET @count = count + 1;
                LEAVE read_loop;
        ELSEIF count > noRows THEN
                LEAVE read_loop;
        END IF;

    END LOOP;
    CLOSE userList;

    select count into result;

END

Upvotes: 1

Views: 3046

Answers (2)

spencer7593
spencer7593

Reputation: 108400

To fix your code, use a conditional test on the done variable to determine whether to leave the loop. (The done variable is initialized to FALSE, and then set to TRUE in the CONTINUE HANDLER when there are no more rows.)

Get rid of the query "count(*)" query, and the noRows variable; those are not needed. (In a concurrent system, it's possible for that count(*) query to return a value that differs from the number of rows returned by a later query. (Consider that its possible for other sessions to insert or delete rows while your procedure is running.)

Also get rid of the references to the @count user variable. You have a mixture of references, to both a user variable @coount and a procedure variable count. These are independent variables. There's no need for your procedure to use a user variable. Instead, stick with using variables that are declared in your procedure. (Save the user variables for when you have a need.)

-- select count(*) into noRows from users;
read_loop: LOOP
    FETCH userList into tempUser;
    IF done THEN
       LEAVE read_loop;
    END IF;
    IF tempUser = userParam THEN
        SET count = 1;
        LEAVE read_loop;
    END IF;
END LOOP;

A more efficient way to code this would be to let the database find the row(s) you are interested in, by adding a WHERE clause on your query. (There's no need for you to fetch rows that aren't going to match the condition you are interested in.)

Modify your cursor definition to include a predicate (i.e. a condition in a WHERE clause) to limit the rows returned: 

DECLARE userList cursor for select userName from users
  WHERE userName = userParam LIMIT 1;

I don't understand the need for a procedure here. A native SQL statement would be much more efficient, e.g.

SELECT 1 AS found FROM users u WHERE u.userName = 'foo' LIMIT 1;

Upvotes: 1

rene
rene

Reputation: 42444

Make sure you increment your loop counter for all branches not only in the succes branch and have result variable to hold your outcome.

DECLARE userFound int default 0;
......
read_loop: LOOP
    FETCH userList into tempUser;
    SET @count = @count + 1;
    IF tempUser = userParam THEN
           SET @userFound = 1
           LEAVE read_loop;
    ELSEIF count > noRows THEN
            LEAVE read_loop;
    END IF;
    .....
    select userFound into result;

And I would expect that this would return the same result (but I'm not an MySql expert) 

CREATE PROCEDURE `VerifyUserNPass`(userParam varchar(50), out result int)
BEGIN
 select COUNT(userName) into result from users where username = @userParam
END

Upvotes: 0

Related Questions