2

I have a php script to update details in a MySQL table. It all worked fine but now I have changed the db connection method to PDO:

$pdo = new PDO('mysql:host=localhost;dbname=****', '****', '*****');

I made various changes to the script to accommodate this so it continues to work, The only place that fails is right at the end after the mysql table has been updated. I get this error:

Fatal error: Uncaught exception 'PDOException' with message 'SQLSTATE[42000]: Syntax error or access violation: 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 'and park_id=31' at line 1' in /home3/danville/public_html/test2/index.php:29 Stack trace: #0 /home3/danville/public_html/test2/index.php(29): PDO->query('update tpf_ride...') #1 {main} thrown in /home3/danville/public_html/test2/index.php on line 29

This is the piece of code causing the error:

$query = "update tpf_rides set name='$name',type='$type'";
        if($topride!=""){$query .= ",top_ride=$topride";}
        if($info!=""){$query .= ",info='$info'";}
        if($height!=""){$query .= ",height=$height";}
        if($length!=""){$query .= ",length=$length";}
        if($speed!=""){$query .= ",speed=$speed";}
        if($inversions!=""){$query .= ",inversions=$inversions";}
    $query .= " where ride_id=".$ride_id." and park_id=".$park_id;  
            $pdo->query($query);
        }

line 29 is this on Notepad++ $pdo->query($query); although the error message seems to reference the line above that $query .= " where ride_id=".$ride_id." and park_id=".$park_id;

Any ideas what I ned to change to stop the error? Additional details - I connect to the db with a require_once include. The updates do take effect despite the error.

Phil
  • 157,677
  • 23
  • 242
  • 245
user2574794
  • 996
  • 3
  • 10
  • 20
  • 1
    My initial guess would be a problem with `$ride_id`. It's probably empty / null – Phil Aug 12 '13 at 00:28
  • Echo the query, or write it to the error_log just before you execute it. Then check the query is what you think it is. –  Aug 12 '13 at 00:31
  • Can you just try to echo the sql code to be sure of what the issue is, probably once, and disable the query execution to ensure update doesnt work from some other part of the code – skv Aug 12 '13 at 00:31
  • 1
    part of the point of PDO is to parametize your query. If some of your values are empty it will do the right thing. Look into using placeholders. – Cfreak Aug 12 '13 at 00:32
  • @Phil I don't know if that is the issue because the whole query was working fine before I changed it to PDO and even now, despite the error the updated details are being added to MySQL correctly – user2574794 Aug 12 '13 at 00:32
  • @phil actually looks like you are right, sorry! I echoed the the query and it seems to be a counting issue. With the particular update there were three records, these 3 echoed correctly but after this was repeated many many times `update tpf_rides set name='',type='' where ride_id= and park_id=31update tpf_rides set name='',type='' where ride_id= and park_id=31` – user2574794 Aug 12 '13 at 00:39
  • What would I need to alter then because all was working before hand? – user2574794 Aug 12 '13 at 00:40

1 Answers1

2

If you're going to switch to PDO, you might as well take advantage of prepared statements and parameter binding. It actually makes your queries much safer from SQL injection and also makes your code more readable. Your query builder approach does complicate things a little but it's still possible. I'd also highly recommend enabling error reporting during development. For example

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

$upd = array('name = :name', 'type = :type');
$values = array(
    'name' => $name,
    'type' => $type,
    'ride_id' => $ride_id,
    'park_id' => $park_id
);

if (!empty($topride)) {
    $upd[] = 'top_ride = :topride'; // :topride is the named parameter placeholder
    $values['topride'] = $topride; // the array key matches the named placeholder above
}
if (!empty($info)) {
    $upd[] = 'info = :info';
    $values['info'] = $info;
}
// and so on

$query = sprintf('UPDATE tpf_rides SET %s WHERE ride_id = :ride_id AND park_id = :park_id',
    implode(', ', $upd));
$stmt = $pdo->prepare($query);
$stmt->execute($values);
Phil
  • 157,677
  • 23
  • 242
  • 245