How to stop all DB queries if one fails

When a user submits a form, I have 3 database actions that happen. If one fails, I don't want any of them to execute. With my current code that isn't happening. For example, if I purposefully misspell something in the SQL query the second query will fail but the first one will work and that record will be updated. How can I change this so if one fails, no changes are made at all to the database?

      
public function editAccom($data)
	{
		$this->db->query("UPDATE `accommodation` SET `name` = :name, `description` = :description, `slug` = :slug WHERE `id` = :id LIMIT 1");
		$this->db->bind(":name", $data['name']);
		$this->db->bind(":id", $data['id']);
		$this->db->bind(":description", $data['description']);
		$this->db->bind(":slug", $data['slug']);
		if($this->db->execute() === false) {
			return false;
		}
		
		$this->db->query("DELETE FROM `accom_pics` WHERE `accom_id` = :id");
		$this->db->bind(":id", $data['id']);
		if($this->db->execute() === false) {
			return false;
		}
		
		foreach($data['pics'] as $pic) {
			$this->db->query("INSERT INTO `accom_pics` (`accom_id`, `pic_name`) VALUES (:accom_id, :pic_name)");
			$this->db->bind(":accom_id", $data['id']);
			$this->db->bind(":pic_name", $pic);
			
			if($this->db->execute() === false) {
			return false;
				
			} 
		}
		
		return true;
	}

Open in new window

LVL 1
Black SulfurAsked:
Who is Participating?

[Product update] Infrastructure Analysis Tool is now available with Business Accounts.Learn More

x
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

David FavorLinux/LXD/WordPress/Hosting SavantCommented:
Simple way is to just place a SELECT as your first statement + return if the exact criteria isn't met.

Also your selection criteria for your UPDATE + DELETE are completely different, so I'm unsure how you imagine the relate.

If you were looking for a relation, you'd change your UPDATE...

from: WHERE `id` = :id

to: WHERE `accom_id` = :id

As it appears these can be different values, if your code is acting as you describe.
Chris StanyonWebDevCommented:
You should really be looking at transactions for this. The idea of a Trasaction is that several queries are grouped together and executed. If they all succeed then you commit the changes. If they don't all succeed, then you roll back the changes.

Can't quite figure out what DB driver you're using so give you specifics.

Something else that strikes me as odd in your code. You look like you're running prepared statements, which is good. However, in your 3rd query, you seem to be preparing and binding the query inside the loop. The idea here is that you prepare and bind once before the loop, and then execute inside the loop:

$this->db->query("INSERT INTO `accom_pics` (`accom_id`, `pic_name`) VALUES (:accom_id, :pic_name)");
$this->db->bind(":accom_id", $data['id']);
$this->db->bind(":pic_name", $pic);

foreach($data['pics'] as $pic) {
	if($this->db->execute() === false) {
		return false;
	} 
}

Open in new window

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
Black SulfurAuthor Commented:
Thanks Chris.

Yeah, I can understand it being a bit confusing because I am using a database class and without seeing that I guess it makes things a bit difficult to debug.
Before you posted this I had done a bit of reading and seen that I should use PDO's rollback() function but in order to do so you have to manually begin the transaction. I tried this and it seems to do the trick. Please point out anything you see that looks dodgy!

      
public function editAccom($data)
	{
		
		$this->db->beginTransaction();
		
		try {
		
		$this->db->query("UPDATE `accommodation` SET `name` = :name, `description` = :description, `slug` = :slug WHERE `id` = :id LIMIT 1");
		$this->db->bind(":name", $data['name']);
		$this->db->bind(":id", $data['id']);
		$this->db->bind(":description", $data['description']);
		$this->db->bind(":slug", $data['slug']);
		$this->db->execute();
		
		$this->db->query("DELETE FROM `accom_pics` WHERE `accom_id` = :id");
		$this->db->bind(":id", $data['id']);
		$this->db->execute();
		
		foreach($data['pics'] as $pic) {
			$this->db->query("INSERT INTO `accom_pics` (`accom_id`, `pic_name`) VALUES (:accom_id, :pic_name)");
			$this->db->bind(":accom_id", $data['id']);
			$this->db->bind(":pic_name", $pic);
			$this->db->execute();
		}
			
			$this->db->commit();
			return true;
			
		}
		
		catch(Exception $e) {
			
			echo $e->getMessage();
			$this->db->rollBack();
		}
	}

Open in new window

PMI ACP® Project Management

