From 4f61cc61322f496441a7f892299b312d0ce05d97 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Tue, 13 Oct 2026 10:25:01 +1100 Subject: [PATCH] Bind Select parameters in placeholder order for positional drivers processSelect() resolved the FROM table before the columns and the quantifier after them, so a FROM sub-select's parameters were bound ahead of column and quantifier parameters that appear earlier in the SQL. Positional drivers bind getPositionalArray() in insertion order, so values landed on the wrong placeholders. The columns and the FROM table now bind into their own containers and are appended after the quantifier in text order. Processing order is unchanged, so parameter names and SQL text are identical. --- src/Sql/Select.php | 48 ++++- test/unit/Sql/SelectParameterOrderTest.php | 233 +++++++++++++++++++++ 2 files changed, 277 insertions(+), 4 deletions(-) create mode 100644 test/unit/Sql/SelectParameterOrderTest.php diff --git a/src/Sql/Select.php b/src/Sql/Select.php index 183750be..27836f02 100644 --- a/src/Sql/Select.php +++ b/src/Sql/Select.php @@ -12,6 +12,7 @@ use function array_key_exists; use function array_key_first; +use function array_keys; use function count; use function explode; use function gettype; @@ -617,6 +618,10 @@ protected function processOrder( /** * Process the select part * + * The FROM table is resolved before the columns and the quantifier after them, so the columns and + * the table bind into their own containers and are appended after the quantifier, keeping the + * parameters in the order their placeholders appear in the SQL. + * * @return list>> * @throws Exception\RuntimeException */ @@ -627,7 +632,10 @@ protected function processSelect( ): array { $expr = 1; - [$table, $fromTable] = $this->resolveSelectTable($this->table, $platform, $driver, $parameterContainer); + $tableParameters = $parameterContainer instanceof ParameterContainer ? new ParameterContainer() : null; + $columnParameters = $parameterContainer instanceof ParameterContainer ? new ParameterContainer() : null; + + [$table, $fromTable] = $this->resolveSelectTable($this->table, $platform, $driver, $tableParameters); $columns = []; foreach ($this->columns as $columnIndexOrAs => $column) { if (self::SQL_STAR === $column) { @@ -643,7 +651,7 @@ protected function processSelect( ], $platform, $driver, - $parameterContainer, + $columnParameters, is_string($columnIndexOrAs) ? $columnIndexOrAs : 'column', ); $columnAs = match (true) { @@ -659,7 +667,7 @@ protected function processSelect( foreach ($this->getJoins()->getJoins() as $join) { $joinName = is_array($join['name']) ? array_key_first($join['name']) : $join['name']; - $joinName = $this->resolveTable($joinName, $platform, $driver, $parameterContainer); + $joinName = $this->resolveTable($joinName, $platform, $driver, $columnParameters); foreach ($join['columns'] as $jKey => $jColumn) { $jColumns = []; @@ -674,7 +682,7 @@ protected function processSelect( ], $platform, $driver, - $parameterContainer, + $columnParameters, is_string($jKey) ? $jKey : 'column', ); $jColumnAs = match (true) { @@ -699,6 +707,8 @@ protected function processSelect( : $this->quantifier; } + $this->appendParameters($parameterContainer, $columnParameters, $tableParameters); + if (null === $table) { return [$columns]; } @@ -748,6 +758,36 @@ protected function processWhere( ]; } + /** + * Append the parameters of separately bound parts, in the order given + */ + private function appendParameters(?ParameterContainer $target, ?ParameterContainer ...$sources): void + { + foreach ($sources as $source) { + if (null === $target || null === $source) { + continue; + } + + $this->copyParameters($source, $target); + } + } + + /** + * Copy each parameter with its type and maximum length, keeping their order + */ + private function copyParameters(ParameterContainer $source, ParameterContainer $target): void + { + foreach (array_keys($source->getNamedArray()) as $name) { + $name = (string) $name; + $target->offsetSet( + $name, + $source->offsetGet($name), + $source->offsetHasErrata($name) ? $source->offsetGetErrata($name) : null, + $source->offsetHasMaxLength($name) ? $source->offsetGetMaxLength($name) : null, + ); + } + } + private function getHaving(): Having { return $this->having ??= new Having(); diff --git a/test/unit/Sql/SelectParameterOrderTest.php b/test/unit/Sql/SelectParameterOrderTest.php new file mode 100644 index 00000000..b4f25c53 --- /dev/null +++ b/test/unit/Sql/SelectParameterOrderTest.php @@ -0,0 +1,233 @@ +}> + */ + public static function outOfOrderSelectProvider(): array + { + return [ + 'quantifier before column' => [ + static fn(): Select => (new Select('t'))->columns(['c' => new Expression('?', ['COLUMN'])]) + ->quantifier(new Expression('TOP ?', ['QUANTIFIER'])), + 'SELECT TOP ? ? AS "c" FROM "t"', + ['QUANTIFIER', 'COLUMN'], + ], + 'column before from sub-select' => [ + static fn(): Select => (new Select(['t' => (new Select('src'))->where(['x' => 'FROM'])])) + ->columns(['c' => new Expression('?', ['COLUMN'])]), + 'SELECT ? AS "c" FROM (SELECT "src".* FROM "src" WHERE "x" = ?) AS "t"', + ['COLUMN', 'FROM'], + ], + 'quantifier before from sub-select' => [ + static fn(): Select => (new Select(['t' => (new Select('src'))->where(['x' => 'FROM'])])) + ->quantifier(new Expression('TOP ?', ['QUANTIFIER'])), + 'SELECT TOP ? "t".* FROM (SELECT "src".* FROM "src" WHERE "x" = ?) AS "t"', + ['QUANTIFIER', 'FROM'], + ], + 'join column before from sub-select' => [ + static fn(): Select => (new Select(['t' => (new Select('src'))->where(['x' => 'FROM'])])) + ->join(['j' => 'jt'], 'j.id = t.id', ['jc' => new Expression('?', ['JOIN-COLUMN'])]), + 'SELECT "t".*, ? AS "jc" FROM (SELECT "src".* FROM "src" WHERE "x" = ?) AS "t"' + . ' INNER JOIN "jt" AS "j" ON "j"."id" = "t"."id"', + ['JOIN-COLUMN', 'FROM'], + ], + 'column sub-select before from sub-select' => [ + static fn(): Select => (new Select(['t' => (new Select('src'))->where(['x' => 'FROM'])])) + ->columns(['c' => (new Select('cs'))->where(['y' => 'COLUMN-SUB-SELECT'])]), + 'SELECT (SELECT "cs".* FROM "cs" WHERE "y" = ?) AS "c"' + . ' FROM (SELECT "src".* FROM "src" WHERE "x" = ?) AS "t"', + ['COLUMN-SUB-SELECT', 'FROM'], + ], + 'quantifier, column and from sub-select' => [ + static fn(): Select => (new Select(['t' => (new Select('src'))->where([ + 'x' => 'FROM', + ])]))->columns(['c' => new Expression('?', ['COLUMN'])]) + ->quantifier(new Expression('TOP ?', ['QUANTIFIER'])), + 'SELECT TOP ? ? AS "c" FROM (SELECT "src".* FROM "src" WHERE "x" = ?) AS "t"', + ['QUANTIFIER', 'COLUMN', 'FROM'], + ], + 'nested from sub-select' => [ + static fn(): Select => new Select([ + 'outer' => (new Select(['inner' => (new Select('src'))->where(['x' => 'INNER-FROM'])])) + ->columns(['c' => new Expression('?', ['OUTER-COLUMN'])]), + ]), + 'SELECT "outer".* FROM (SELECT ? AS "c"' + . ' FROM (SELECT "src".* FROM "src" WHERE "x" = ?) AS "inner") AS "outer"', + ['OUTER-COLUMN', 'INNER-FROM'], + ], + 'every clause' => [ + static fn(): Select => (new Select(['t' => (new Select('src'))->where([ + 'x' => 'FROM', + ])]))->columns(['c' => new Expression('?', ['COLUMN'])]) + ->quantifier(new Expression('TOP ?', ['QUANTIFIER'])) + ->join( + ['j' => (new Select('js'))->where(['z' => 'JOIN-TABLE'])], + new Operator('j.k', Operator::OPERATOR_EQUAL_TO, 'JOIN-ON'), + ['jc' => new Expression('?', ['JOIN-COLUMN'])], + ) + ->where(['w' => 'WHERE']) + ->group(new Expression('?', ['GROUP'])) + ->having(['h' => 'HAVING']) + ->order(new Expression('?', ['ORDER'])) + ->limit(5) + ->offset(10), + 'SELECT TOP ? ? AS "c", ? AS "jc" FROM (SELECT "src".* FROM "src" WHERE "x" = ?) AS "t"' + . ' INNER JOIN (SELECT "js".* FROM "js" WHERE "z" = ?) AS "j" ON "j"."k" = ?' + . ' WHERE "w" = ? GROUP BY ? HAVING "h" = ? ORDER BY ? LIMIT ? OFFSET ?', + [ + 'QUANTIFIER', + 'COLUMN', + 'JOIN-COLUMN', + 'FROM', + 'JOIN-TABLE', + 'JOIN-ON', + 'WHERE', + 'GROUP', + 'HAVING', + 'ORDER', + 5, + 10, + ], + ], + ]; + } + + #[Test] + public function appendsAfterParametersAlreadyInTheContainer(): void + { + $select = (new Select(['t' => (new Select('src'))->where(['x' => 'FROM'])])) + ->columns(['c' => new Expression('?', ['COLUMN'])]); + + $parameterContainer = new ParameterContainer(['existing' => 'EXISTING']); + + $this->prepare($select, $this->createDriver('positional'), $parameterContainer); + + static::assertSame(['EXISTING', 'COLUMN', 'FROM'], $parameterContainer->getPositionalArray()); + } + + /** + * @param Closure(): Select $build + * @param list $expectedValues + */ + #[DataProvider('outOfOrderSelectProvider')] + #[Test] + public function bindsPositionalParametersInPlaceholderOrder( + Closure $build, + string $expectedSql, + array $expectedValues, + ): void { + $parameterContainer = new ParameterContainer(); + + $sql = $this->prepare($build(), $this->createDriver('positional'), $parameterContainer); + + static::assertSame( + [$expectedSql, $expectedValues], + [$sql, $parameterContainer->getPositionalArray()], + ); + } + + #[Test] + public function keepsNamedParameterNamesAndSql(): void + { + $select = (new Select(['t' => (new Select('src'))->where(['x' => 'FROM'])]))->columns([ + 'c' => new Expression('?', ['COLUMN']), + 'sq' => (new Select('cs'))->where(['y' => 'COLUMN-SUB-SELECT']), + ]) + ->quantifier(new Expression('TOP ?', ['QUANTIFIER'])) + ->join(['j' => 'jt'], 'j.id = t.id', ['jc' => new Expression('?', ['JOIN-COLUMN'])]) + ->where(['w' => 'WHERE']) + ->limit(5) + ->offset(10); + + $parameterContainer = new ParameterContainer(); + + $sql = $this->prepare($select, $this->createDriver('named'), $parameterContainer); + + $namedParameters = $parameterContainer->getNamedArray(); + ksort($namedParameters); + + static::assertSame( + [ + 'SELECT TOP :quantifier1 :c1 AS "c", (SELECT "cs".* FROM "cs" WHERE "y" = :subselect2where1) AS "sq",' + . ' :jc1 AS "jc" FROM (SELECT "src".* FROM "src" WHERE "x" = :subselect1where1) AS "t"' + . ' INNER JOIN "jt" AS "j" ON "j"."id" = "t"."id" WHERE "w" = :where1' + . ' LIMIT :limit OFFSET :offset', + [ + 'c1' => 'COLUMN', + 'jc1' => 'JOIN-COLUMN', + 'limit' => 5, + 'offset' => 10, + 'quantifier1' => 'QUANTIFIER', + 'subselect1where1' => 'FROM', + 'subselect2where1' => 'COLUMN-SUB-SELECT', + 'where1' => 'WHERE', + ], + ], + [$sql, $namedParameters], + ); + } + + #[Test] + public function keepsParameterTypeOfReorderedParameters(): void + { + $select = (new Select(['t' => (new Select('src'))->limit(5)])) + ->columns(['c' => new Expression('?', ['COLUMN'])]); + + $parameterContainer = new ParameterContainer(); + + $this->prepare($select, $this->createDriver('positional'), $parameterContainer); + + static::assertSame( + [['COLUMN', 5], ParameterContainer::TYPE_INTEGER], + [$parameterContainer->getPositionalArray(), $parameterContainer->offsetGetErrata('subselect1limit')], + ); + } + + private function createDriver(string $prepareType): DriverInterface + { + $formatParameterName = static fn(string $name): string => 'named' === $prepareType ? ":{$name}" : '?'; + + $driver = $this->createStub(DriverInterface::class); + $driver->method('getPrepareType')->willReturn($prepareType); + $driver->method('formatParameterName')->willReturnCallback($formatParameterName); + + return $driver; + } + + private function prepare(Select $select, DriverInterface $driver, ParameterContainer $parameterContainer): string + { + $adapter = $this->createStub(AdapterInterface::class); + $adapter->method('getPlatform')->willReturn(new TrustingSql92Platform()); + $adapter->method('getDriver')->willReturn($driver); + + return (string) $select->prepareStatement($adapter, new StatementContainer(null, $parameterContainer)) + ->getSql(); + } +}