Throwing exception when passing empty list of values as condition

Fixes #7092
cakephp · Nov 2, 2015 · e196c1d · e196c1d
1 parent a448f49
commit e196c1d
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 4 deletions.
diff --git a/src/Database/Expression/Comparison.php b/src/Database/Expression/Comparison.php
@@ -14,6 +14,7 @@
  */
 namespace Cake\Database\Expression;
 
+use Cake\Database\Exception as DatabaseException;
 use Cake\Database\ExpressionInterface;
 use Cake\Database\ValueBinder;
 
@@ -189,9 +190,12 @@ protected function _stringExpression($generator)
             $value = $this->_flattenValue($this->_value, $generator, $type);
 
             // To avoid SQL errors when comparing a field to a list of empty values,
-            // generate a condition that will always evaluate to false
+            // better just throw an exception here
             if ($value === '') {
-                return ['1 != 1', ''];
+                $field = $this->_field instanceof ExpressionInterface ? $this->_field->sql($generator) : $this->_field;
+                throw new DatabaseException(
+                    "Imposible to generate condition with empty list of values for field ($field)"
+                );
             }
         } else {
             $template .= '%s %s';

diff --git a/src/ORM/Marshaller.php b/src/ORM/Marshaller.php
@@ -368,6 +368,10 @@ protected function _belongsToMany(Association $assoc, array $data, $options = []
      */
     protected function _loadAssociatedByIds($assoc, $ids)
     {
+        if (empty($ids)) {
+            return [];
+        }
+
         $target = $assoc->target();
         $primaryKey = (array)$target->primaryKey();
         $multi = count($primaryKey) > 1;

diff --git a/tests/TestCase/Database/QueryTest.php b/tests/TestCase/Database/QueryTest.php
@@ -636,8 +636,10 @@ public function testSelectWhereArrayType()
 
     /**
      * Tests that passing an empty array type to any where condition will not
-     * result in an error, but in an empty result set
+     * result in a SQL error, but an internal exception
      *
+     * @expectedException Cake\Database\Exception
+     * @expectedExceptionMessage Imposible to generate condition with empty list of values for field (id)
      * @return void
      */
     public function testSelectWhereArrayTypeEmpty()
@@ -648,7 +650,24 @@ public function testSelectWhereArrayTypeEmpty()
             ->from('comments')
             ->where(['id' => []], ['id' => 'integer[]'])
             ->execute();
-        $this->assertCount(0, $result);
+    }
+
+    /**
+     * Tests exception message for impossible condition when using an expression
+     * @expectedException Cake\Database\Exception
+     * @expectedExceptionMessage with empty list of values for field (SELECT 1)
+     * @return void
+     */
+    public function testSelectWhereArrayTypeEmptyWithExpression()
+    {
+        $query = new Query($this->connection);
+        $result = $query
+            ->select(['id'])
+            ->from('comments')
+            ->where(function ($exp, $q) {
+                return $exp->in($q->newExpr('SELECT 1'), []);
+            })
+            ->execute();
     }
 
     /**

diff --git a/tests/TestCase/ORM/MarshallerTest.php b/tests/TestCase/ORM/MarshallerTest.php
@@ -1083,6 +1083,14 @@ public function testOneGenerateBelongsToManyEntitiesFromIds()
         $result = $marshall->one($data, ['associated' => ['Tags']]);
         $this->assertCount(0, $result->tags);
 
+        $data = [
+            'title' => 'Haz tags',
+            'body' => 'Some content here',
+            'tags' => ['_ids' => []]
+        ];
+        $result = $marshall->one($data, ['associated' => ['Tags']]);
+        $this->assertCount(0, $result->tags);
+
         $data = [
             'title' => 'Haz tags',
             'body' => 'Some content here',