4

I've just converted the following PHP code to a MySQL stored procedure. There is no obvious syntax error as I could execute it with PHPMyAdmin. I can see it with

SELECT routine_definition
FROM information_schema.routines
WHERE routine_schema = 'chess';

As this is the first time I've written a stored procedure, I would like to know

  • Does the stored procedure do what I think it does? (See flowchart in "What it should do")
  • Is the stored procedure plain SQL (to some standard) or will I only be able to use this with MySQL databases? What is MySQL specific? Could I get rid of that?
  • Is there any way I can improve this stored procedure? Are there best practices that I broke?
  • Do I have to sanitize input data when I use stored procedures?

Here is a short overview over the database and all code. But I hope this is not necessary to answer my questions.

What it should do

enter image description here

New stored procedure

DELIMITER //
CREATE PROCEDURE ChallengeUser(
   IN challengedUserID INT,
   IN currentUserID INT,
   OUT startedGamePlayerUsername varchar(255),
   OUT startedGameID INT,
   OUT incorrectID BIT,
   OUT alreadyChallengedPlayer BIT,
   OUT alreadyChallengedGameID INT
  )
    BEGIN
        SELECT `username` AS startedGamePlayerUsername FROM chess_users
          WHERE `user_id` = challengedUserID 
               AND `user_id` != currentUserID LIMIT 1;

        IF startedGamePlayerUsername IS NOT NULL THEN
            SELECT `id` FROM `chess_games` 
              WHERE `whiteUserID` = currentUserID 
                AND `blackUserID` = challengedUserID 
                AND `outcome` = -1 LIMIT 1;
            IF id IS NULL THEN
                SELECT `softwareID` AS `whitePlayerSoftwareID` 
                  FROM chess_users 
                WHERE `user_id`=currentUserID LIMIT 1;
                SELECT `softwareID` AS `blackPlayerSoftwareID` 
                  FROM chess_users 
                  WHERE `user_id`=challengedUserID LIMIT 1;
                INSERT INTO `chess_games` (`tournamentID`, `whiteUserID`, 
                                  `blackUserID`, `whitePlayerSoftwareID`, 
                                     `blackPlayerSoftwareID`, `moveList`) 
                    VALUES (NULL, currentUserID, challengedUserID, 
                            whitePlayerSoftwareID, 
                            blackPlayerSoftwareID, "");

                /* Get the id of the just inserted tuple */
                SELECT `id` AS startedGameID FROM chess_games 
                  WHERE `whiteUserID` = whitePlayerSoftwareID 
                    AND `blackUserID` = blackPlayerSoftwareID 
                    AND `whitePlayerSoftwareID` = whitePlayerSoftwareID 
                    AND `blackPlayerSoftwareID` = blackPlayerSoftwareID 
                    AND `moveList` = "" LIMIT 1;
            ELSE
                SET alreadyChallengedPlayer = 1;
                SET alreadyChallengedGameID = id;
            END IF;
        ELSE
            SET incorrectID = 1;
        END IF;
    END //
DELIMITER ;

New PHP code

function challengeUser2($user_id, $t) {
    global $conn;
    $stmt = $conn->prepare("CALL ChallengeUser(?,?,@startedGamePlayerUsername,".
                       ."@startedGameID,@incorrectID,"
                       ."@alreadyChallengedPlayer,@alreadyChallengedGameID)");
    $test = USER_ID;
    $stmt->bindParam(1, $user_id);
    $stmt->bindParam(2, $test);
    $returnValue = $stmt->execute();

    echo "Return Value\n";
    print_r($returnValue);
    echo "################\n\nstmt\n";
    print_r($stmt);

    echo "################\n\nrow\n";
    $row = $stmt->fetch(PDO::FETCH_ASSOC);
    print_r($row);
}

What it prints out

Return Value
1################

stmt
PDOStatement Object
(
    [queryString] => CALL ChallengeUser(?,?,@startedGamePlayerUsername,
       @startedGameID,@incorrectID,
       @alreadyChallengedPlayer,@alreadyChallengedGameID)
)
################

row
Array
(
    [startedGamePlayerUsername] => test
)

What it should do

It should have created a new entry in the table chess_games. But there is no new entry and there is no value for incorrectID or alreadyChallengedPlayer. So I think I made a mistake.