Prepare for the PMI Agile Certified Practitioner (PMI-ACP)® exam, which formally recognizes your knowledge of agile principles and your skill with agile techniques.

Chris StanyonWebDevCommented:
Yeah - that's the general idea. Couple of things - firstly, make sure you have enabled Exceptions for PDO. By default it generates errors rather than throwing Exceptions. You can set this like so:

$this->db->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION );

Secondly, I've already mentioned this, but when preparing queries to be ran in a loop, you only need to prepare the query once and set up the bindings once. You should NOT be prepping and binding inside the loop - you lose all the performance gains of a prepared query if you do that.

// Prepare the Query
$this->db->query("INSERT INTO `accom_pics` (`accom_id`, `pic_name`) VALUES (:accom_id, :pic_name)");

// Bind the parameters
$this->db->bind(":accom_id", $data['id']);
$this->db->bind(":pic_name", $pic);

// Now loop through the data
foreach($data['pics'] as $pic) {
    $this->db->execute();
}

Open in new window

You might also want to re-think how you handle the return values of the function. For example, you return True if it's successful but echo a message if it's not. You may want to return False after the rollback - really depends on how you consume your class.
Black SulfurAuthor Commented:
Thanks Chris,

I tried what you said but this fails and the database transactions don't take place?

           $this->db->query("UPDATE `accommodation` SET `name` = :name, `description` = :description, `slug` = :slug WHERE `id` = :id LIMIT 1");
		$this->db->bind(":name", $data['name']);
		$this->db->bind(":id", $data['id']);
		$this->db->bind(":description", $data['description']);
		$this->db->bind(":slug", $data['slug']);
		$this->db->execute();
		
		$this->db->query("DELETE FROM `accom_pics` WHERE `accom_id` = :id");
		$this->db->bind(":id", $data['id']);
		$this->db->execute();
		
			
		$this->db->query("INSERT INTO `accom_pics` (`accom_id`, `pic_name`) VALUES (:accom_id, :pic_name)");
		$this->db->bind(":accom_id", $data['id']);
		$this->db->bind(":pic_name", $pic);	
			
		foreach($data['pics'] as $pic) {
			$this->db->execute();
		}

Open in new window

Chris StanyonWebDevCommented:
OK. I'm assming you're only showing part of your code as there's nothing in there that starts / commits or rolls back a transaction.

I don't know what's behind your class query() and bind() methods, so that may be the issue. My point was simply that under normal operations, you don't prepare and bind a query inside the loop. It makes no sense.

If it's not working for you then you may have to go your original route and do the prep and binding inside of the loop. Bear in mind though as I've already pointed out, you will lose the performance benefits of the prepared statements.
Black SulfurAuthor Commented:
Hi Chris, I would like to do the right thing and not bind inside the loop as you mentioned. Hopefully more code will help. Here is the database class:

class Database {
	
	private $host = DB_HOST;
	private $user = DB_USER;
	private $pass = DB_PASS;
	private $dbname = DB_NAME;
	
	private $dbh;
	private $stmt;
	private $error;
	
	public function __construct() {
		$dsn = "mysql:host=$this->host;dbname=$this->dbname";
		$options = array(
			PDO::ATTR_PERSISTENT => true,
			PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
		);
		
		try {
			$this->dbh = new PDO($dsn, $this->user, $this->pass, $options);
			
		} catch(PDOexception $e) {
			$this->error = $e->getMessage();
			echo $this->error;
		}
	}
	
	// Prepare statement with query
	public function query($sql) {
		$this->stmt = $this->dbh->prepare($sql);
	}
	
	// Bind values
	public function bind($param, $value, $type = null) {
		if(is_null($type)) {
			switch(true) {
				case is_int($value):
					$type = PDO::PARAM_INT;
					break;
				case is_bool($value):
					$type = PDO::PARAM_BOOL;
					break;
				case is_null($value):
					$type = PDO::PARAM_NULL;
					break;
				default:
					$type = PDO::PARAM_STR;
			}
		}
		
		$this->stmt->bindValue($param, $value, $type);
	}
	
	// Execute the prepared statement
	public function execute() {
		return $this->stmt->execute();
	}
	
	// Get result set as array object
	public function resultSet() {
		$this->execute();
		return $this->stmt->fetchAll(PDO::FETCH_OBJ);
	}
	
	// Get single record as object
	public function single() {
		$this->execute();
		return $this->stmt->fetch(PDO::FETCH_OBJ);
	}
	
	// Get row count
	public function rowCount() {
		return $this->stmt->rowCount();
	}
	
