Support questionmark placeholders better

vimeo · Feb 2, 2021 · c5fcbad · c5fcbad
1 parent c387df2
commit c5fcbad
Show file tree

Hide file tree

Showing 12 changed files with 85 additions and 43 deletions.
diff --git a/src/FakePdoTrait.php b/src/FakePdoTrait.php
@@ -75,6 +75,9 @@ public function bindValue($key, $value, $type = \PDO::PARAM_STR) : void
     {
         if (\is_string($key) && $key[0] !== ':') {
             $key = ':' . $key;
+        } elseif (\is_int($key)) {
+            // Parameter offsets start at 1, which is weird.
+            --$key;
         }
 
         $this->boundValues[$key] = $value;
@@ -137,7 +140,7 @@ public function universalExecute(?array $params = null)
                 try {
                     $raw_result = Processor\SelectProcessor::process(
                         $this->conn,
-                        new Processor\Scope($this->boundValues),
+                        new Processor\Scope(array_merge($params ?? [], $this->boundValues)),
                         $parsed_query
                     );
                 } catch (Processor\ProcessorException $runtime_exception) {
@@ -185,7 +188,7 @@ function ($row) {
             case Query\InsertQuery::class:
                 $this->affectedRows = Processor\InsertProcessor::process(
                     $this->conn,
-                    new Processor\Scope($this->boundValues),
+                    new Processor\Scope(array_merge($params ?? [], $this->boundValues)),
                     $parsed_query
                 );
 
@@ -194,7 +197,7 @@ function ($row) {
             case Query\UpdateQuery::class:
                 $this->affectedRows = Processor\UpdateProcessor::process(
                     $this->conn,
-                    new Processor\Scope($this->boundValues),
+                    new Processor\Scope(array_merge($params ?? [], $this->boundValues)),
                     $parsed_query
                 );
 
@@ -203,7 +206,7 @@ function ($row) {
             case Query\DeleteQuery::class:
                 $this->affectedRows = Processor\DeleteProcessor::process(
                     $this->conn,
-                    new Processor\Scope($this->boundValues),
+                    new Processor\Scope(array_merge($params ?? [], $this->boundValues)),
                     $parsed_query
                 );
 

diff --git a/src/Parser/ExpressionParser.php b/src/Parser/ExpressionParser.php
@@ -14,8 +14,8 @@
 use Vimeo\MysqlEngine\Query\Expression\InOperatorExpression;
 use Vimeo\MysqlEngine\Query\Expression\IntervalOperatorExpression;
 use Vimeo\MysqlEngine\Query\Expression\StubExpression;
-use Vimeo\MysqlEngine\Query\Expression\ParameterExpression;
-use Vimeo\MysqlEngine\Query\Expression\PlaceholderExpression;
+use Vimeo\MysqlEngine\Query\Expression\NamedPlaceholderExpression;
+use Vimeo\MysqlEngine\Query\Expression\QuestionMarkPlaceholderExpression;
 use Vimeo\MysqlEngine\Query\Expression\PositionExpression;
 use Vimeo\MysqlEngine\Query\Expression\RowExpression;
 use Vimeo\MysqlEngine\Query\Expression\SubqueryExpression;
@@ -228,15 +228,15 @@ public function tokenToExpression(Token $token)
                 }
 
                 if ($token->value === '?') {
-                    if ($token->parameterName === null) {
-                        if ($token->parameterOffset !== null) {
-                            return new PlaceholderExpression($token, $token->parameterOffset);
-                        }
+                    if ($token->parameterOffset !== null) {
+                        return new QuestionMarkPlaceholderExpression($token, $token->parameterOffset);
+                    }
 
-                        throw new ParserException('? encountered with unknown offset');
+                    if ($token->parameterName !== null) {
+                        return new NamedPlaceholderExpression($token, $token->parameterName);
                     }
 
-                    return new ParameterExpression($token, $token->parameterName);
+                    throw new ParserException('? encountered with unknown offset');
                 }
 
                 if ($token->value[0] === '@') {

diff --git a/src/Parser/LimitParser.php b/src/Parser/LimitParser.php
@@ -3,7 +3,8 @@
 
 use Vimeo\MysqlEngine\TokenType;
 use Vimeo\MysqlEngine\Query\Expression\ConstantExpression;
-use Vimeo\MysqlEngine\Query\Expression\ParameterExpression;
+use Vimeo\MysqlEngine\Query\Expression\NamedPlaceholderExpression;
+use Vimeo\MysqlEngine\Query\Expression\QuestionMarkPlaceholderExpression;
 use Vimeo\MysqlEngine\Query\LimitClause;
 
 final class LimitParser
@@ -45,7 +46,13 @@ public function parse()
         if ($next->type === TokenType::NUMERIC_CONSTANT) {
             $limit = new ConstantExpression($next);
         } elseif ($next->type === TokenType::IDENTIFIER && $next->value === '?') {
-            $limit = new ParameterExpression($next, $next->parameterName);
+            if ($next->parameterOffset !== null) {
+                $limit = new QuestionMarkPlaceholderExpression($next, $next->parameterOffset);
+            } elseif ($next->parameterName !== null) {
+                $limit = new NamedPlaceholderExpression($next, $next->parameterName);
+            } else {
+                throw new ParserException('? encountered with unknown offset');
+            }
         } else {
             throw new ParserException("Expected integer or parameter after OFFSET");
         }
@@ -64,7 +71,7 @@ public function parse()
                 if ($next->type === TokenType::NUMERIC_CONSTANT) {
                     $offset = new ConstantExpression($next);
                 } elseif ($next->type === TokenType::IDENTIFIER && $next->value === '?') {
-                    $offset = new ParameterExpression($next, $next->parameterName);
+                    $offset = new NamedPlaceholderExpression($next, $next->parameterName);
                 } else {
                     throw new ParserException("Expected integer or parameter after OFFSET");
                 }
@@ -81,7 +88,7 @@ public function parse()
                     if ($next->type === TokenType::NUMERIC_CONSTANT) {
                         $limit = new ConstantExpression($next);
                     } elseif ($next->type === TokenType::IDENTIFIER && $next->value === '?') {
-                        $limit = new ParameterExpression($next, $next->parameterName);
+                        $limit = new NamedPlaceholderExpression($next, $next->parameterName);
                     } else {
                         throw new ParserException("Expected integer or parameter after OFFSET");
                     }

diff --git a/src/Parser/SQLParser.php b/src/Parser/SQLParser.php
@@ -209,8 +209,7 @@ private static function buildTokenListFromLexemes(array $tokens)
         $out = [];
         $count = \count($tokens);
 
-        // all parameters in PDO MySQL start at 1. I know, it's weird.
-        $parameter_offset = 1;
+        $parameter_offset = 0;
 
         foreach ($tokens as $i => [$token, $start]) {
             $trimmed_token = \trim($token);

diff --git a/src/Processor/Expression/Evaluator.php b/src/Processor/Expression/Evaluator.php
@@ -90,15 +90,11 @@ public static function evaluate(
             case \Vimeo\MysqlEngine\Query\Expression\VariableExpression::class:
                 return VariableEvaluator::evaluate($scope, $expr);
 
-            case \Vimeo\MysqlEngine\Query\Expression\ParameterExpression::class:
-                return ParameterEvaluator::evaluate($scope, $expr);
+            case \Vimeo\MysqlEngine\Query\Expression\NamedPlaceholderExpression::class:
+                return NamedPlaceholderEvaluator::evaluate($scope, $expr);
 
-            case \Vimeo\MysqlEngine\Query\Expression\PlaceholderExpression::class:
-                if (\array_key_exists($expr->offset, $scope->parameters)) {
-                    return $scope->parameters[$expr->offset];
-                }
-
-                throw new ProcessorException('Parameter offset ' . $expr->offset . ' out of range');
+            case \Vimeo\MysqlEngine\Query\Expression\QuestionMarkPlaceholderExpression::class:
+                return QuestionMarkPlaceholderEvaluator::evaluate($scope, $expr);
 
             default:
                 throw new ProcessorException('Unsupported expression ' . get_class($expr));
@@ -231,7 +227,7 @@ public static function getColumnSchema(
                 // it defaults to string
                 return new Column\Varchar(10);
 
-            case \Vimeo\MysqlEngine\Query\Expression\ParameterExpression::class:
+            case \Vimeo\MysqlEngine\Query\Expression\NamedPlaceholderExpression::class:
                 if (\array_key_exists($expr->parameterName, $scope->parameters)) {
                     return self::getColumnTypeFromValue($expr, $scope->parameters[$expr->parameterName]);
                 }
@@ -240,7 +236,7 @@ public static function getColumnSchema(
                 // it defaults to string
                 return new Column\Varchar(10);
 
-            case \Vimeo\MysqlEngine\Query\Expression\PlaceholderExpression::class:
+            case \Vimeo\MysqlEngine\Query\Expression\QuestionMarkPlaceholderExpression::class:
                 if (\array_key_exists($expr->offset, $scope->parameters)) {
                     return self::getColumnTypeFromValue($expr, $scope->parameters[$expr->offset]);
                 }

diff --git a/...ocessor/Expression/ParameterEvaluator.php → .../Expression/NamedPlaceholderEvaluator.php b/...ocessor/Expression/ParameterEvaluator.php → .../Expression/NamedPlaceholderEvaluator.php
@@ -2,15 +2,15 @@
 namespace Vimeo\MysqlEngine\Processor\Expression;
 
 use Vimeo\MysqlEngine\Processor\ProcessorException;
-use Vimeo\MysqlEngine\Query\Expression\ParameterExpression;
+use Vimeo\MysqlEngine\Query\Expression\NamedPlaceholderExpression;
 use Vimeo\MysqlEngine\Processor\Scope;
 
-final class ParameterEvaluator
+final class NamedPlaceholderEvaluator
 {
     /**
      * @return mixed
      */
-    public static function evaluate(Scope $scope, ParameterExpression $expr)
+    public static function evaluate(Scope $scope, NamedPlaceholderExpression $expr)
     {
         if (\array_key_exists($expr->parameterName, $scope->parameters)) {
             return $scope->parameters[$expr->parameterName];

diff --git a/src/Processor/Expression/QuestionMarkPlaceholderEvaluator.php b/src/Processor/Expression/QuestionMarkPlaceholderEvaluator.php
@@ -0,0 +1,21 @@
+<?php
+namespace Vimeo\MysqlEngine\Processor\Expression;
+
+use Vimeo\MysqlEngine\Processor\ProcessorException;
+use Vimeo\MysqlEngine\Query\Expression\QuestionMarkPlaceholderExpression;
+use Vimeo\MysqlEngine\Processor\Scope;
+
+final class QuestionMarkPlaceholderEvaluator
+{
+    /**
+     * @return mixed
+     */
+    public static function evaluate(Scope $scope, QuestionMarkPlaceholderExpression $expr)
+    {
+        if (\array_key_exists($expr->offset, $scope->parameters)) {
+            return $scope->parameters[$expr->offset];
+        }
+
+        throw new ProcessorException('Parameter offset ' . $expr->offset . ' out of range');
+    }
+}
diff --git a/src/Processor/Processor.php b/src/Processor/Processor.php
@@ -5,6 +5,7 @@
 use Vimeo\MysqlEngine\Query\Expression\BinaryOperatorExpression;
 use Vimeo\MysqlEngine\Query\Expression\ColumnExpression;
 use Vimeo\MysqlEngine\Query\Expression\ConstantExpression;
+use Vimeo\MysqlEngine\Query\Expression\NamedPlaceholderExpression;
 use Vimeo\MysqlEngine\Query\LimitClause;
 use Vimeo\MysqlEngine\Schema\Column\IntegerColumn;
 use Vimeo\MysqlEngine\Schema\TableDefinition;
@@ -109,14 +110,18 @@ protected static function applyLimit(?LimitClause $limit, Scope $scope, QueryRes
             $offset = 0;
         } elseif ($limit->offset instanceof ConstantExpression) {
             $offset = (int) $limit->offset->value;
+        } elseif ($limit->offset instanceof NamedPlaceholderExpression) {
+            $offset = (int) Expression\NamedPlaceholderEvaluator::evaluate($scope, $limit->offset);
         } else {
-            $offset = (int) Expression\ParameterEvaluator::evaluate($scope, $limit->offset);
+            $offset = (int) Expression\QuestionMarkPlaceholderEvaluator::evaluate($scope, $limit->offset);
         }
 
         if ($limit->rowcount instanceof ConstantExpression) {
             $rowcount = (int) $limit->rowcount->value;
+        } elseif ($limit->rowcount instanceof NamedPlaceholderExpression) {
+            $rowcount = (int) Expression\NamedPlaceholderEvaluator::evaluate($scope, $limit->rowcount);
         } else {
-            $rowcount = (int) Expression\ParameterEvaluator::evaluate($scope, $limit->rowcount);
+            $rowcount = (int) Expression\QuestionMarkPlaceholderEvaluator::evaluate($scope, $limit->rowcount);
         }
 
         return new QueryResult(

diff --git a/src/Query/Expression/ParameterExpression.php → ...Expression/NamedPlaceholderExpression.php b/src/Query/Expression/ParameterExpression.php → ...Expression/NamedPlaceholderExpression.php
@@ -4,7 +4,7 @@
 use Vimeo\MysqlEngine\Parser\Token;
 use Vimeo\MysqlEngine\TokenType;
 
-final class ParameterExpression extends Expression
+final class NamedPlaceholderExpression extends Expression
 {
     /**
      * @var string

diff --git a/...uery/Expression/PlaceholderExpression.php → ...ion/QuestionMarkPlaceholderExpression.php b/...uery/Expression/PlaceholderExpression.php → ...ion/QuestionMarkPlaceholderExpression.php
@@ -4,7 +4,7 @@
 use Vimeo\MysqlEngine\Parser\Token;
 use Vimeo\MysqlEngine\TokenType;
 
-final class PlaceholderExpression extends Expression
+final class QuestionMarkPlaceholderExpression extends Expression
 {
     /**
      * @var int

diff --git a/src/Query/LimitClause.php b/src/Query/LimitClause.php
@@ -3,19 +3,20 @@
 
 use Vimeo\MysqlEngine\Query\Expression\ConstantExpression;
 use Vimeo\MysqlEngine\Query\Expression\Expression;
-use Vimeo\MysqlEngine\Query\Expression\ParameterExpression;
+use Vimeo\MysqlEngine\Query\Expression\NamedPlaceholderExpression;
+use Vimeo\MysqlEngine\Query\Expression\QuestionMarkPlaceholderExpression;
 
 final class LimitClause
 {
-    /** @var ConstantExpression|ParameterExpression|null */
+    /** @var ConstantExpression|NamedPlaceholderExpression|QuestionMarkPlaceholderExpression|null */
     public $offset;
 
-    /** @var ConstantExpression|ParameterExpression */
+    /** @var ConstantExpression|NamedPlaceholderExpression|QuestionMarkPlaceholderExpression */
     public $rowcount;
 
     /**
-     * @param ConstantExpression|ParameterExpression $offset
-     * @param ConstantExpression|ParameterExpression $rowcount
+     * @param ConstantExpression|NamedPlaceholderExpression|QuestionMarkPlaceholderExpression $offset
+     * @param ConstantExpression|NamedPlaceholderExpression|QuestionMarkPlaceholderExpression $rowcount
      */
     public function __construct(?Expression $offset, Expression $rowcount)
     {

diff --git a/tests/EndToEndTest.php b/tests/EndToEndTest.php
@@ -70,14 +70,24 @@ public function testPlaceholders()
     {
         $pdo = self::getConnectionToFullDB(false);
 
-        $query = $pdo->prepare("SELECT id FROM `video_game_characters` WHERE `id` > ? ORDER BY `id` ASC");
+        $query = $pdo->prepare("SELECT id FROM `video_game_characters` WHERE `id` > ? ORDER BY `id` ASC LIMIT ?");
         $query->bindValue(1, 14);
+        $query->bindValue(2, 1);
         $query->execute();
 
         $this->assertSame(
             [
-                ['id' => 15],
-                ['id' => 16]
+                ['id' => 15]
+            ],
+            $query->fetchAll(\PDO::FETCH_ASSOC)
+        );
+
+        $query = $pdo->prepare("SELECT id FROM `video_game_characters` WHERE `id` > ? ORDER BY `id` ASC LIMIT ?");
+        $query->execute([14, 1]);
+
+        $this->assertSame(
+            [
+                ['id' => 15]
             ],
             $query->fetchAll(\PDO::FETCH_ASSOC)
         );