2

This code works, table is updating, but server responds: "You have an error in your SQL syntax".

Asking for the sake of interest. Please tell me where is an error

$id = $_POST['id'];
$name = $_POST['name'];
$image = $_POST['image'];
$price = $_POST['price'];

mysql_connect("localhost","main","password");
mysql_select_db("main");

$result = mysql_query("SELECT * FROM goods WHERE id='".$id."'");
if(mysql_num_rows($result) > 0) {
    $newquery = mysql_query("UPDATE goods SET name='".$name."', image='".$image."', price='".$price."' WHERE id='".$id."'");
    if(!mysql_query($newquery)) {
        die('Invalid query: ' . mysql_error());
    } else {
        echo "Updated successfully";
        }
    } else {
        echo "Error: there is no such product in DB";
    }

Error:

Invalid query: 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 '1' at line 1

Jay Blanchard
  • 34,243
  • 16
  • 77
  • 119
Danny
  • 31
  • 1
  • 3
  • *YOU* tell *US* what the error is. Post the *WHOLE* error message. – John Conde Jul 14 '15 at 18:23
  • 4
    [Your script is at risk for SQL Injection Attacks.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) – Jay Blanchard Jul 14 '15 at 18:24
  • 2
    If you can, you should [stop using `mysql_*` functions](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php). They are no longer maintained and are [officially deprecated](https://wiki.php.net/rfc/mysql_deprecation). Learn about [prepared](http://en.wikipedia.org/wiki/Prepared_statement) [statements](http://php.net/manual/en/pdo.prepared-statements.php) instead, and consider using PDO, [it's really not hard](http://jayblanchard.net/demystifying_php_pdo.html). – Jay Blanchard Jul 14 '15 at 18:25
  • *"For the sake of interest?"* – Jay Blanchard Jul 14 '15 at 18:25
  • The WHOLE error message is: Invalid query: 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 '1' at line 1 – Danny Jul 14 '15 at 18:26
  • It may be that ID isn't expecting a String. The where condition may be `where id=".$id`. But yeah you shouldn't do it like this because you are vulnerable to sql injection. Someone just has to modify the post to try to get database info – MiltoxBeyond Jul 14 '15 at 18:27
  • 5
    Echo the query and see what it looks like. – Rahul Jul 14 '15 at 18:27
  • I call shenanigans - if you have a syntax error *no* data is inserted to the database. – Jay Blanchard Jul 14 '15 at 18:28

3 Answers3

1

Your insert values are strings. So you have to use " or ' to wrap it:

$sql = "UPDATE users SET fullname = '".$fullname."', bio = '".$bio."', 
birthdate = '".$birthdate."', pass = '".$newpass."',
WHERE username = '".$susername."'";
Pierre.Vriens
  • 2,117
  • 75
  • 29
  • 42
0

'name' is a MySQL keyword. If you plan to use it as a column name then you must use back ticks when performing queries on the column:

"UPDATE `goods` SET `name`='".$name."', `image`='".$image."', `price`='".$price."' WHERE `id`='".$id."'"
Jay Blanchard
  • 34,243
  • 16
  • 77
  • 119
  • No, it didn't make sense. Still syntax error. I'll check your links later, thanks for your time – Danny Jul 14 '15 at 18:42
-2

Your where condition is wrong. It is expecting an integer not a string for ID.

Should be:

$newquery = mysql_query("UPDATE goods SET name='".$name."', image='".$image."', price='".$price."' WHERE id=".$id);

But you should note 2 things.

  1. mysql_query and all mysql_ functions have been replaced by mysqli_

  2. you are vulnerable to SQL injection. You should use PDO or any ORM/Database Abstraction to handle queries for you to prevent that.

MiltoxBeyond
  • 2,683
  • 1
  • 13
  • 12
  • A couple of things: 1.) Try both a string and a number to select from an integer column in MySQL and see what happens. B.) `mysqli_*` has not replaced `mysql_*`, it is the improved API and in some cases has a completely different syntax. And 3.)If you use PDO there would be no need to use `mysqli_*` ¯\\_(ツ)_/¯ – Jay Blanchard Jul 14 '15 at 18:36
  • 1. More than likely either his parameter is coming in wrong, where it may even have a quote around the value or something else is bugging out his statement. B) mysqli has a common interface so most times you can use mysqli_ functions as a drop in replacement although the OO version is preferred. and 3) I just mention that he should use PDO mainly to avoid having to do all the hard work of filtering out input to prevent sql injection – MiltoxBeyond Jul 14 '15 at 18:39
  • You can filter input when using MySQLi too. – Jay Blanchard Jul 14 '15 at 18:53
  • True. But hopefully using PDO methods they can hopefully avoid the pitfalls which bring about those vulnerabilities. – MiltoxBeyond Jul 14 '15 at 18:59