Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions src/Sql/AbstractSql.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -106,7 +109,7 @@
}
}

return rtrim(implode(' ', $sqls), characters: "\n ,");

Check warning on line 112 in src/Sql/AbstractSql.php

View workflow job for this annotation

GitHub Actions / qa / Mutation testing

Escaped Mutant for Mutator "UnwrapRtrim": @@ @@ } } - return rtrim(implode(' ', $sqls), characters: "\n ,"); + return implode(' ', $sqls); } /**
}

/**
Expand All @@ -114,7 +117,7 @@
* @param SqlParameters $parameters
* @throws Exception\RuntimeException
*/
protected function createSqlFromSpecificationAndParameters(array|string $specifications, array $parameters): string

Check warning on line 120 in src/Sql/AbstractSql.php

View workflow job for this annotation

GitHub Actions / qa / Mutation testing

Escaped Mutant for Mutator "ProtectedVisibility": @@ @@ * @PARAM SqlParameters $parameters * @throws Exception\RuntimeException */ - protected function createSqlFromSpecificationAndParameters(array|string $specifications, array $parameters): string + private function createSqlFromSpecificationAndParameters(array|string $specifications, array $parameters): string { if (is_string($specifications)) { return vsprintf($specifications, $this->flattenParameterList($parameters));
{
if (is_string($specifications)) {
return vsprintf($specifications, $this->flattenParameterList($parameters));
Expand All @@ -136,16 +139,16 @@
* @param list<ArgumentInterface> $arguments
* @return list<ArgumentInterface>
*/
protected function flattenExpressionValues(array $arguments): array

Check warning on line 142 in src/Sql/AbstractSql.php

View workflow job for this annotation

GitHub Actions / qa / Mutation testing

Escaped Mutant for Mutator "ProtectedVisibility": @@ @@ * @PARAM list<ArgumentInterface> $arguments * @return list<ArgumentInterface> */ - protected function flattenExpressionValues(array $arguments): array + private function flattenExpressionValues(array $arguments): array { $hasValues = false; foreach ($arguments as $argument) {
{
$hasValues = false;

Check warning on line 144 in src/Sql/AbstractSql.php

View workflow job for this annotation

GitHub Actions / qa / Mutation testing

Escaped Mutant for Mutator "FalseValue": @@ @@ */ protected function flattenExpressionValues(array $arguments): array { - $hasValues = false; + $hasValues = true; foreach ($arguments as $argument) { if (! $argument instanceof Values) { continue;
foreach ($arguments as $argument) {
if (! $argument instanceof Values) {

Check warning on line 146 in src/Sql/AbstractSql.php

View workflow job for this annotation

GitHub Actions / qa / Mutation testing

Escaped Mutant for Mutator "LogicalNot": @@ @@ { $hasValues = false; foreach ($arguments as $argument) { - if (! $argument instanceof Values) { + if ($argument instanceof Values) { continue; }

Check warning on line 146 in src/Sql/AbstractSql.php

View workflow job for this annotation

GitHub Actions / qa / Mutation testing

Escaped Mutant for Mutator "InstanceOf_": @@ @@ { $hasValues = false; foreach ($arguments as $argument) { - if (! $argument instanceof Values) { + if ($argument instanceof Values) { continue; }
continue;
}

$hasValues = true;
break;

Check warning on line 151 in src/Sql/AbstractSql.php

View workflow job for this annotation

GitHub Actions / qa / Mutation testing

Escaped Mutant for Mutator "Break_": @@ @@ } $hasValues = true; - break; + continue; } if (! $hasValues) {
}

if (! $hasValues) {
Expand Down Expand Up @@ -473,6 +476,48 @@
: $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<string> $columns
* @return array<string, string>
*/
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
Expand Down
17 changes: 10 additions & 7 deletions src/Sql/Insert.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use function implode;
use function is_scalar;
use function range;
use function strval;

/**
* @api
Expand Down Expand Up @@ -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;
}

Expand Down
46 changes: 27 additions & 19 deletions src/Sql/Update.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
*
Expand Down
141 changes: 141 additions & 0 deletions test/unit/Sql/InsertColumnNameTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
<?php

declare(strict_types=1);

namespace PhpDbTest\Sql;

use PhpDb\Adapter\Driver\DriverInterface;
use PhpDb\Adapter\Driver\PdoDriverInterface;
use PhpDb\Adapter\StatementContainer;
use PhpDb\Sql\AbstractSql;
use PhpDb\Sql\Insert;
use PhpDbTest\AdapterTestTrait;
use PhpDbTest\TestAsset\TrustingSql92Platform;
use PHPUnit\Framework\Attributes\CoversMethod;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\Group;
use PHPUnit\Framework\Attributes\Test;
use PHPUnit\Framework\TestCase;

#[Group('unit')]
#[CoversMethod(Insert::class, 'processInsert')]
#[CoversMethod(AbstractSql::class, 'resolveParameterNames')]
final class InsertColumnNameTest extends TestCase
{
use AdapterTestTrait;

/**
* @return array<string, array{array<string, int>, string, array<string, int>}>
*/
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, array{array<string, int>, 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<string, int> $values
* @param array<string, int> $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<string, int> $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()));
}
}
Loading
Loading