-3

Ok, so I may be missing something but I build a query in php.

$query=("UPDATE values SET max=max+$value, right=right+$value WHERE 
         id IN ($placement_id)";
$pdo->$query($query);

Now, the weird thing happens when I check the database. All of the rows with the matching ids update the max column, but the right column doesn't. I made sure they are all int types in the database.

Am I missing something?

Ende Neu
  • 15,581
  • 5
  • 57
  • 68
Elias Baez
  • 43
  • 1
  • 6
  • 4
    [**`RIGHT`**](http://dev.mysql.com/doc/refman/5.6/en/reserved-words.html) is reserved keyword in mysql you need back-ticks arround your column – M Khalid Junaid Aug 28 '14 at 19:55
  • 1
    ...as is [`values`](http://dev.mysql.com/doc/refman/5.6/en/reserved-words.html) – Funk Forty Niner Aug 28 '14 at 19:56
  • I think this query should be throwing an error, rather that modifying the contents of any columns. And it looks like you are entirely missing the idea that the code is vulnerable to SQL injection. – spencer7593 Aug 28 '14 at 20:01

1 Answers1

5

You're using two reserved words, right and values

Wrap those words in backticks, or choose different words for your table and columns.

$query=("UPDATE `values` SET max=max+$value, `right`=`right`+$value WHERE 
     id IN ($placement_id)";

However, max could also play tricks on you, so wrap that in backticks also. It's an aggregate function.

$query=("UPDATE `values` SET `max`=`max`+$value, `right`=`right`+$value WHERE 
     id IN ($placement_id)";

Having error reporting on, would have signaled the errors:

error_reporting(E_ALL);
ini_set('display_errors', 1);

Since it seems like you're using PDO (and tagged as such): $pdo->$query($query);

Add $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); right after the connection is opened.

Sidenote: Your present code is open to SQL injection. Use prepared statements, or PDO with prepared statements. The latter being what you seem to be using as an MySQL API library.

Quoting spencer7593 from a comment:

"Also, setting PDO::ERRMODE_EXCEPTION on the connection will cause PDO to throw errors. (This isn't the most appropriate approach, I'm not recommending this in all cases. But if we're not going to bother writing code to perform the check, to see whether a statement succeeded or not, we'd let MySQL errors go unnoticed, with the default PDO::ERRMODE_SILENT".

Community
  • 1
  • 1
Funk Forty Niner
  • 74,450
  • 15
  • 68
  • 141
  • Thank you very much. I can't believe I missed that. – Elias Baez Aug 28 '14 at 20:06
  • 1
    +1 Also, setting **`PDO::ERRMODE_EXCEPTION`** on the connection will cause PDO to throw errors. (This isn't the most appropriate approach, I'm not recommending this in all cases. But if we're not going to bother writing code to perform the check, to see whether a statement succeeded or not, we'd let MySQL errors go unnoticed, with the default **`PDO::ERRMODE_SILENT`**. – spencer7593 Aug 28 '14 at 20:07
  • +1 @spencer7593 Good point. I will make a note of it quoting you/credit to. – Funk Forty Niner Aug 28 '14 at 20:08