diff --git a/src/Sql/AbstractSql.php b/src/Sql/AbstractSql.php index 5ea9aaad..05865d4d 100644 --- a/src/Sql/AbstractSql.php +++ b/src/Sql/AbstractSql.php @@ -18,16 +18,23 @@ use PhpDb\Sql\Platform\PlatformDecoratorInterface; use function array_key_first; +use function array_map; use function count; +use function explode; use function get_object_vars; use function implode; use function is_array; use function is_string; +use function preg_split; use function rtrim; use function str_replace; +use function strtolower; use function strtoupper; use function vsprintf; +use const PREG_SPLIT_DELIM_CAPTURE; +use const PREG_SPLIT_NO_EMPTY; + /** * @api * @@ -247,7 +254,7 @@ protected function processExpression( $parameterContainer, ) : $platform->quoteValue((string) $argument->getValue()), - $argument instanceof Identifier => $platform->quoteIdentifierInFragment($argument->getValue()), + $argument instanceof Identifier => $this->quoteIdentifierArgument($argument->getValue(), $platform), $argument instanceof Literal => $argument->getValue(), $argument instanceof Identifiers => $this->processIdentifiersArgument($argument, $platform), $argument instanceof SelectArgument => $this->processExpressionOrSelect( @@ -330,7 +337,7 @@ protected function processIdentifiersArgument( $processedIdentifiers = []; foreach ($identifiers as $identifier) { - $processedIdentifiers[] = $platform->quoteIdentifierInFragment($identifier); + $processedIdentifiers[] = $this->quoteIdentifierArgument($identifier, $platform); } return implode(', ', $processedIdentifiers); @@ -555,6 +562,66 @@ private function matchSpecification(array $specifications, int $parameterCount): throw Exception\RuntimeException::forUnsupportedParameterCount(); } + /** + * Quote an identifier argument, keeping fragment quoting wherever it renders valid SQL + * + * Fragment quoting leaves spaces, dots, `*` and `AS` bare and quotes every other piece the + * platform splits out, so a name holding any other delimiter renders as adjacent quoted + * pieces, and names separated only by spaces render as a bare sequence of identifiers. Both + * are invalid, so such a name is quoted whole as an identifier chain instead, split only on + * the qualifier dot with a `*` part left bare. + */ + private function quoteIdentifierArgument(string $identifier, PlatformInterface $platform): string + { + $isValidFragment = true; + $previousIsName = false; + $afterSpace = false; + + foreach ((array) preg_split( + '/([ .*])/', + $identifier, + -1, + PREG_SPLIT_DELIM_CAPTURE | PREG_SPLIT_NO_EMPTY, + ) as $part) { + if (' ' === $part) { + $afterSpace = true; + continue; + } + + $isName = '.' !== $part && '*' !== $part && 'as' !== strtolower((string) $part); + if ( + $isName + && ( + ( + $previousIsName + && $afterSpace + ) + || $platform->quoteIdentifierInFragment((string) $part) !== $platform->quoteIdentifier( + (string) $part, + ) + ) + ) { + $isValidFragment = false; + break; + } + + $previousIsName = $isName; + $afterSpace = false; + } + + if ($isValidFragment) { + return $platform->quoteIdentifierInFragment($identifier); + } + + return implode( + $platform->getIdentifierSeparator(), + array_map( + static fn(string $part): string => '*' === $part ? $part : $platform->quoteIdentifier($part), + explode('.', $identifier), + ), + ); + } + private function quoteJoinTableIdentifier(TableIdentifier $identifier, PlatformInterface $platform): string { [$table, $schema] = $identifier->getTableAndSchema(); diff --git a/test/unit/Sql/PredicateIdentifierNameTest.php b/test/unit/Sql/PredicateIdentifierNameTest.php new file mode 100644 index 00000000..07a90870 --- /dev/null +++ b/test/unit/Sql/PredicateIdentifierNameTest.php @@ -0,0 +1,302 @@ + + */ + public static function brokenNameProvider(): array + { + return [ + 'hyphen' => ['x-y', '"x-y"'], + 'space' => ['my col', '"my col"'], + 'quote character' => ['a"b', '"a\'b"'], + 'non-ascii' => ['nâme', '"nâme"'], + 'tab' => ["a\tb", "\"a\tb\""], + 'dotted hyphen' => ['t-1.x-y', '"t-1"."x-y"'], + 'dotted space' => ['t.my col', '"t"."my col"'], + 'hyphen qualified star' => ['t-1.*', '"t-1".*'], + ]; + } + + /** + * @return iterable + */ + public static function brokenPredicateProvider(): iterable + { + foreach (self::predicateMethodProvider() as $method => [$apply, $template]) { + foreach (self::brokenNameProvider() as $case => [$name, $quoted]) { + yield "{$method} {$case}" => [$apply, $template, $name, $quoted]; + } + } + } + + /** + * @return array + */ + public static function hyphenWordPlatformProvider(): array + { + return [ + 'hyphen' => ['x-y', '`x-y`'], + 'hyphen arithmetic' => ['a-b * c', '`a-b` * `c`'], + 'dotted hyphen' => ['t-1.x-y', '`t-1`.`x-y`'], + 'space' => ['my col', '`my col`'], + 'quote character' => ['a`b', '`a``b`'], + 'plus' => ['a+b', '`a+b`'], + ]; + } + + /** + * @return array + */ + public static function predicateMethodProvider(): array + { + return [ + 'equalTo' => [static fn(Predicate $p, string $n): Predicate => $p->equalTo($n, 1), "%s = '1'"], + 'notEqualTo' => [static fn(Predicate $p, string $n): Predicate => $p->notEqualTo($n, 1), "%s != '1'"], + 'lessThan' => [static fn(Predicate $p, string $n): Predicate => $p->lessThan($n, 1), "%s < '1'"], + 'greaterThan' => [static fn(Predicate $p, string $n): Predicate => $p->greaterThan($n, 1), "%s > '1'"], + 'like' => [static fn(Predicate $p, string $n): Predicate => $p->like($n, 'a%'), "%s LIKE 'a%%'"], + 'in' => [static fn(Predicate $p, string $n): Predicate => $p->in($n, [1, 2]), "%s IN ('1', '2')"], + 'between' => [ + static fn(Predicate $p, string $n): Predicate => $p->between($n, 1, 2), + "%s BETWEEN '1' AND '2'", + ], + 'isNull' => [static fn(Predicate $p, string $n): Predicate => $p->isNull($n), '%s IS NULL'], + ]; + } + + /** + * Names whose fragment quoting renders valid SQL, and that rendering + * + * @return array + */ + public static function unchangedNameProvider(): array + { + return [ + 'plain' => ['col', '"col"'], + 'dotted' => ['t.col', '"t"."col"'], + 'qualified star' => ['t.*', '"t".*'], + 'arithmetic' => ['price*qty', '"price"*"qty"'], + 'spaced arithmetic' => ['a * b', '"a" * "b"'], + 'dotted arithmetic' => ['t.a * t.b', '"t"."a" * "t"."b"'], + 'inline alias' => ['x as y', '"x" as "y"'], + 'inline alias upper' => ['x AS y', '"x" AS "y"'], + 'comma' => ['a,b', '"a,b"'], + 'empty' => ['', ''], + ]; + } + + /** + * @return iterable + */ + public static function unchangedPredicateProvider(): iterable + { + foreach (self::predicateMethodProvider() as $method => [$apply, $template]) { + foreach (self::unchangedNameProvider() as $case => [$name, $quoted]) { + yield "{$method} {$case}" => [$apply, $template, $name, $quoted]; + } + } + } + + #[Test] + #[DataProvider('hyphenWordPlatformProvider')] + public function followsThePlatformFragmentPatternForWordCharacters(string $name, string $quoted): void + { + $select = new Select('foo'); + $select->where->equalTo($name, 1); + + self::assertSame( + "SELECT `foo`.* FROM `foo` WHERE {$quoted} = '1'", + $select->getSqlString(new TrustingHyphenWordPlatform()), + ); + } + + /** + * @param Closure(Predicate, string): Predicate $apply + */ + #[Test] + #[DataProvider('unchangedPredicateProvider')] + public function keepsValidWhereNameRenderingUnchanged( + Closure $apply, + string $template, + string $name, + string $quoted, + ): void { + $select = new Select('foo'); + $apply($select->where, $name); + + self::assertSame( + sprintf("SELECT \"foo\".* FROM \"foo\" WHERE {$template}", $quoted), + $select->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + public function leavesExpressionWhereUnchanged(): void + { + $select = new Select('foo'); + $select->where(new Expression('x-y = ?', [1])); + + self::assertSame( + "SELECT \"foo\".* FROM \"foo\" WHERE x-y = '1'", + $select->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + public function leavesLiteralWhereUnchanged(): void + { + $select = new Select('foo'); + $select->where(new Literal('x-y = 1')); + + self::assertSame( + 'SELECT "foo".* FROM "foo" WHERE x-y = 1', + $select->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + public function leavesStringWhereUnchanged(): void + { + $select = new Select('foo'); + $select->where('x-y = 1'); + + self::assertSame( + 'SELECT "foo".* FROM "foo" WHERE x-y = 1', + $select->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + #[DataProvider('brokenNameProvider')] + #[DataProvider('unchangedNameProvider')] + public function quotesDeleteWhereName(string $name, string $quoted): void + { + $delete = new Delete('foo'); + $delete->where->equalTo($name, 1); + + self::assertSame( + "DELETE FROM \"foo\" WHERE {$quoted} = '1'", + $delete->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + public function quotesEachNameOfAnIdentifiersArgument(): void + { + $select = new Select('foo'); + $select->where(new Expression('(?) IN (SELECT 1)', [Argument::identifiers(['x-y', 't.col', 'a * b'])])); + + self::assertSame( + 'SELECT "foo".* FROM "foo" WHERE ("x-y", "t"."col", "a" * "b") IN (SELECT 1)', + $select->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + #[DataProvider('brokenNameProvider')] + #[DataProvider('unchangedNameProvider')] + public function quotesHavingName(string $name, string $quoted): void + { + $select = new Select('foo'); + $select->having->greaterThan($name, 1); + + self::assertSame( + "SELECT \"foo\".* FROM \"foo\" HAVING {$quoted} > '1'", + $select->getSqlString(new TrustingSql92Platform()), + ); + } + + /** + * @param Closure(Predicate, string): Predicate $apply + */ + #[Test] + #[DataProvider('brokenPredicateProvider')] + public function quotesPreviouslyBrokenWhereNameAsOneIdentifier( + Closure $apply, + string $template, + string $name, + string $quoted, + ): void { + $select = new Select('foo'); + $apply($select->where, $name); + + self::assertSame( + sprintf("SELECT \"foo\".* FROM \"foo\" WHERE {$template}", $quoted), + $select->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + #[DataProvider('brokenNameProvider')] + #[DataProvider('unchangedNameProvider')] + public function quotesUpdateWhereName(string $name, string $quoted): void + { + $update = new Update('foo'); + $update->set(['bar' => 2]); + $update->where->equalTo($name, 1); + + self::assertSame( + "UPDATE \"foo\" SET \"bar\" = '2' WHERE {$quoted} = '1'", + $update->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + #[DataProvider('brokenNameProvider')] + #[DataProvider('unchangedNameProvider')] + public function quotesWhereArrayKeyAsPredicateName(string $name, string $quoted): void + { + $select = new Select('foo'); + $select->where([$name => 1]); + + self::assertSame( + "SELECT \"foo\".* FROM \"foo\" WHERE {$quoted} = '1'", + $select->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + #[DataProvider('brokenNameProvider')] + #[DataProvider('unchangedNameProvider')] + public function quotesWhereArrayKeyWithNullAsIsNullName(string $name, string $quoted): void + { + $select = new Select('foo'); + $select->where([$name => null]); + + self::assertSame( + "SELECT \"foo\".* FROM \"foo\" WHERE {$quoted} IS NULL", + $select->getSqlString(new TrustingSql92Platform()), + ); + } +} diff --git a/test/unit/TestAsset/TrustingHyphenWordPlatform.php b/test/unit/TestAsset/TrustingHyphenWordPlatform.php new file mode 100644 index 00000000..1e65219e --- /dev/null +++ b/test/unit/TestAsset/TrustingHyphenWordPlatform.php @@ -0,0 +1,27 @@ +quoteTrustedValue($value); + } +}