PHP MySQL issues

[thelock]

Not sure what is going on with my code but for some reason I can't get the database to update (using code that has worked on MySQL 4 now running on MySQL 5).

mysql_query("update products set desc='$desc' where id=$ref;")

I'm sure I am just missing something simple but any pointers would be great. I have tried using the code that is on tizag (going back to basics) but still get the error:

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 'desc='

Cheers

 

[kaidoristm]

Try this

mysql_query("UPDATE products SET desc='$desc' WHERE id='$ref'");

 

[Killswitch]

mysql_query("UPDATE products SET desc='".$desc."' WHERE id=".$ref);

Never just put your variable in the query, or anything for that matter, always have ".$var." or if you don't use quotes and just use apostrophes, always put '.$var.'

 

[thelock]

Thanks guys; I feel like such a dunce.

After throwing my computer out the window I have realised that the reason that it wasn't updating the mysql table was because I had a field called 'desc' which is obviously a MySQL command and can't be a field name. DOH!!

Cheers to kaidoristm and killswitch

 

[Killswitch]

Yeah, I always make my field names part of the table, say I have articles table, then the description field would be article_desc.

 

[voyevoda]

Try this

mysql_query("UPDATE products SET desc='$desc' WHERE id='$ref'");

This is the exact same query he's trying to run, but you forgot a semicolon, so it wouldn't work anyways. Case doesn't matter in SQL keywords and function names (e.g., MIN(), MAX(), GROUP BY, etc.).

mysql_query("UPDATE products SET desc='".$desc."' WHERE id=".$ref);

Never just put your variable in the query, or anything for that matter, always have ".$var." or if you don't use quotes and just use apostrophes, always put '.$var.'

There is nothing wrong with interpolation. It's generally more readable. Why are you recommending against it? Your concatenation approach results in the exact same (REALLY BAD) SQL query. Either way allows arbitrary SQL code to be injected (like "-- DROP DATABASE `whatever`;").

What you really want to be using is something like this:

function mysql_safe_query($query, $params=false) { 
    if ($params) { 
        foreach ($params as &$v) { $v = mysql_real_escape_string($v); }
        $sql_query = vsprintf( str_replace("?","'%s'",$query), $params );    
        $sql_query = mysql_query($sql_query);
    } else { 
        $sql_query = mysql_query($query);
    } 

    return ($sql_query); 
}

And you would write SQL queries like this:

mysql_safe_query("UPDATE products SET desc='?' WHERE id=?;", $description, $product_id);

This method properly sanitizes all input that is being used to dynamically generate the SQL query.

Christ, PHP makes me sad.

Yeah, I always make my field names part of the table, say I have articles table, then the description field would be article_desc.

This suggestion brought to you by the Department of Redundancy Department. How about something that's actually descriptive, like... 'description'? I'm revoking your advice giving privileges until further notice.

 

[sipser]

If you run into this in the future and don't want to rename your column, enclose it into ``'s. Example:

 mysql_query("UPDATE products SET `desc`='value' WHERE `id`='value");

 

[Killswitch]

There is nothing wrong with interpolation. It's generally more readable. Why are you recommending against it? Your concatenation approach results in the exact same (REALLY BAD) SQL query. Either way allows arbitrary SQL code to be injected (like "-- DROP DATABASE `whatever`;").

What you really want to be using is something like this:

function mysql_safe_query($query, $params=false) { 
    if ($params) { 
        foreach ($params as &$v) { $v = mysql_real_escape_string($v); }
        $sql_query = vsprintf( str_replace("?","'%s'",$query), $params );    
        $sql_query = mysql_query($sql_query);
    } else { 
        $sql_query = mysql_query($query);
    } 

    return ($sql_query); 
}

And you would write SQL queries like this:

mysql_safe_query("UPDATE products SET desc='?' WHERE id=?;", $description, $product_id);

This method properly sanitizes all input that is being used to dynamically generate the SQL query.

Christ, PHP makes me sad.

I was just assuming he had already sanitized the variables, I was here to help him fix his query, not secure his code for him

This suggestion brought to you by the Department of Redundancy Department. How about something that's actually descriptive, like... 'description'? I'm revoking your advice giving privileges until further notice.

Because I just find it easier doing it that way, neither way are wrong.