diff --git a/src/Sql/Select.php b/src/Sql/Select.php index 183750be..7348393f 100644 --- a/src/Sql/Select.php +++ b/src/Sql/Select.php @@ -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; @@ -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; @@ -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, @@ -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} diff --git a/test/unit/Sql/SelectOrderNameTest.php b/test/unit/Sql/SelectOrderNameTest.php new file mode 100644 index 00000000..d400142b --- /dev/null +++ b/test/unit/Sql/SelectOrderNameTest.php @@ -0,0 +1,142 @@ +|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 + */ + 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, 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|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|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|string $order + */ + private function render(ExpressionInterface|array|string $order): string + { + return (new Select('foo'))->order($order) + ->getSqlString(new TrustingSql92Platform()); + } +} diff --git a/test/unit/TestAsset/HyphenWordSql92Platform.php b/test/unit/TestAsset/HyphenWordSql92Platform.php new file mode 100644 index 00000000..224cbe48 --- /dev/null +++ b/test/unit/TestAsset/HyphenWordSql92Platform.php @@ -0,0 +1,15 @@ +