	// Get last id
	public function lastId() {
		return $this->dbh->lastInsertId();
	}
	
	public function beginTransaction() {
		return $this->dbh->beginTransaction();
	}
	
	public function rollBack() {
		return $this->dbh->rollBack();
	}
	
	public function commit() {
		return $this->dbh->commit();
	}
}

Open in new window


And here is the whole method:

public function editAccom($data)
	{
		
		$this->db->beginTransaction();
		
		try {
		
		$this->db->query("UPDATE `accommodation` SET `name` = :name, `description` = :description, `slug` = :slug WHERE `id` = :id LIMIT 1");
		$this->db->bind(":name", $data['name']);
		$this->db->bind(":id", $data['id']);
		$this->db->bind(":description", $data['description']);
		$this->db->bind(":slug", $data['slug']);
		$this->db->execute();
		
		$this->db->query("DELETE FROM `accom_pics` WHERE `accom_id` = :id");
		$this->db->bind(":id", $data['id']);
		$this->db->execute();
		
		foreach($data['pics'] as $pic) {
			$this->db->query("INSERT INTO `accom_pics` (`accom_id`, `pic_name`) VALUES (:accom_id, :pic_name)");
			$this->db->bind(":accom_id", $data['id']);
			$this->db->bind(":pic_name", $pic);
			$this->db->execute();
		}
			
			$this->db->commit();
			return true;
			
		}
		
		catch(Exception $e) {
			
			echo $e->getMessage();
			$this->db->rollBack();
		}
	}

Open in new window

Chris StanyonWebDevCommented:
Ahh -  I see what's going on.

In your Database class, you're binding the data by using bindValue(), and not bindParam(). The difference is that bindValue() binds the value at that point. With bindParam, it's the variable itself that's bound and the value is read in each time you call execute().

$name = "Mickey Mouse";
$stmt = $db->prepare("SELECT * FROM table WHERE someValue = :myVar");
$stmt->bindValue("myVar", $name); <-- Bind the value of $name (Mickey Mouse) to the query.
$name = "Donald Duck";
$stmt->execute();
// Query executed = SELECT * FROM table WHERE someValue = 'Mickey Mouse';

Open in new window

$name = "Mickey Mouse";
$stmt = $db->prepare("SELECT * FROM table WHERE someValue = :myVar");
$stmt->bindParam("myVar", $name); <-- Bind the variable $name to the query
$name = "Donald Duck";
$stmt->execute();
// Query executed = SELECT * FROM table WHERE someValue = 'Donald Duck';

Open in new window

Change line 51 of your class to this:

$this->stmt->bindParam($param, $value, $type);
Black SulfurAuthor Commented:
Thanks Chris, I will certainly try that. And I also just want to take the time to read up on the difference between the two as I don't really understand.
Chris StanyonWebDevCommented:
No worries. The differences are subtle at first glance. Basically, if a value won't change when you execute, then bindValue() - if it will change, then bindParam()

:)
Black SulfurAuthor Commented:
Thanks Chris. I have made the change you suggested and now I get an error that reads:

Undefined variable: pic

Is this not because pic is only defined in the foreach loop but I am trying to bind it before the loop?
Chris StanyonWebDevCommented:
OK. You need to pass the variable in By Reference, instead of By Value. Change your bind() method definition to this:

public function bind($param, &$value, $type = null) {
    ...

Open in new window

Notice that we're passing in the $value by reference (&$value);
Black SulfurAuthor Commented:
Thanks, Chris.

I realize this is getting tedious now but this gives me another elsewhere:

Uncaught Error: Cannot pass parameter 2 by reference

And that is where I am passing a static text value in (Chalets) instead of a variable

$this->db->query("SELECT `name`, `slug`,`id` FROM `accommodation` WHERE `category` = :category");
$this->db->bind(":category", "Chalets");

Open in new window

Chris StanyonWebDevCommented:
OK. Based on the subtle differences between bindValue and bindParam, you may want to have 2 methods in your class - one for each.

Failing that, you can work around it by assigning your static value to a variable before binding it:

$this->db->query("SELECT `name`, `slug`,`id` FROM `accommodation` WHERE `category` = :category");
$cat = "Chalets";
$this->db->bind(":category", $cat);

Open in new window

Black SulfurAuthor Commented:
Phew, finally working! Thanks for you help and patience! :)
Chris StanyonWebDevCommented:
No worries. Glad we got there
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
PHP

From novice to tech pro — start learning today.