Merge pull request #264 from Ocramius/fix/predicate-expressions-shoul…

…d-not-have-values-to-be-bound-when-none-given Fix: prevent bound parameters being `[null]` when no parameters are given to `Predicate#expression()`
laminas · Dec 5, 2022 · a03d8df · a03d8df
2 parents 1125ef2 + 1c5ad3a
commit a03d8df
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 2 deletions.
diff --git a/src/Sql/Predicate/Predicate.php b/src/Sql/Predicate/Predicate.php
@@ -250,7 +250,7 @@ public function notLike($identifier, $notLike)
     public function expression($expression, $parameters = null)
     {
         $this->addPredicate(
-            new Expression($expression, $parameters),
+            new Expression($expression, func_num_args() > 1 ? $parameters : []),
             $this->nextPredicateCombineOperator ?: $this->defaultCombination
         );
         $this->nextPredicateCombineOperator = null;

diff --git a/test/unit/Sql/Predicate/PredicateTest.php b/test/unit/Sql/Predicate/PredicateTest.php
@@ -2,10 +2,15 @@
 
 namespace LaminasTest\Db\Sql\Predicate;
 
+use Laminas\Db\Adapter\Platform\Sql92;
 use Laminas\Db\Sql\Expression;
 use Laminas\Db\Sql\Predicate\Predicate;
+use Laminas\Db\Sql\Select;
+use Laminas\Stdlib\ErrorHandler;
 use PHPUnit\Framework\TestCase;
 
+use const E_USER_NOTICE;
+
 class PredicateTest extends TestCase
 {
     public function testEqualToCreatesOperatorPredicate()
@@ -240,7 +245,7 @@ public function testExpressionNullParameters()
         $predicate->expression('foo = bar');
         $predicates = $predicate->getPredicates();
         $expression = $predicates[0][1];
-        self::assertEquals([null], $expression->getParameters());
+        self::assertEquals([], $expression->getParameters());
     }
 
     /**
@@ -276,4 +281,59 @@ public function testLiteral()
             $predicate->getExpressionData()
         );
     }
+
+    public function testCanCreateExpressionsWithoutAnyBoundSqlParameters(): void
+    {
+        $where1 = new Predicate();
+
+        $where1->expression('some_expression()');
+
+        self::assertSame(
+            'SELECT "a_table".* FROM "a_table" WHERE (some_expression())',
+            $this->makeSqlString($where1)
+        );
+    }
+
+    public function testWillBindSqlParametersToExpressionsWithGivenParameter(): void
+    {
+        $where = new Predicate();
+
+        $where->expression('some_expression(?)', null);
+
+        self::assertSame(
+            'SELECT "a_table".* FROM "a_table" WHERE (some_expression(\'\'))',
+            $this->makeSqlString($where)
+        );
+    }
+
+    public function testWillBindSqlParametersToExpressionsWithGivenStringParameter(): void
+    {
+        $where = new Predicate();
+
+        $where->expression('some_expression(?)', 'a string');
+
+        self::assertSame(
+            'SELECT "a_table".* FROM "a_table" WHERE (some_expression(\'a string\'))',
+            $this->makeSqlString($where)
+        );
+    }
+
+    private function makeSqlString(Predicate $where): string
+    {
+        $select = new Select('a_table');
+
+        $select->where($where);
+
+        // this is still faster than connecting to a real DB for this kind of test.
+        // we are using unsafe SQL quoting on purpose here: this raises warnings in production.
+        ErrorHandler::start(E_USER_NOTICE);
+
+        try {
+            $string = $select->getSqlString(new Sql92());
+        } finally {
+            ErrorHandler::stop();
+        }
+
+        return $string;
+    }
 }