Fix issues with insert() and subqueries.

insert queries should accept subqueries as values for columns. This lets the query builder be more composable and cover even more use cases without much overhead. I've also added the missing traverse() implementation as @lorenzo pointed out.
cakephp · Sep 21, 2014 · ac5282d · ac5282d
1 parent bbbe6a9
commit ac5282d
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 14 deletions.
diff --git a/src/Database/Expression/ValuesExpression.php b/src/Database/Expression/ValuesExpression.php
@@ -146,25 +146,28 @@ public function sql(ValueBinder $generator) {
 
 		$i = 0;
 		$defaults = array_fill_keys($this->_columns, null);
+		$placeholders = [];
+
 		foreach ($this->_values as $row) {
 			$row = array_merge($defaults, $row);
+			$rowPlaceholders = [];
 			foreach ($row as $column => $value) {
 				if ($value instanceof ExpressionInterface) {
-					$value = $value->sql($generator);
+					$rowPlaceholders[] = '(' . $value->sql($generator) . ')';
+					continue;
 				}
 				$type = $this->typeMap()->type($column);
-				$generator->bind($i++, $value, $type);
+				$placeholder = $generator->placeholder($i);
+				$rowPlaceholders[] = $placeholder;
+				$generator->bind($placeholder, $value, $type);
 			}
+			$placeholders[] = implode(', ', $rowPlaceholders);
 		}
 
 		if ($this->query()) {
 			return ' ' . $this->query()->sql($generator);
 		}
 
-		$placeholders = [];
-		$numColumns = count($this->_columns);
-		$rowPlaceholders = implode(', ', array_fill(0, $numColumns, '?'));
-		$placeholders = array_fill(0, count($this->_values), $rowPlaceholders);
 		return sprintf(' VALUES (%s)', implode('), (', $placeholders));
 	}
 
@@ -186,6 +189,14 @@ public function traverse(callable $visitor) {
 			if ($v instanceof ExpressionInterface) {
 				$v->traverse($visitor);
 			}
+			if (!is_array($v)) {
+				continue;
+			}
+			foreach ($v as $column => $field) {
+				if ($field instanceof ExpressionInterface) {
+					$field->traverse($visitor);
+				}
+			}
 		}
 	}
 

diff --git a/tests/TestCase/Database/QueryTest.php b/tests/TestCase/Database/QueryTest.php
@@ -2130,7 +2130,7 @@ public function testInsertSimple() {
 		$result = $query->sql();
 		$this->assertQuotedQuery(
 			'INSERT INTO <articles> \(<title>, <body>\) ' .
-			'VALUES \(\?, \?\)',
+			'VALUES \(:c0, :c1\)',
 			$result,
 			true
 		);
@@ -2166,7 +2166,7 @@ public function testInsertSparseRow() {
 		$result = $query->sql();
 		$this->assertQuotedQuery(
 			'INSERT INTO <articles> \(<title>, <body>\) ' .
-			'VALUES \(\?, \?\)',
+			'VALUES \(:c0, :c1\)',
 			$result,
 			true
 		);
@@ -2305,12 +2305,18 @@ public function testInsertExpressionValues() {
 			->into('articles')
 			->values(['title' => $query->newExpr("SELECT 'jose'")]);
 
-		$result = $query->sql();
-		$this->assertQuotedQuery(
-			"INSERT INTO <articles> \(<title>\) VALUES (?)",
-			$result,
-			true
-		);
+		$result = $query->execute();
+		$this->assertCount(1, $result);
+
+		$subquery = new Query($this->connection);
+		$subquery->select(['name'])
+			->from('authors')
+			->where(['id' => 1]);
+
+		$query = new Query($this->connection);
+		$query->insert(['title'])
+			->into('articles')
+			->values(['title' => $subquery]);
 		$result = $query->execute();
 		$this->assertCount(1, $result);
 	}