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
34 changes: 33 additions & 1 deletion src/Sql/Select.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@

use function array_key_exists;
use function array_key_first;
use function array_map;
use function count;
use function explode;
use function gettype;
use function implode;
use function is_array;
use function is_int;
use function is_numeric;
Expand All @@ -23,6 +25,7 @@
use function key;
use function method_exists;
use function preg_split;
use function str_replace;
use function strcasecmp;
use function stripos;
use function strtolower;
Expand Down Expand Up @@ -604,7 +607,7 @@ protected function processOrder(
}

$orders[] = [
$platform->quoteIdentifierInFragment($k),
$this->quoteOrderIdentifier($k, $platform),
strcasecmp(trim($v), self::ORDER_DESCENDING) === 0
? self::ORDER_DESCENDING
: self::ORDER_ASCENDING,
Expand Down Expand Up @@ -770,6 +773,35 @@ private function prefixFromTable(?string $fromTable, PlatformInterface $platform
: '';
}

/**
* Quote an order name, keeping fragment quoting only where it renders each part as a single identifier
*
* Fragment quoting splits on every non-word character, so a name such as `x-y` or `my col` would render
* as several quoted pieces. Such a name is quoted whole as an identifier chain instead, split only on
* the table qualifier dot. Names whose parts between `*` and `.` already quote whole, such as `t.col`
* or `price*qty`, render exactly as before.
*/
private function quoteOrderIdentifier(string $name, PlatformInterface $platform): string
{
foreach (explode('.', str_replace(self::SQL_STAR, replace: '.', subject: $name)) as $segment) {
$segment = trim($segment, characters: ' ');
$quoted = $platform->quoteIdentifierInFragment($segment);
if ($quoted !== $segment && $quoted !== $platform->quoteIdentifier($segment)) {
return implode(
$platform->getIdentifierSeparator(),
array_map(
static fn(string $part): string => self::SQL_STAR === $part
? $part
: $platform->quoteIdentifier($part),
explode('.', $name),
),
);
}
}

return $platform->quoteIdentifierInFragment($name);
}

/**
* @param Select|TableReference|null $table
* @return array{0: string|null, 1: string}
Expand Down
142 changes: 142 additions & 0 deletions test/unit/Sql/SelectOrderNameTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
<?php

declare(strict_types=1);

namespace PhpDbTest\Sql;

use PhpDb\Adapter\Platform\PlatformInterface;
use PhpDb\Sql\Expression;
use PhpDb\Sql\ExpressionInterface;
use PhpDb\Sql\Select;
use PhpDbTest\TestAsset\HyphenWordSql92Platform;
use PhpDbTest\TestAsset\TestSql92Platform;
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(Select::class, 'order')]
#[CoversMethod(Select::class, 'processOrder')]
#[CoversMethod(Select::class, 'quoteOrderIdentifier')]
final class SelectOrderNameTest extends TestCase
{
/**
* @return array<string, array{ExpressionInterface|array<int|string, string|ExpressionInterface>|string, string}>
*/
public static function brokenOrderProvider(): array
{
return [
'hyphen' => ['x-y', '"x-y" ASC'],
'hyphen descending' => ['x-y DESC', '"x-y" DESC'],
'dotted hyphen' => ['t-1.x-y DESC', '"t-1"."x-y" DESC'],
'quote character' => ['a"b DESC', '"a\'b" DESC'],
'non-ascii' => ['café DESC', '"café" DESC'],
'hyphen in comma list' => ['x-y DESC, b', '"x-y" DESC, "b" ASC'],
'arithmetic with hyphen' => ['price*x-y DESC', '"price*x-y" DESC'],
'hyphen array key' => [['x-y' => 'DESC'], '"x-y" DESC'],
'space array key' => [['my col' => 'DESC'], '"my col" DESC'],
'dotted space array key' => [['t.my col' => 'ASC'], '"t"."my col" ASC'],
'quote character array key' => [['a"b' => 'DESC'], '"a\'b" DESC'],
'non-ascii array key' => [['café' => 'DESC'], '"café" DESC'],
'hyphen array list' => [['x-y DESC', 'b'], '"x-y" DESC, "b" ASC'],
'hyphen qualified star' => ['x-y.* DESC', '"x-y".* DESC'],
];
}

/**
* @return array<string, array{PlatformInterface, string, string}>
*/
public static function platformOrderProvider(): array
{
return [
'hyphen word character' => [new HyphenWordSql92Platform(), 'x-y DESC', '"x-y" DESC'],
'hyphen word character arithmetic' => [
new HyphenWordSql92Platform(),
'price*x-y DESC',
'"price"*"x-y" DESC',
],
'hyphen word character with plus' => [new HyphenWordSql92Platform(), 'x+y DESC', '"x+y" DESC'],
'identifier quoting off' => [
new TestSql92Platform(quoteIdentifiers: false),
'x-y DESC',
'x-y DESC',
],
'identifier quoting off arithmetic' => [
new TestSql92Platform(quoteIdentifiers: false),
'price*qty DESC',
'price*qty DESC',
],
];
}

/**
* @return array<string, array{ExpressionInterface|array<int|string, string|ExpressionInterface>|string, string}>
*/
public static function validOrderProvider(): array
{
return [
'plain' => ['a', '"a" ASC'],
'plain descending' => ['a DESC', '"a" DESC'],
'lower case descending' => ['a desc', '"a" DESC'],
'dotted' => ['t.col DESC', '"t"."col" DESC'],
'comma list' => ['a DESC, b', '"a" DESC, "b" ASC'],
'arithmetic' => ['price*qty DESC', '"price"*"qty" DESC'],
'string with space' => ['my col DESC', '"my" ASC'],
'array key' => [['a' => 'DESC'], '"a" DESC'],
'array key unknown direction' => [['a' => 'sideways'], '"a" ASC'],
'spaced arithmetic key' => [['price * qty' => 'DESC'], '"price" * "qty" DESC'],
'dotted arithmetic key' => [['t.price*t.qty' => 'DESC'], '"t"."price"*"t"."qty" DESC'],
'trailing space key' => [['a ' => 'DESC'], '"a" DESC'],
'safe word part' => [['t.as' => 'DESC'], '"t".as DESC'],
'array list' => [['a DESC', 'b'], '"a" DESC, "b" ASC'],
'expression' => [new Expression('RAND()'), 'RAND()'],
'expression in array' => [['a', new Expression('RAND()')], '"a" ASC, RAND()'],
];
}

#[Test]
#[DataProvider('platformOrderProvider')]
public function followsThePlatformFragmentRulesForOrderNames(
PlatformInterface $platform,
string $order,
string $expected,
): void {
$select = (new Select('foo'))->order($order);

self::assertStringEndsWith(" ORDER BY {$expected}", $select->getSqlString($platform));
}

/**
* @param ExpressionInterface|array<int|string, string|ExpressionInterface>|string $order
*/
#[Test]
#[DataProvider('brokenOrderProvider')]
public function quotesOrderNameAsIdentifierChainWhenFragmentQuotingWouldBreakIt(
ExpressionInterface|array|string $order,
string $expected,
): void {
self::assertSame("SELECT \"foo\".* FROM \"foo\" ORDER BY {$expected}", $this->render($order));
}

/**
* @param ExpressionInterface|array<int|string, string|ExpressionInterface>|string $order
*/
#[Test]
#[DataProvider('validOrderProvider')]
public function rendersValidOrderUnchanged(ExpressionInterface|array|string $order, string $expected): void
{
self::assertSame("SELECT \"foo\".* FROM \"foo\" ORDER BY {$expected}", $this->render($order));
}

/**
* @param ExpressionInterface|array<int|string, string|ExpressionInterface>|string $order
*/
private function render(ExpressionInterface|array|string $order): string
{
return (new Select('foo'))->order($order)
->getSqlString(new TrustingSql92Platform());
}
}
15 changes: 15 additions & 0 deletions test/unit/TestAsset/HyphenWordSql92Platform.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

declare(strict_types=1);

namespace PhpDbTest\TestAsset;

use PhpDb\Adapter\Platform\Sql92;

/**
* SQL-92 platform whose identifier fragment pattern treats `-` as a word character, as phpdb-mysql does
*/
final class HyphenWordSql92Platform extends Sql92
{
protected string $quoteIdentifierFragmentPattern = '/([^0-9,a-z,A-Z$_\-:])/i';
}
Loading