PHP single query inside a while loop not running on MySQL

Question

$sql="SELECT retail_peak, number from callplandata ";
$rs=mysql_query($sql,$conn);
$sql2='';
while($result=mysql_fetch_array($rs)) {
    $sql2.="UPDATE callplandata set ".$_POST["callplancopy_newname"]." = '".$result[$_POST["callplancopy"]]."' where number = '".$result["number"]."'; ";
}
$rs2=mysql_query($sql2,$conn) or die(mysql_error());

I am trying to run the above queries, i have set $sql2 with a ; on the end so i just run one query rather than many separate queries.

I am getting this Error message:

You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'UPDATE callplandata set dcabr = '0' where number = '44*116'; UPDATE callplandata' at line 1

when i echo $sql2, it looks like - http://www.wepaste.com/sql2/

**warning** your code is extemely vulnerable to sql injection attacks! — Daniel A. White, Aug 05 '14 at 16:21
**Warning:** mysql extension is [deprecated](http://stackoverflow.com/questions/13944956) as of PHP 5.5.0, and will be removed in the future. Instead, the [MySQLi](http://www.php.net/manual/en/book.mysqli.php) or [PDO_MySQL](http://www.php.net/manual/en/ref.pdo-mysql.php) extension should be used. Please don't use `mysql_*` to develop new code. — bansi, Aug 05 '14 at 16:22
mysql_query will usually execute only one statement, see the [manual](http://www.php.net/manual/en/function.mysql-query.php): *mysql_query() sends a unique query (multiple queries are not supported) to the currently active database on the server that's associated with the specified link_identifier.* and the red box.. The mysql_* functions are deprecated and I'm sure you could get the desired result with just one statement. — VMai, Aug 05 '14 at 16:23
Your approach is to run a select query and then loop through the results and run an update query in that loop. A more efficient approach is to run a single update query that updates one table from another. Better yet, drop the field you are updating and simply get it from the other table when needed. — Dan Bracuk, Aug 05 '14 at 16:30
I see this question a lot. Please read [common database debugging for PHP and MySQL](http://jason.pureconcepts.net/2013/04/common-debugging-php-mysql/). — Jason McCreary, Aug 05 '14 at 16:38

score 0 · Answer 1 · answered Aug 05 '14 at 16:24

0

mysql is deprecated but it also doesn't allow multiple statements in a single query.

You can however use multiple statements in a single query with mysqli by using mysqli_multi_query

answered Aug 05 '14 at 16:24

Rob

12,659
4
39
56

Note that there is an upper limit on the size of the SQL text that can be sent to the server: **`SHOW VARIABLES LIKE 'max_allowed_packet'`**. – spencer7593 Aug 05 '14 at 17:08
The more pressing issue is that there is no need for multiple UPDATE statements; the same effect can be achieved with a _single_ UPDATE statement, without need for a SELECT, retrieving all rows, and processing in a loop. A single UPDATE statement would also be much more clear, making it more obvious what the code is doing, not obfuscating what's going on with an unnecessary SELECT and a loop. – spencer7593 Aug 05 '14 at 17:15
That is true but this isn't an attempt to address best practices, sql injection, server configuration, or design issues. The op has a syntax error and this answer explains why. Not sure how either of those comments have anything to do with this. – Rob Aug 05 '14 at 17:31
True: the mysql interface is deprecated. And true: the mysql interface does not support multiple statements in a query. If the "op has a syntax error", why would we recommend that OP change the interface... to enable OP to implement an inefficient anti-pattern? I don't subscribe to encouraging "poor practices". – spencer7593 Aug 05 '14 at 17:42
Why would I not recommend changing the interface? It's deprecated. Also, multiple select statements in a query isn't always ideal but it's hardly an anti pattern. Anyways, I'm done here. Go heckle someone else. – Rob Aug 05 '14 at 17:45
I don't mean for you to feel heckled. I was trying to provide some guidance to op. You seem to have missed my point: I didn't say that "multiple statements in a query" was an anti-pattern. (If that were true, OP could avoid that by simply executing each statement individually within the loop.) The anti-pattern is the whole approach of processing the set row-by-agonizing-row in a loop; that's entirely unnecessary and horribly inefficient. – spencer7593 Aug 05 '14 at 18:03

Arth · Answer 2 · 2014-08-05T16:38:09.707

0

Your immediate problem is that you are concatenating the $sql2 queries in the while loop to make one long string and then trying to execute the long string as one query.

You should move the execution of $sql2 into the while loop and drop the .= operator in favor of =:

$sql2=''; // Don't need this line

while($result=mysql_fetch_array($rs)) {
  $sql2="
    UPDATE callplandata 
       SET ".$_POST["callplancopy_newname"]."='".$result[$_POST["callplancopy"]]."'
     WHERE number = '".$result["number"]."'
  ";
  $rs2=mysql_query($sql2,$conn) or die(mysql_error());
}

You could also follow Rob's suggestion and execute the long string as a multiple query.

You would also do well to heed the warnings in the comments about SQL injection and deprecated functions.

You can actually run this as one statement by dropping the WHERE clause.. it is the same logic.

edited Aug 05 '14 at 16:38

answered Aug 05 '14 at 16:26

Arth

12,789
5
37
69

the reason i made one long query is because it will then just contact the database once and run the whole query in one go. when i copy and paste the echoed $sql2 directly into MySQL it runs it fine, its just not working in PHP – Charlie Aug 05 '14 at 16:28
@Charlie Fair enough.. then follow Rob's suggestion or something similar in PDO. – Arth Aug 05 '14 at 16:31
i dont suppose you have a PDO example that would work the same as this ? – Charlie Aug 05 '14 at 16:33
@Charlie, hmm looks like PDO multiple queries are a bit dodge, you can run this as one statement without the `WHERE` though.. it's the same logic. – Arth Aug 05 '14 at 16:39
i need the WHERE clause so it adds the correct values to the new column. basically i am adding a new column and copying each row value from another column – Charlie Aug 05 '14 at 16:40
i just want to be able to duplicate a whole column and its values for each row and change the column name – Charlie Aug 05 '14 at 16:42
@Charlie, Whaaaat? You can't change a column name with an `UPDATE`! You need the `ALTER TABLE` DDL for that and then a single `UPDATE`. Unless you are building an insane DB creation platform, if you are trying to do this in PHP via POST your data structure is probably incorrect. – Arth Aug 05 '14 at 16:46
i am using ALTER TABLE to create a new column then updating. check my full code here: http://pastebin.com/bBHfn95y – Charlie Aug 05 '14 at 16:48
@Charlie Ahh, I misunderstood. Sounds like you can just do one `UPDATE callplandata SET **new_column_name** = **old_column_name**` that'll copy the data for all rows. Still a bizarre case however. :) – Arth Aug 05 '14 at 16:57
should i do this within the while loop ? – Charlie Aug 05 '14 at 17:02
The more immediate problem is the anti-pattern of selecting and retrieving all rows from the table, and then updating each individual row. Updating _all_ of the rows in the table can be accomplished much more efficiently with a _single_ UPDATE statement. There's no need for a `SELECT`, retrieving all rows and processing them in a loop. – spencer7593 Aug 05 '14 at 17:10
@Charlie You don't need the select, or the while loop. – Arth Aug 05 '14 at 17:40
@spencer7593 Yep, finally got to that conclusion! But after this comment thread.. it appears that there was a whole other level to this question. It looks like your answer is more thorough and starts from a firmer base than mine however. – Arth Aug 05 '14 at 17:43

spencer7593 · Answer 3 · 2014-08-06T21:05:18.840

You are using an anti-pattern for what this code is trying to achieve: to update all rows in the callplancopy table (where the number column is not null) to set a column equal to a value.

(NOTE: the "WHERE number =" in the original UPDATE statement would effectively prevent rows with a NULL value in that column from being updated.)

The entire mess of code is performing RBAR (Row By Agonizing Row) what could be more simply and efficiently accomplished with just one single UPDATE statement issued to the database:

UPDATE callplandata d
   SET d.`somecol` = 'someval'
 WHERE d.number IS NOT NULL

(NOTE: The WHERE clause is included to reproduce the behavior of the original UPDATE statements, avoiding updating rows where the number column is NULL. If that's not desired, or is not necessary, then the WHERE clause can be omitted.)

(NOTE: This assumes that you are assigning a literal value to the column, as in the original UPDATE, where we see "callplancopy" enclosed in single quotes, making it a string literal. If you are meaning to copy the value from another column in the row, then we'd enclose the column identifier in backticks, not single quotes.)

SET d.`somecol` = d.`some_other_col`

If we insist on using the deprecated mysql interface, we really need use the mysql_real_escape_string function to make unsafe values "safe" for inclusion in the SQL text.

$sql = "UPDATE callplandata d
           SET d.`" . mysql_real_escape_string($_POST["callplancopy_newname"]) . "`"
         . " = d.`" . mysql_real_escape_string($_POST["callplancopy"] . "`
         WHERE d.number IS NOT NULL";

# for debugging, echo out the SQL text
#echo $sql;

NOTE: The PHP mysql interface is deprecated. New development should make use of the PDO or mysqli interface.

PHP single query inside a while loop not running on MySQL

3 Answers3