CODEWITHSUNDEEP
phppdo
user2489944
user2489944

Reputation: 3

Updating data with php pdo

I'm new to using PDO and I got all this to work with adding and deleting entries, but I can't get it to update data. I'm not sure if my sql statement is off or if I'm just missing something here.

if (isset($_GET['id'])) {
    $id = $_GET['id'];
    $data = $article->fetch_data($id);

    if(isset($_POST['title'], $_POST['content'])) {
        $title = $_POST['title'];
        $content = nl2br($_POST['content']);

        if (empty($title) or empty($content)) {
            $error = 'All fields are required!';
        } else {
            $query = $pdo->prepare('UPDATE articles SET article_title = ?, article_content = ?, article_timestamp = ? WHERE article_id = $id');

            $query->bindValue(1, $title);
            $query->bindValue(2, $content);
            $query->bindValue(3, time());

            $query->execute();

            header('Location: index.php');
        }
    }
}

Upvotes: 0

Views: 8567

Answers (2)

Gohn67
Gohn67

Reputation: 10648

You need to double quote your query to interpolate the $id variable. If you use single quotes your variables will be interpreted as $id.

$query = $pdo->prepare("UPDATE articles SET article_title = ?, article_content = ?, article_timestamp = ? WHERE article_id = $id");

As others have said this can lead to SQL injection.

Instead you should also bind the $id variable: If you are using an int as your id you may also want to use PDO::PARAM_INT. Example $query->bindValue(4, $id, PDO::PARAM_INT);

$query = $pdo->prepare("UPDATE articles SET article_title = ?, article_content = ?, 

article_timestamp = ? WHERE article_id = ?");
$query->execute(array($title, $content, time(), $id));

Upvotes: 2

Cameeob2003
Cameeob2003

Reputation: 482

I personally would write this as so:

if (isset($_GET['id'])) {
    $id = $_GET['id'];
    $data = $article->fetch_data($id);

    if(isset($_POST['title'], $_POST['content'])) {
    $title = $_POST['title'];
    $content = nl2br($_POST['content']);

    if (empty($title) or empty($content)) {
        $error = 'All fields are required!';
    } else {
        $sql = "UPDATE articles SET article_title = :title, article_content = :content, article_timestamp = :timestamp WHERE article_id = :id";
        $query = $pdo->prepare($sql);

        $query->bindValue(":title", $title);
        $query->bindValue(":content", $content);
        $query->bindValue(":timestamp", time());
        $query->bindValue(":id", $id);


        try {
          $result = $query->execute();
        } catch(PDOException $e) {
          echo $e->getCode() . " - " . $e->getMessage();
        }

        if($result) {
          header('Location: index.php');
        }
    }
}

}

This is just my preferred way. If you are taking user input, (which im assuming you are from the $_GET['id']) then you should pass the bind of the ID as well to help you even further protect from SQL injection. Also the way I learned PDO was by binding my variables with the " : ". Cant say I've really ever seen anyone use the ? when dealing in PDO, but then again im more of a solo dev guy.

Anyway that should give you what you need.

Edit: One more thing I forgot to mention. Add the $result = $query->execute(); because PDO will return true if the statement is successful and false if it is not. So you can redirect to a different page or re-show the form with a flash message etc.

Upvotes: 1

Related Questions