Prevent unnecessary joins / complex conditions in delete

cakephp · Jul 27, 2016 · 5caac5f · 5caac5f
1 parent b17d580
commit 5caac5f
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 10 deletions.
diff --git a/lib/Cake/Model/Datasource/Database/Mysql.php b/lib/Cake/Model/Datasource/Database/Mysql.php
@@ -437,14 +437,7 @@ public function delete(Model $model, $conditions = null) {
 		if (empty($conditions)) {
 			$alias = $joins = false;
 		}
-		$complexConditions = false;
-		$fields = array_keys($this->describe($model));
-		foreach ((array)$conditions as $key => $value) {
-			if (strpos($key, $model->alias) === false && !in_array($key, $fields, true)) {
-				$complexConditions = true;
-				break;
-			}
-		}
+		$complexConditions = $this->_deleteNeedsComplexConditions($model, $conditions);
 		if (!$complexConditions) {
 			$joins = false;
 		}
@@ -460,6 +453,27 @@ public function delete(Model $model, $conditions = null) {
 		return true;
 	}
 
+/**
+ * Checks whether complex conditions are needed for a delete with the given conditions.
+ *
+ * @param Model $model The model to delete from.
+ * @param mixed $conditions The conditions to use.
+ * @return bool Whether or not complex conditions are needed
+ */
+	protected function _deleteNeedsComplexConditions(Model $model, $conditions) {
+		$fields = array_keys($this->describe($model));
+		foreach ((array)$conditions as $key => $value) {
+			if (in_array(strtolower(trim($key)), $this->_sqlBoolOps, true)) {
+				if ($this->_deleteNeedsComplexConditions($model, $value)) {
+					return true;
+				}
+			} elseif (strpos($key, $model->alias) === false && !in_array($key, $fields, true)) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 /**
  * Sets the database encoding
  *

diff --git a/lib/Cake/Model/Datasource/DboSource.php b/lib/Cake/Model/Datasource/DboSource.php
@@ -183,6 +183,13 @@ class DboSource extends DataSource {
  */
 	protected $_sqlOps = array('like', 'ilike', 'rlike', 'or', 'not', 'in', 'between', 'regexp', 'similar to');
 
+/**
+ * The set of valid SQL boolean operations usable in a WHERE statement
+ *
+ * @var array
+ */
+	protected $_sqlBoolOps = array('and', 'or', 'not', 'and not', 'or not', 'xor', '||', '&&');
+
 /**
  * Indicates the level of nested transactions
  *
@@ -2678,7 +2685,6 @@ public function conditions($conditions, $quoteValues = true, $where = true, Mode
 	public function conditionKeysToString($conditions, $quoteValues = true, Model $Model = null) {
 		$out = array();
 		$data = $columnType = null;
-		$bool = array('and', 'or', 'not', 'and not', 'or not', 'xor', '||', '&&');
 
 		foreach ($conditions as $key => $value) {
 			$join = ' AND ';
@@ -2696,7 +2702,7 @@ public function conditionKeysToString($conditions, $quoteValues = true, Model $M
 			} elseif (is_numeric($key) && is_string($value)) {
 				$out[] = $this->_quoteFields($value);
 			} elseif ((is_numeric($key) && is_array($value)) || in_array(strtolower(trim($key)), $bool)) {
-				if (in_array(strtolower(trim($key)), $bool)) {
+				if (in_array(strtolower(trim($key)), $this->_sqlBoolOps)) {
 					$join = ' ' . strtoupper($key) . ' ';
 				} else {
 					$key = $join;

diff --git a/lib/Cake/Test/Case/Model/Datasource/Database/MysqlTest.php b/lib/Cake/Test/Case/Model/Datasource/Database/MysqlTest.php
@@ -4060,10 +4060,15 @@ public function testDeleteNoComplexCondition() {
 		$this->Dbo->expects($this->at(0))->method('execute')
 			->with("DELETE `Article` FROM `$db`.`articles` AS `Article`   WHERE `id` = 1");
 
+		$this->Dbo->expects($this->at(1))->method('execute')
+			->with("DELETE `Article` FROM `$db`.`articles` AS `Article`   WHERE NOT (`id` = 1)");
+
 		$Article = new Article();
 
 		$conditions = array('id' => 1);
 		$this->Dbo->delete($Article, $conditions);
+		$conditions = array('NOT' => array('id' => 1));
+		$this->Dbo->delete($Article, $conditions);
 	}
 
 /**