Martin Thoma
  • 124,992
  • 159
  • 614
  • 958
  • don't you need to specify the input/output parameters ?, look at the examples [here](http://php.net/manual/en/pdo.prepared-statements.php) they are bit different from how your calling it. – DevZer0 Jul 17 '13 at 12:46
  • @DevZer0: I think I did that. But I am new to stored procedures, so I might have made a mistake. Is my stored procedure wrong or my PHP code that calls this procedure? – Martin Thoma Jul 17 '13 at 12:48
  • i am not experienced with SP. but your SP looks logical, but the calling php code looks some what different from the examples in [here](http://php.net/manual/en/pdo.prepared-statements.php) – DevZer0 Jul 17 '13 at 12:49
  • first of all You shoud use `FUNCTION` instead of `PROCEDURE` to use `IN/OUT` – jaczes Jul 17 '13 at 13:07
  • @jaczes: According to [this answer](http://stackoverflow.com/a/1179778/562769) functions cannot `INSERT`. But I definitely need that. – Martin Thoma Jul 17 '13 at 13:12
  • @moose, could You provide some simple data ? It would be helpful :) I'm working on Your procedure and wanted to check before posting it.... – jaczes Jul 17 '13 at 13:16
  • The complete project is [on GitHub](https://github.com/MartinThoma/community-chess). To get it work, you only have to adjust the database connection string in `wrapper.inc.php` (lines 23-26), execute the SQL-file `install/chess.sql` to generate the database. Then you can login as user `abc` with password `abc`. Click on "Challenge" in the menu at the left side, click on "Challenge test". Now the function `challengeUser(...)` in file `additional.inc.php` will be executed. You can replace this function with your new code. – Martin Thoma Jul 17 '13 at 13:26
  • @jaczes: The [image of the database](https://raw.github.com/MartinThoma/community-chess/master/documentation/chess-database-schema.png) might also be useful. Here are links to [additional.inc.php](https://github.com/MartinThoma/community-chess/blob/master/additional.inc.php#L33) and [wrapper.inc.php](https://github.com/MartinThoma/community-chess/blob/master/wrapper.inc.php#L22) – Martin Thoma Jul 17 '13 at 13:39

1 Answers1

0

Sorry for such messy code - i'm at work = no much time, but should help You. You have to add data into tables: USERS and SOFTWARE. There was problem with NULL handling, and passing query result into variable.

EDIT: fix for query "get the id of the just inserted tuple"

DELIMITER $$

DROP PROCEDURE IF EXISTS `ChallengeUser`$$

CREATE DEFINER=`root`@`localhost` PROCEDURE `ChallengeUser`(
challengedUserID INT,
currentUserID INT,
startedGamePlayerUsername  VARCHAR(255),
startedGameID INT,
incorrectID INT,
alreadyChallengedPlayer INT,
alreadyChallengedGameID INT
)
BEGIN

DECLARE TMP_ID INT DEFAULT 0;
DECLARE TMP_W_PLAYER INT DEFAULT 0;    
DECLARE TMP_B_PLAYER INT DEFAULT 0;


    SELECT `username` INTO startedGamePlayerUsername 
      FROM chess_users
    WHERE `user_id` = challengedUserID 
      AND `user_id` != currentUserID LIMIT 1;

    IF startedGamePlayerUsername IS NOT NULL THEN

        SELECT `id` INTO TMP_ID 
          FROM `chess_games` 
        WHERE `whiteUserID` = currentUserID 
            AND `blackUserID` = challengedUserID 
            AND `outcome` = -1 
        LIMIT 1;

        -- here was bad NULL handling    
        IF TMP_ID IS NULL OR TMP_ID='' THEN

            SELECT `softwareID` INTO TMP_W_PLAYER
              FROM chess_users 
            WHERE `user_id`=currentUserID 
            LIMIT 1;

            SELECT `softwareID` INTO TMP_B_PLAYER 
              FROM chess_users 
            WHERE `user_id`=challengedUserID 
            LIMIT 1;

            INSERT INTO `chess_games` 
        (`tournamentID`, `whiteUserID`,`blackUserID`, `whitePlayerSoftwareID`,`blackPlayerSoftwareID`, `moveList`)
                     SELECT NULL, currentUserID, challengedUserID, TMP_W_PLAYER, TMP_B_PLAYER, "";           


            /* Get the id of the just inserted tuple */
            SELECT max(`id`) INTO startedGameID FROM chess_games 
              WHERE `whiteUserID` = currentUserID 
                AND `blackUserID` = challengedUserID 
                AND `whitePlayerSoftwareID` = TMP_W_PLAYER 
                AND `blackPlayerSoftwareID` = TMP_B_PLAYER
                AND `moveList` = "";
        ELSE
            SET alreadyChallengedPlayer = 1;
            SET alreadyChallengedGameID = TMP_ID;
        END IF;
    ELSE
        SET incorrectID = 1;
    END IF;

    SELECT startedGamePlayerUsername,startedGameID,   incorrectID ,   alreadyChallengedPlayer ,   alreadyChallengedGameID;

END$$

DELIMITER ;
jaczes
  • 1,366
  • 2
  • 8
  • 16