CODEWITHSUNDEEP
mysqlstored-procedures
43rdworld
43rdworld

Reputation: 97

Creating a MySQL stored procedure to update records

I'm converting all of my existing MSSQL databases and stored procedures am stuck on a new stored procedure where I need to update an existing record. The procedure gets called from a web form once a record has been inserted into the database and en email sent successfully (or at least passed off to the SMTP server)

I've had a working procedure in MSSQL for a long time but am trying to convert it to MySQL. I'm passing in 3 variables - a bit indicating the email got sent, a string indicating which SMTP server has been used to sent the email and a unique record id so I'll know what record to update. I'm also adding the date and time to another field to know when the procedure ran.

I've got the following but keep getting an error "#1064 - You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 7 - yet I don't see anything off at line 7 - at least to my eye.

The code I'm trying to use is:

CREATE PROCEDURE `sp_Test`(
  `emailSent_In` BIGINT UNSIGNED,
  `emailTransport_In` VARCHAR(100),
  `formSecret_In` VARCHAR(32)
)
BEGIN
  SET @`query` := CONCAT('UPDATE ',`tbl_JustSayThanks`,' 
  SET `emailSent` = `emailSent_In`,
  `emailTransport` = ',`emailTransport_In`,',
`emailSentDate` = NOW()
    WHERE `formSecret` = ', `formSecret_In`, '');
  PREPARE `stmt` FROM @`query`;
  EXECUTE `stmt`;
       @`query` := NULL;
  DEALLOCATE PREPARE `stmt`;
END//
DELIMITER ;

Just FYI, I'm using the CONCAT based on a previous answer I received from wchiquito and will be passing in the table name eventually. But, I wanted to get it to work on a simplified level before going there.

Upvotes: 3

Views: 33964

Answers (1)

Bill Karwin
Bill Karwin

Reputation: 562260

The following is wrong:

SET @`query` := CONCAT('UPDATE ',`tbl_JustSayThanks`,'

because you seem to be concatenating your SQL text with the value of tbl_JustSayThanks, but I think you mean to use the identifier itself. This should therefore be:

SET @`query` := CONCAT('UPDATE `tbl_JustSayThanks`',

The following is wrong:

`emailTransport` = ',`emailTransport_In`,',

because the variable is a VARCHAR but you don't quote it as a string literal in your SQL statement. It's easy to get mixed up with the multiple levels of quoting. It should be:

`emailTransport` = ''', `emailTransport_In`, ''',

The following is wrong for the same reason:

WHERE `formSecret` = ', `formSecret_In`, '');

it should be:

WHERE `formSecret` = ''', `formSecret_In`, '''');

This still suffers from SQL injection problems, unless you can guarantee that the input parameters are safe (which is not a good assumption). If you need to concatenate values into your SQL expressions, you should use the QUOTE() function to do escaping:

SET @query = CONCAT('
  UPDATE tbl_JustSayThanks 
  SET emailSent = ', QUOTE(emailSent_In), '
      emailTransport = ', QUOTE(emailTransport_In), '
      emailSentDate = NOW()
  WHERE formSecret = ', QUOTE(formSecret_In));

More comments:

  • You don't need to delimit every identifier with back-ticks, only those that conflict with SQL reserved words, or contain whitespace or punctuation or international characters. None of your identifiers you show require delimiting.
  • When you use prepared statements, you should use query parameters with the ? placeholders, intead of concatenating variables into the SQL string. You don't quote parameter placeholders in your SQL query. That way you won't run into hard-to-debug syntax errors like the ones you found.

Here's an example showing the fixes:

CREATE PROCEDURE sp_Test(
  emailSent_In BIGINT UNSIGNED,
  emailTransport_In VARCHAR(100),
  formSecret_In VARCHAR(32)
)
BEGIN
  SET @query = '
    UPDATE tbl_JustSayThanks
    SET emailSent = ?,
        emailTransport = ?,
        emailSentDate = NOW()
    WHERE formSecret = ?';
  SET @es = emailSent_In;
  SET @et = emailTransport_In;
  SET @fs = formSecret_In;
  PREPARE stmt FROM @query;
  EXECUTE stmt USING @es, @et, @fs;
  DEALLOCATE PREPARE stmt;
END//
DELIMITER ;

Final comment:

  • Your example query has no dynamic syntax elements, only dynamic values. So you don't need to use a prepared statement at all.

This is how I'd really write the procedure:

CREATE PROCEDURE sp_Test(
  emailSent_In BIGINT UNSIGNED,
  emailTransport_In VARCHAR(100),
  formSecret_In VARCHAR(32)
)
BEGIN
  UPDATE tbl_JustSayThanks
  SET emailSent = emailSent_In,
      emailTransport = emailTransport_In,
      emailSentDate = NOW()
  WHERE formSecret = formSecret_In;
END//
DELIMITER ;

You should also be aware that MySQL stored procedures are greatly inferior to Microsoft SQL Server. MySQL doesn't keep compiled stored procedures, it doesn't support packages, it doesn't have a debugger... I recommend you do not use MySQL stored procedures. Use application code instead.

Upvotes: 9

Related Questions