Fix: an SQL compiled with an empty IN clause causes a database error

Finesse · Jan 19, 2019 · 4edba72 · 4edba72
1 parent 31f57f4
commit 4edba72
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 4 deletions.
diff --git a/src/Grammars/CommonGrammar.php b/src/Grammars/CommonGrammar.php
@@ -539,11 +539,15 @@ protected function compileCriterion(Criterion $criterion, array &$bindings): str
 
         if ($criterion instanceof InCriterion) {
             if (is_array($criterion->values)) {
-                $values = [];
+                if (empty($criterion->values)) {
+                    return $this->compileEmptyInCriterion($criterion, $bindings);
+                }
+
+                $subQuery = '';
                 foreach ($criterion->values as $value) {
-                    $values[] = $this->compileValue($value, $bindings);
+                    $subQuery .= ($subQuery ? ', ' : '(') . $this->compileValue($value, $bindings);
                 }
-                $subQuery = '('.implode(', ', $values).')';
+                $subQuery .= ')';
             } else {
                 $subQuery = $this->compileSubQuery($criterion->values, $bindings);
             }
@@ -571,6 +575,20 @@ protected function compileCriterion(Criterion $criterion, array &$bindings): str
         throw new InvalidQueryException('The given criterion '.get_class($criterion).' is unknown');
     }
 
+    /**
+     * Converts an IN criterion with zero values to an SQL query text. Some SQL dialects don't support `IN ()`.
+     *
+     * @param InCriterion $criterion Criterion
+     * @param array $bindings Bound values (array is filled by link)
+     * @return string SQL text or an empty string
+     */
+    protected function compileEmptyInCriterion(InCriterion $criterion, array &$bindings): string
+    {
+        // "In empty set" is always false, "not in empty set" is always true
+        $this->mergeBindings($bindings, [$criterion->not ? 1 : 0]);
+        return '?';
+    }
+
     /**
      * Converts a single order to an SQL query text.
      *

diff --git a/src/Grammars/SQLiteGrammar.php b/src/Grammars/SQLiteGrammar.php
@@ -4,6 +4,7 @@
 
 use Finesse\QueryScribe\Exceptions\InvalidQueryException;
 use Finesse\QueryScribe\Query;
+use Finesse\QueryScribe\QueryBricks\Criteria\InCriterion;
 use Finesse\QueryScribe\StatementInterface;
 
 /**
@@ -37,6 +38,18 @@ public function compileDelete(Query $query): StatementInterface
         return parent::compileDelete($query);
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    protected function compileEmptyInCriterion(InCriterion $criterion, array &$bindings): string
+    {
+        return sprintf(
+            '%s %sIN ()',
+            $this->compileIdentifier($criterion->column, $bindings),
+            $criterion->not ? 'NOT ' : ''
+        );
+    }
+
     /**
      * {@inheritDoc}
      */

diff --git a/tests/Grammars/CommonGrammarTest.php b/tests/Grammars/CommonGrammarTest.php
@@ -425,13 +425,15 @@ public function testCompileWhere()
                     )
                 ) AND
                 (MONTH(date)) IN (?, ?, ?) AND
+                ? AND
+                ? AND
                 "position" IS NULL AND
                 "author_id" NOT IN (
                     SELECT "id"
                     FROM "users"
                     WHERE "deleted" = ?
                 )
-        ', [0, '%boss%', '\\', 'Important', 'Hello', 1, 4, 6, true], $grammar->compileSelect(
+        ', [0, '%boss%', '\\', 'Important', 'Hello', 1, 4, 6, 0, 1, true], $grammar->compileSelect(
             (new Query)
                 ->from('posts')
                 ->where('date', '<', new Raw('NOW()'))
@@ -455,6 +457,8 @@ public function testCompileWhere()
                         ->where('content', 'Hello');
                 })
                 ->whereIn(new Raw('MONTH(date)'), [1, 4, 6])
+                ->whereIn('status', [])
+                ->whereNotIn('reaction', [])
                 ->whereNull('position')
                 ->where(function () {}) // Empty group
                 ->whereNotIn('author_id', function (Query $query) {

diff --git a/tests/Grammars/SQLiteGrammarTest.php b/tests/Grammars/SQLiteGrammarTest.php
@@ -95,4 +95,25 @@ public function testCompileDelete()
             $this->assertEquals('Table alias is not allowed in delete query', $exception->getMessage());
         });
     }
+
+    /**
+     * Tests the `compileEmptyInCriterion` method
+     */
+    public function testCompileEmptyInCriterion()
+    {
+        $grammar = new SQLiteGrammar();
+
+        $this->assertStatement('
+            SELECT *
+            FROM "posts"
+            WHERE
+              "type" IN () AND
+              "status" NOT IN ()
+        ', [], $grammar->compileSelect(
+            (new Query)
+                ->table('posts')
+                ->whereIn('type', [])
+                ->whereNotIn('status', [])
+        ));
+    }
 }