Skip to content

Commit

Permalink
Optimize string literal escape performance (#1196)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mvorisek
mvorisek authored Apr 15, 2024
1 parent 319cb43 commit c2baf07
Show file tree
Hide file tree
Showing 10 changed files with 132 additions and 104 deletions.
2 changes: 1 addition & 1 deletion phpstan.neon.dist
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ parameters:
-
message: '~^Class Doctrine\\DBAL\\(Platforms\\SqlitePlatform|Schema\\SqliteSchemaManager) referenced with incorrect case: Doctrine\\DBAL\\(Platforms\\SQLitePlatform|Schema\\SQLiteSchemaManager)\.$~'
path: '*'
count: 24
count: 22

# TODO these rules are generated, this ignores should be fixed in the code
# for src/Schema/TestCase.php
Expand Down
14 changes: 7 additions & 7 deletions src/Persistence/Sql/Mssql/ExpressionTrait.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ protected function escapeStringLiteral(string $value): string
}

$parts = [];
foreach (explode("\0", $value) as $i => $v) {
if ($i > 0) {
$parts[] = 'NCHAR(0)';
}

if ($v !== '') {
foreach (preg_split('~(\x00{1,4000})~', $value, -1, \PREG_SPLIT_DELIM_CAPTURE) as $i => $v) {
if (($i % 2) === 1) {
$parts[] = strlen($v) === 1
? 'nchar(0)'
: 'replicate(nchar(0), ' . strlen($v) . ')';
} elseif ($v !== '') {
foreach (mb_str_split($v, 4000) as $v2) {
// TODO report "select N'\'':n?'" issue to https://github.com/microsoft/msphpsql
foreach (preg_split('~(:+)~', $v2, -1, \PREG_SPLIT_DELIM_CAPTURE) as $v3) {
Expand All @@ -54,7 +54,7 @@ protected function escapeStringLiteral(string $value): string
$partsLeft = array_slice($parts, 0, intdiv(count($parts), 2));
$partsRight = array_slice($parts, count($partsLeft));

return 'CONCAT(CAST(' . $buildConcatSqlFx($partsLeft) . ' AS NVARCHAR(MAX)), ' . $buildConcatSqlFx($partsRight) . ')';
return 'concat(cast(' . $buildConcatSqlFx($partsLeft) . ' as NVARCHAR(MAX)), ' . $buildConcatSqlFx($partsRight) . ')';
}

return reset($parts);
Expand Down
12 changes: 5 additions & 7 deletions src/Persistence/Sql/Mysql/ExpressionTrait.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,10 @@ trait ExpressionTrait
protected function escapeStringLiteral(string $value): string
{
$parts = [];
foreach (explode("\0", $value) as $i => $v) {
if ($i > 0) {
$parts[] = 'x\'00\'';
}

if ($v !== '') {
foreach (preg_split('~((?:\x00+[^\x00]{1,100})*\x00+)~', $value, -1, \PREG_SPLIT_DELIM_CAPTURE) as $i => $v) {
if (($i % 2) === 1) {
$parts[] = 'x\'' . bin2hex($v) . '\'';
} elseif ($v !== '') {
$parts[] = '\'' . str_replace(['\'', '\\'], ['\'\'', '\\\\'], $v) . '\'';
}
}
Expand All @@ -29,7 +27,7 @@ protected function escapeStringLiteral(string $value): string
$partsLeft = array_slice($parts, 0, intdiv(count($parts), 2));
$partsRight = array_slice($parts, count($partsLeft));

return 'CONCAT(' . $buildConcatSqlFx($partsLeft) . ', ' . $buildConcatSqlFx($partsRight) . ')';
return 'concat(' . $buildConcatSqlFx($partsLeft) . ', ' . $buildConcatSqlFx($partsRight) . ')';
}

return reset($parts);
Expand Down
22 changes: 11 additions & 11 deletions src/Persistence/Sql/Oracle/ExpressionTrait.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,18 @@ trait ExpressionTrait
protected function escapeStringLiteral(string $value): string
{
$parts = [];
foreach (explode("\0", $value) as $i => $v) {
if ($i > 0) {
$parts[] = 'CHR(0)';
}

if ($v !== '') {
foreach (preg_split('~(\x00+)~', $value, -1, \PREG_SPLIT_DELIM_CAPTURE) as $i => $v) {
if (($i % 2) === 1) {
$parts[] = strlen($v) === 1
? 'chr(0)'
: 'rpad(chr(0), ' . strlen($v) . ', chr(0))';
} elseif ($v !== '') {
// workaround https://github.com/php/php-src/issues/13958
foreach (preg_split('~(\\\+)(?=\'|$)~', $v, -1, \PREG_SPLIT_DELIM_CAPTURE) as $i2 => $v2) {
if (($i2 % 2) === 1) {
for ($j = 0; $j < strlen($v2); ++$j) {
$parts[] = 'chr(' . ord('\\') . ')';
}
$parts[] = strlen($v2) === 1
? 'chr(' . ord('\\') . ')'
: 'rpad(chr(' . ord('\\') . '), ' . strlen($v2) . ', chr(' . ord('\\') . '))';
} elseif ($v2 !== '') {
$parts[] = '\'' . str_replace('\'', '\'\'', $v2) . '\'';
}
Expand All @@ -38,7 +38,7 @@ protected function escapeStringLiteral(string $value): string
$partsLeft = array_slice($parts, 0, intdiv(count($parts), 2));
$partsRight = array_slice($parts, count($partsLeft));

return 'CONCAT(' . $buildConcatSqlFx($partsLeft) . ', ' . $buildConcatSqlFx($partsRight) . ')';
return 'concat(' . $buildConcatSqlFx($partsLeft) . ', ' . $buildConcatSqlFx($partsRight) . ')';
}

return reset($parts);
Expand Down Expand Up @@ -98,7 +98,7 @@ protected function convertLongStringToClobExpr(string $value): Expression
$partsLeft = array_slice($parts, 0, intdiv(count($parts), 2));
$partsRight = array_slice($parts, count($partsLeft));

return 'CONCAT(' . $buildConcatExprFx($partsLeft) . ', ' . $buildConcatExprFx($partsRight) . ')';
return 'concat(' . $buildConcatExprFx($partsLeft) . ', ' . $buildConcatExprFx($partsRight) . ')';
}

$exprArgs[] = reset($parts);
Expand Down
20 changes: 8 additions & 12 deletions src/Persistence/Sql/Postgresql/ExpressionTrait.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,17 @@ protected function escapeStringLiteral(string $value): string
}

$parts = [];
foreach (explode("\0", $value) as $i => $v) {
if ($i > 0) {
foreach (preg_split('~((?:\x00+[^\x00]{1,100})*\x00+)~', $value, -1, \PREG_SPLIT_DELIM_CAPTURE) as $i => $v) {
if (($i % 2) === 1) {
// will raise SQL error, PostgreSQL does not support \0 character
$parts[] = 'convert_from(decode(\'00\', \'hex\'), \'UTF8\')';
}

if ($v !== '') {
$parts[] = 'convert_from(decode(\'' . bin2hex($v) . '\', \'hex\'), \'UTF8\')';
} elseif ($v !== '') {
// workaround https://github.com/php/php-src/issues/13958
foreach (preg_split('~(\\\+)(?=\'|$)~', $v, -1, \PREG_SPLIT_DELIM_CAPTURE) as $i2 => $v2) {
if (($i2 % 2) === 1) {
if (strlen($v2) === 1) {
$parts[] = 'chr(' . ord('\\') . ')';
} else {
$parts[] = 'repeat(chr(' . ord('\\') . '), ' . strlen($v2) . ')';
}
$parts[] = strlen($v2) === 1
? 'chr(' . ord('\\') . ')'
: 'repeat(chr(' . ord('\\') . '), ' . strlen($v2) . ')';
} elseif ($v2 !== '') {
$parts[] = '\'' . str_replace('\'', '\'\'', $v2) . '\'';
}
Expand All @@ -51,7 +47,7 @@ protected function escapeStringLiteral(string $value): string
$partsLeft = array_slice($parts, 0, intdiv(count($parts), 2));
$partsRight = array_slice($parts, count($partsLeft));

return 'CONCAT(' . $buildConcatSqlFx($partsLeft) . ', ' . $buildConcatSqlFx($partsRight) . ')';
return 'concat(' . $buildConcatSqlFx($partsLeft) . ', ' . $buildConcatSqlFx($partsRight) . ')';
}

return reset($parts);
Expand Down
68 changes: 38 additions & 30 deletions src/Persistence/Sql/Query.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
*/
abstract class Query extends Expression
{
private const FIELD_INT_STRING_PREFIX = "\xff_int-string_";

/** Template name for render. */
public string $mode = 'select';

Expand Down Expand Up @@ -52,12 +54,11 @@ abstract class Query extends Expression
* a dot or a space), you should wrap it into expression:
* $q->field($q->expr('{}', ['fun...ky.field']), 'f');
*
* @param string|Expressionable $field Specifies field to select
* @param string $alias Specify alias for this field
* @param string|Expressionable $field
*
* @return $this
*/
public function field($field, $alias = null)
public function field($field, ?string $alias = null)
{
$this->_setArgs('field', $alias, $field);

Expand All @@ -79,17 +80,17 @@ protected function _renderField($addAlias = true): string
return $this->consume($this->defaultField, self::ESCAPE_PARAM);
}

return $this->defaultField;
return $this->escapeIdentifierSoft($this->defaultField);
}

$res = [];
foreach ($this->args['field'] as $alias => $field) {
// do not add alias when:
// - we don't want aliases
// - OR alias is the same as field
// - OR alias is numeric
if (is_string($alias) && str_starts_with($alias, self::FIELD_INT_STRING_PREFIX)) {
$alias = substr($alias, strlen(self::FIELD_INT_STRING_PREFIX));
}

if ($addAlias === false
|| (is_string($field) && $alias === $field)
|| $alias === $field
|| is_int($alias)
) {
$alias = null;
Expand All @@ -98,7 +99,7 @@ protected function _renderField($addAlias = true): string
// will parameterize the value and escape if necessary
$field = $this->consume($field, self::ESCAPE_IDENTIFIER_SOFT);

if ($alias) {
if ($alias !== null) {
// field alias cannot be expression, so simply escape it
$field .= ' ' . $this->escapeIdentifier($alias);
}
Expand All @@ -121,19 +122,21 @@ protected function _renderFieldNoalias(): string
/**
* Specify a table to be used in a query.
*
* @param string|Expressionable $table Specifies table
* @param string $alias Specify alias for this table
* @param string|Expressionable $table
*
* @return $this
*/
public function table($table, $alias = null)
public function table($table, ?string $alias = null)
{
if ($table instanceof self && $alias === null) {
throw new Exception('If table is set as subquery, then table alias is required');
}
if ($alias === null) {
if ($table instanceof self) {
throw (new Exception('Table alias is required when table is set as subquery'))
->addMoreInfo('table', $table);
}

if (is_string($table) && $alias === null) {
$alias = $table;
if (is_string($table)) {
$alias = $table;
}
}

$this->_setArgs('table', $alias, $table);
Expand Down Expand Up @@ -219,10 +222,7 @@ protected function _renderFrom(): ?string
/**
* Specify WITH query to be used.
*
* @param Query $cursor Specifies cursor query or array [alias => query] for adding multiple
* @param string $alias Specify alias for this cursor
* @param array<int, string>|null $fields Optional array of field names used in cursor
* @param bool $recursive Is it recursive?
* @param array<int, string>|null $fields
*
* @return $this
*/
Expand Down Expand Up @@ -1183,22 +1183,30 @@ protected function _renderCase(): ?string
/**
* Sets value in args array. Doesn't allow duplicate aliases.
*
* @param string $what Where to set it - table|field
* @param string|null $alias Alias name
* @param mixed $value Value to set in args array
* @param mixed $value
*/
protected function _setArgs($what, $alias, $value): void
protected function _setArgs(string $kind, ?string $alias, $value): void
{
if ($alias === null) {
$this->args[$what][] = $value;
$this->args[$kind][] = $value;
} else {
if (isset($this->args[$what][$alias])) {
if (isset($this->args[$kind][$alias])) {
throw (new Exception('Alias must be unique'))
->addMoreInfo('what', $what)
->addMoreInfo('kind', $kind)
->addMoreInfo('alias', $alias);
}

$this->args[$what][$alias] = $value;
if ($alias === (string) (int) $alias) {
if ($kind === 'field') {
$alias = self::FIELD_INT_STRING_PREFIX . $alias;
} else {
throw (new Exception('Alias must be not int-string'))
->addMoreInfo('kind', $kind)
->addMoreInfo('alias', $alias);
}
}

$this->args[$kind][$alias] = $value;
}
}

Expand Down
10 changes: 4 additions & 6 deletions src/Persistence/Sql/Sqlite/ExpressionTrait.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,10 @@ trait ExpressionTrait
protected function escapeStringLiteral(string $value): string
{
$parts = [];
foreach (explode("\0", $value) as $i => $v) {
if ($i > 0) {
$parts[] = 'x\'00\'';
}

if ($v !== '' || $i === 0) {
foreach (preg_split('~((?:\x00+[^\x00]{1,100})*\x00+)~', $value, -1, \PREG_SPLIT_DELIM_CAPTURE) as $i => $v) {
if (($i % 2) === 1) {
$parts[] = 'x\'' . bin2hex($v) . '\'';
} elseif ($v !== '' || $i === 0) {
$parts[] = '\'' . str_replace('\'', '\'\'', $v) . '\'';
}
}
Expand Down
31 changes: 25 additions & 6 deletions tests/Persistence/Sql/ExpressionTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,18 +246,37 @@ public function testExpr(): void

public function testEscapeStringLiteral(): void
{
$escapeStringLiteralFx = \Closure::bind(static function ($value) {
$e = new SqliteExpression();
$escapeStringLiteralFx = \Closure::bind(static function ($value, $expressionClass = SqliteExpression::class) {
$e = new $expressionClass();

return $e->escapeStringLiteral($value);
}, null, SqliteExpression::class);
}, null, Expression::class);

self::assertSame('\'\'', $escapeStringLiteralFx(''));
self::assertSame('\'foo\'', $escapeStringLiteralFx('foo'));
self::assertSame('(\'\' || x\'00\')', $escapeStringLiteralFx("\0"));
self::assertSame('(\'\' || (x\'00\' || \'a\'))', $escapeStringLiteralFx("\0a"));
self::assertSame('(\'a\' || x\'00\')', $escapeStringLiteralFx("a\0"));
self::assertSame('((\'a\' || x\'00\') || (\'b\' || (x\'00\' || \'c\')))', $escapeStringLiteralFx("a\0b\0c"));
self::assertSame('(\'a\' || x\'0000\')', $escapeStringLiteralFx("a\0\0"));
self::assertSame('(\'a\' || x\'' . str_repeat('00', 10_000) . '\')', $escapeStringLiteralFx('a' . str_repeat("\0", 10_000)));
self::assertSame('(\'a\' || (x\'006200\' || \'c\'))', $escapeStringLiteralFx("a\0b\0c"));
self::assertSame(
'(\'a\' || (x\'00' . str_repeat('62', 100) . '00\' || \'c\'))',
$escapeStringLiteralFx("a\0" . str_repeat('b', 100) . "\0c")
);
self::assertSame(
'((\'a\' || x\'00\') || (\'' . str_repeat('b', 101) . '\' || (x\'00\' || \'c\')))',
$escapeStringLiteralFx("a\0" . str_repeat('b', 101) . "\0c")
);

self::assertSame('\'foo\'', $escapeStringLiteralFx('foo', MysqlExpression::class));
self::assertSame('x\'00\'', $escapeStringLiteralFx("\0", MysqlExpression::class));
self::assertSame(
'concat(\'a\', concat(x\'00' . str_repeat('62', 100) . '00\', \'c\'))',
$escapeStringLiteralFx("a\0" . str_repeat('b', 100) . "\0c", MysqlExpression::class)
);
self::assertSame(
'concat(concat(\'a\', x\'00\'), concat(\'' . str_repeat('b', 101) . '\', concat(x\'00\', \'c\')))',
$escapeStringLiteralFx("a\0" . str_repeat('b', 101) . "\0c", MysqlExpression::class)
);
}

public function testEscapeIdentifier(): void
Expand Down
Loading

0 comments on commit c2baf07

Please sign in to comment.