diff --git a/src/Sql/AbstractSql.php b/src/Sql/AbstractSql.php index 5ea9aaad..d3693cdd 100644 --- a/src/Sql/AbstractSql.php +++ b/src/Sql/AbstractSql.php @@ -17,12 +17,15 @@ use PhpDb\Sql\Argument\Values; use PhpDb\Sql\Platform\PlatformDecoratorInterface; +use function array_key_exists; use function array_key_first; use function count; use function get_object_vars; use function implode; use function is_array; use function is_string; +use function preg_match; +use function preg_replace; use function rtrim; use function str_replace; use function strtoupper; @@ -473,6 +476,48 @@ protected function resolveColumnValue( : $platform->quoteValue((string) $column); } + /** + * Map column names to valid named-placeholder names + * + * Names made only of word characters are kept; any other name has each other character + * replaced by an underscore and is suffixed until it collides with no other name. + * + * @param list $columns + * @return array + */ + protected function resolveParameterNames(array $columns): array + { + $used = []; + foreach ($columns as $column) { + if (preg_match('/^[a-zA-Z0-9_]+$/', $column) !== 1) { + continue; + } + + $used[$column] = true; + } + + $names = []; + foreach ($columns as $column) { + if (array_key_exists($column, $used)) { + $names[$column] = $column; + continue; + } + + $base = (string) preg_replace('/[^a-zA-Z0-9_]/', replacement: '_', subject: $column); + $name = $base; + $suffix = 0; + while (array_key_exists($name, $used)) { + $suffix++; + $name = "{$base}_{$suffix}"; + } + + $used[$name] = true; + $names[$column] = $name; + } + + return $names; + } + /** * @return ($table is null ? null : string) * @throws Exception\RuntimeException diff --git a/src/Sql/Insert.php b/src/Sql/Insert.php index ab76640d..708fbdc6 100644 --- a/src/Sql/Insert.php +++ b/src/Sql/Insert.php @@ -19,6 +19,7 @@ use function implode; use function is_scalar; use function range; +use function strval; /** * @api @@ -171,22 +172,24 @@ protected function processInsert( throw Exception\InvalidArgumentException::forMissingValuesOrSelect(); } - $columns = []; - $values = []; - $i = 0; - $isPdoDriver = $driver instanceof PdoDriverInterface; + $columns = []; + $values = []; + $i = 0; + $isPdoDriver = $driver instanceof PdoDriverInterface; + $parameterNames = $this->resolveParameterNames(array_map(strval(...), array_keys($this->columns))); foreach ($this->columns as $column => $value) { $columns[] = $platform->quoteIdentifier((string) $column); if (is_scalar($value) && $parameterContainer && $driver instanceof DriverInterface) { // use incremental value instead of column name for PDO // @see https://github.com/zendframework/zend-db/issues/35 + $name = $parameterNames[(string) $column] ?? (string) $column; if ($isPdoDriver) { - $column = 'c_' . $i++; + $name = 'c_' . $i++; } - $values[] = $driver->formatParameterName((string) $column); - $parameterContainer->offsetSet($column, $value); + $values[] = $driver->formatParameterName($name); + $parameterContainer->offsetSet($name, $value); continue; } diff --git a/src/Sql/Update.php b/src/Sql/Update.php index ee9e6020..1552fedd 100644 --- a/src/Sql/Update.php +++ b/src/Sql/Update.php @@ -13,11 +13,15 @@ use PhpDb\Sql\Predicate\PredicateInterface; use function array_key_exists; +use function array_keys; +use function array_map; +use function explode; use function implode; use function is_numeric; use function is_scalar; use function is_string; use function strtolower; +use function strval; /** * @api @@ -188,32 +192,25 @@ protected function processSet( ?DriverInterface $driver = null, ?ParameterContainer $parameterContainer = null, ): string { - $setSql = []; - $i = 0; - $isPdoDriver = $driver instanceof PdoDriverInterface; - - foreach ($this->getSet() as $column => $value) { - $prefix = $this->resolveColumnValue( - [ - 'column' => $column, - 'fromTable' => '', - 'isIdentifier' => true, - ], - $platform, - $driver, - $parameterContainer, - 'column', - ); + $setSql = []; + $i = 0; + $isPdoDriver = $driver instanceof PdoDriverInterface; + $set = $this->getSet(); + $parameterNames = $this->resolveParameterNames(array_map(strval(...), array_keys($set->toArray()))); + + foreach ($set as $column => $value) { + $prefix = $this->quoteSetColumn($column, $platform); $prefix .= ' = '; if (is_scalar($value) && $parameterContainer && $driver instanceof DriverInterface) { // use incremental value instead of column name for PDO // @see https://github.com/zendframework/zend-db/issues/35 + $name = $parameterNames[$column] ?? $column; if ($isPdoDriver) { - $column = 'c_' . $i++; + $name = 'c_' . $i++; } - $setSql[] = $prefix . $driver->formatParameterName($column); - $parameterContainer->offsetSet($column, $value); + $setSql[] = $prefix . $driver->formatParameterName($name); + $parameterContainer->offsetSet($name, $value); continue; } @@ -286,6 +283,17 @@ private function getWhere(): Where return $this->where ??= new Where(); } + /** + * Quote a SET target as an identifier chain, splitting only on the table qualifier dot + */ + private function quoteSetColumn(string $column, PlatformInterface $platform): string + { + return implode( + $platform->getIdentifierSeparator(), + array_map($platform->quoteIdentifier(...), explode('.', $column)), + ); + } + /** * __clone * diff --git a/test/unit/Sql/InsertColumnNameTest.php b/test/unit/Sql/InsertColumnNameTest.php new file mode 100644 index 00000000..dec80599 --- /dev/null +++ b/test/unit/Sql/InsertColumnNameTest.php @@ -0,0 +1,141 @@ +, string, array}> + */ + public static function insertColumnPreparedProvider(): array + { + return [ + 'plain' => [ + ['title' => 1], + 'INSERT INTO "t" ("title") VALUES (:title)', + ['title' => 1], + ], + 'dotted' => [ + ['film.title' => 1], + 'INSERT INTO "t" ("film.title") VALUES (:film_title)', + ['film_title' => 1], + ], + 'hyphen' => [ + ['x-y' => 1], + 'INSERT INTO "t" ("x-y") VALUES (:x_y)', + ['x_y' => 1], + ], + 'space' => [ + ['a b' => 1], + 'INSERT INTO "t" ("a b") VALUES (:a_b)', + ['a_b' => 1], + ], + 'quote character' => [ + ['c"d' => 1], + 'INSERT INTO "t" ("c\'d") VALUES (:c_d)', + ['c_d' => 1], + ], + 'sanitised names collide' => [ + ['a.b' => 1, 'a-b' => 2], + 'INSERT INTO "t" ("a.b", "a-b") VALUES (:a_b, :a_b_1)', + ['a_b' => 1, 'a_b_1' => 2], + ], + 'sanitised name precedes plain name' => [ + ['a.b' => 1, 'a_b' => 2], + 'INSERT INTO "t" ("a.b", "a_b") VALUES (:a_b_1, :a_b)', + ['a_b_1' => 1, 'a_b' => 2], + ], + ]; + } + + /** + * @return array, string}> + */ + public static function insertColumnSqlProvider(): array + { + return [ + 'plain' => [['title' => 1], 'INSERT INTO "t" ("title") VALUES (\'1\')'], + 'dotted' => [['film.title' => 1], 'INSERT INTO "t" ("film.title") VALUES (\'1\')'], + 'hyphen' => [['x-y' => 1], 'INSERT INTO "t" ("x-y") VALUES (\'1\')'], + 'space' => [['a b' => 1], 'INSERT INTO "t" ("a b") VALUES (\'1\')'], + 'quote character' => [['c"d' => 1], 'INSERT INTO "t" ("c\'d") VALUES (\'1\')'], + ]; + } + + #[Test] + public function keepsIncrementalParameterNamesForPdoDriver(): void + { + $driver = $this->createStub(PdoDriverInterface::class); + $driver->method('getPrepareType')->willReturn('named'); + $driver->method('formatParameterName')->willReturnCallback(static fn(string $name): string => ":{$name}"); + $statement = new StatementContainer(); + + (new Insert('t'))->values(['film.title' => 1, 'x-y' => 2, 'plain' => 3]) + ->prepareStatement($this->createMockAdapter($driver), $statement); + + self::assertSame( + [ + 'INSERT INTO "t" ("film.title", "x-y", "plain") VALUES (:c_0, :c_1, :c_2)', + ['c_0' => 1, 'c_1' => 2, 'c_2' => 3], + ], + [$statement->getSql(), $statement->getParameterContainer()?->getNamedArray()], + ); + } + + /** + * @param array $values + * @param array $expectedParameters + */ + #[Test] + #[DataProvider('insertColumnPreparedProvider')] + public function preparesInsertValueWithValidPlaceholder( + array $values, + string $expectedSql, + array $expectedParameters, + ): void { + $driver = $this->createStub(DriverInterface::class); + $driver->method('getPrepareType')->willReturn('named'); + $driver->method('formatParameterName')->willReturnCallback(static fn(string $name): string => ":{$name}"); + $statement = new StatementContainer(); + + (new Insert('t'))->values($values) + ->prepareStatement($this->createMockAdapter($driver), $statement); + + self::assertSame( + [$expectedSql, $expectedParameters], + [$statement->getSql(), $statement->getParameterContainer()?->getNamedArray()], + ); + } + + /** + * @param array $values + */ + #[Test] + #[DataProvider('insertColumnSqlProvider')] + public function rendersInsertColumnAsSingleQuotedIdentifier(array $values, string $expected): void + { + $insert = (new Insert('t'))->values($values); + + self::assertSame($expected, $insert->getSqlString(new TrustingSql92Platform())); + } +} diff --git a/test/unit/Sql/UpdateColumnNameTest.php b/test/unit/Sql/UpdateColumnNameTest.php new file mode 100644 index 00000000..1f8cc8a6 --- /dev/null +++ b/test/unit/Sql/UpdateColumnNameTest.php @@ -0,0 +1,147 @@ +, string, array}> + */ + public static function setColumnPreparedProvider(): array + { + return [ + 'plain' => [ + ['title' => 1], + 'UPDATE "t" SET "title" = :title', + ['title' => 1], + ], + 'dotted' => [ + ['film.title' => 1], + 'UPDATE "t" SET "film"."title" = :film_title', + ['film_title' => 1], + ], + 'hyphen' => [ + ['x-y' => 1], + 'UPDATE "t" SET "x-y" = :x_y', + ['x_y' => 1], + ], + 'space' => [ + ['a b' => 1], + 'UPDATE "t" SET "a b" = :a_b', + ['a_b' => 1], + ], + 'quote character' => [ + ['c"d' => 1], + 'UPDATE "t" SET "c\'d" = :c_d', + ['c_d' => 1], + ], + 'sanitised names collide' => [ + ['a.b' => 1, 'a-b' => 2], + 'UPDATE "t" SET "a"."b" = :a_b, "a-b" = :a_b_1', + ['a_b' => 1, 'a_b_1' => 2], + ], + 'sanitised name precedes plain name' => [ + ['a.b' => 1, 'a_b' => 2], + 'UPDATE "t" SET "a"."b" = :a_b_1, "a_b" = :a_b', + ['a_b_1' => 1, 'a_b' => 2], + ], + 'suffixed name is also plain' => [ + ['a.b' => 1, 'a_b_1' => 2, 'a_b' => 3], + 'UPDATE "t" SET "a"."b" = :a_b_2, "a_b_1" = :a_b_1, "a_b" = :a_b', + ['a_b_2' => 1, 'a_b_1' => 2, 'a_b' => 3], + ], + ]; + } + + /** + * @return array, string}> + */ + public static function setColumnSqlProvider(): array + { + return [ + 'plain' => [['title' => 1], 'UPDATE "t" SET "title" = \'1\''], + 'dotted' => [['film.title' => 1], 'UPDATE "t" SET "film"."title" = \'1\''], + 'hyphen' => [['x-y' => 1], 'UPDATE "t" SET "x-y" = \'1\''], + 'space' => [['a b' => 1], 'UPDATE "t" SET "a b" = \'1\''], + 'quote character' => [['c"d' => 1], 'UPDATE "t" SET "c\'d" = \'1\''], + 'dotted hyphen' => [['f-1.x-y' => 1], 'UPDATE "t" SET "f-1"."x-y" = \'1\''], + ]; + } + + #[Test] + public function keepsIncrementalParameterNamesForPdoDriver(): void + { + $driver = $this->createStub(PdoDriverInterface::class); + $driver->method('getPrepareType')->willReturn('named'); + $driver->method('formatParameterName')->willReturnCallback(static fn(string $name): string => ":{$name}"); + $statement = new StatementContainer(); + + (new Update('t'))->set(['film.title' => 1, 'x-y' => 2, 'plain' => 3]) + ->prepareStatement($this->createMockAdapter($driver), $statement); + + self::assertSame( + [ + 'UPDATE "t" SET "film"."title" = :c_0, "x-y" = :c_1, "plain" = :c_2', + ['c_0' => 1, 'c_1' => 2, 'c_2' => 3], + ], + [$statement->getSql(), $statement->getParameterContainer()?->getNamedArray()], + ); + } + + /** + * @param array $set + * @param array $expectedParameters + */ + #[Test] + #[DataProvider('setColumnPreparedProvider')] + public function preparesSetColumnWithQuotedIdentifierChainAndValidPlaceholder( + array $set, + string $expectedSql, + array $expectedParameters, + ): void { + $driver = $this->createStub(DriverInterface::class); + $driver->method('getPrepareType')->willReturn('named'); + $driver->method('formatParameterName')->willReturnCallback(static fn(string $name): string => ":{$name}"); + $statement = new StatementContainer(); + + (new Update('t'))->set($set)->prepareStatement($this->createMockAdapter($driver), $statement); + + self::assertSame( + [$expectedSql, $expectedParameters], + [$statement->getSql(), $statement->getParameterContainer()?->getNamedArray()], + ); + } + + /** + * @param array $set + */ + #[Test] + #[DataProvider('setColumnSqlProvider')] + public function rendersSetColumnAsQuotedIdentifierChain(array $set, string $expected): void + { + $update = (new Update('t'))->set($set); + + self::assertSame($expected, $update->getSqlString(new TrustingSql92Platform())); + } +}