Fixed issue with QueryExpression not parenthesizing correctly subquer…

…ies.
cakephp · May 17, 2015 · 5575940 · 5575940
1 parent 9e59980
commit 5575940
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 1 deletion.
diff --git a/src/Database/Expression/QueryExpression.php b/src/Database/Expression/QueryExpression.php
@@ -16,6 +16,7 @@
 
 use Cake\Database\ExpressionInterface;
 use Cake\Database\Expression\IdentifierExpression;
+use Cake\Database\Query;
 use Cake\Database\TypeMapTrait;
 use Cake\Database\ValueBinder;
 use Countable;
@@ -406,7 +407,9 @@ public function sql(ValueBinder $generator)
         $template = ($this->count() === 1) ? '%s' : '(%s)';
         $parts = [];
         foreach ($this->_conditions as $part) {
-            if ($part instanceof ExpressionInterface) {
+            if ($part instanceof Query) {
+                $part = '(' . $part->sql($generator) . ')';
+            } elseif ($part instanceof ExpressionInterface) {
                 $part = $part->sql($generator);
             }
             $parts[] = $part;

diff --git a/tests/TestCase/ORM/QueryRegressionTest.php b/tests/TestCase/ORM/QueryRegressionTest.php
@@ -839,4 +839,28 @@ public function testMatchingWithNoFields()
             ->toList();
         $this->assertEquals([2], $results);
     }
+
+     /**
+     * Tests that using a subquery as part of an expression will not make invalid SQL
+     *
+     * @return void
+     */
+    public function testSubqueryInSelectExpression() {
+        $table = TableRegistry::get('Comments');
+        $ratio = $table->find()
+            ->select(function ($query) use ($table) {
+                $allCommentsCount = $table->find()->select($query->func()->count('*'));
+                $countToFloat = $query->newExpr([$query->func()->count('*'), '1.0'])->type('*');
+                return [
+                    'ratio' => $query
+                        ->newExpr($countToFloat)
+                        ->add($allCommentsCount)
+                        ->type('/')
+                ];
+            })
+            ->where(['user_id' => 1])
+            ->first()
+            ->ratio;
+        $this->assertEquals(0.5, $ratio);
+    }
 }