Fix whole query regexes performance (#1018)

atk4 · Jul 5, 2022 · 836b28c · 836b28c
1 parent 0a0b1e8
commit 836b28c
Show file tree

Hide file tree

Showing 16 changed files with 112 additions and 96 deletions.
diff --git a/src/Persistence.php b/src/Persistence.php
@@ -26,9 +26,9 @@ abstract class Persistence
     public const HOOK_AFTER_ADD = self::class . '@afterAdd';
 
     /** @const string */
-    public const ID_LOAD_ONE = self::class . '@idLoadOne';
+    public const ID_LOAD_ONE = self::class . '@idLoadOne-qZ5TJwMVJ4LzVhuN';
     /** @const string */
-    public const ID_LOAD_ANY = self::class . '@idLoadAny';
+    public const ID_LOAD_ANY = self::class . '@idLoadAny-qZ5TJwMVJ4LzVhuN';
 
     /** @var bool internal only, prevent recursion */
     private $typecastSaveSkipNormalize = false;

diff --git a/src/Persistence/Csv.php b/src/Persistence/Csv.php
@@ -38,21 +38,18 @@ class Csv extends Persistence
 
     /**
      * Mode of operation. 'r' for reading and 'w' for writing.
-     * If you manually set this operation, it will be used
-     * for file opening.
+     * If you manually set this operation, it will be used for file opening.
      *
      * @var string
      */
     public $mode;
 
     /** @var string Delimiter in CSV file. */
     public $delimiter = ',';
-
     /** @var string Enclosure in CSV file. */
     public $enclosure = '"';
-
     /** @var string Escape character in CSV file. */
-    public $escape_char = '\\';
+    public $escapeChar = '\\';
 
     /** @var array|null Array of field names. */
     public $header;
@@ -105,7 +102,7 @@ public function closeFile(): void
      */
     public function getLine(): ?array
     {
-        $data = fgetcsv($this->handle, 0, $this->delimiter, $this->enclosure, $this->escape_char);
+        $data = fgetcsv($this->handle, 0, $this->delimiter, $this->enclosure, $this->escapeChar);
         if ($data === false) {
             return null;
         }
@@ -120,7 +117,7 @@ public function getLine(): ?array
      */
     public function putLine(array $data): void
     {
-        $ok = fputcsv($this->handle, $data, $this->delimiter, $this->enclosure, $this->escape_char);
+        $ok = fputcsv($this->handle, $data, $this->delimiter, $this->enclosure, $this->escapeChar);
         if ($ok === false) {
             throw new Exception('Cannot write to CSV file');
         }

diff --git a/src/Persistence/Sql.php b/src/Persistence/Sql.php
@@ -177,7 +177,7 @@ public function expr(Model $model, $expr, array $args = []): Expression
             return $this->getConnection()->expr($expr, $args);
         }
         preg_replace_callback(
-            '/\[[a-z0-9_]*\]|{[a-z0-9_]*}/i',
+            '~\[\w*\]|\{\w*\}~',
             function ($matches) use (&$args, $model) {
                 $identifier = substr($matches[0], 1, -1);
                 if ($identifier && !isset($args[$identifier])) {
@@ -644,6 +644,28 @@ protected function deleteRaw(Model $model, $idRaw): void
         $model->hook(self::HOOK_AFTER_DELETE_QUERY, [$delete, $c]);
     }
 
+    public function typecastSaveField(Field $field, $value)
+    {
+        $value = parent::typecastSaveField($field, $value);
+
+        if ($value !== null && $this->binaryTypeIsEncodeNeeded($field->getTypeObject())) {
+            $value = $this->binaryTypeValueEncode($value);
+        }
+
+        return $value;
+    }
+
+    public function typecastLoadField(Field $field, $value)
+    {
+        $value = parent::typecastLoadField($field, $value);
+
+        if ($value !== null && $this->binaryTypeIsDecodeNeeded($field->getTypeObject(), $value)) {
+            $value = $this->binaryTypeValueDecode($value);
+        }
+
+        return $value;
+    }
+
     public function getFieldSqlExpression(Field $field, Expression $expression): Expression
     {
         if (isset($field->getOwner()->persistence_data['use_table_prefixes'])) {

diff --git a/src/Persistence/Sql/BinaryTypeCompatibilityTypecastTrait.php b/src/Persistence/Sql/BinaryTypeCompatibilityTypecastTrait.php
@@ -5,7 +5,6 @@
 namespace Atk4\Data\Persistence\Sql;
 
 use Atk4\Data\Exception;
-use Atk4\Data\Field;
 use Doctrine\DBAL\Platforms\OraclePlatform;
 use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
 use Doctrine\DBAL\Platforms\SQLServerPlatform;
@@ -67,29 +66,19 @@ private function binaryTypeIsEncodeNeeded(Type $type): bool
         return false;
     }
 
-    public function typecastSaveField(Field $field, $value)
+    /**
+     * @param scalar $value
+     */
+    private function binaryTypeIsDecodeNeeded(Type $type, $value): bool
     {
-        $value = parent::typecastSaveField($field, $value);
-
-        if ($value !== null && $this->binaryTypeIsEncodeNeeded($field->getTypeObject())) {
-            $value = $this->binaryTypeValueEncode($value);
-        }
-
-        return $value;
-    }
-
-    public function typecastLoadField(Field $field, $value)
-    {
-        $value = parent::typecastLoadField($field, $value);
-
-        if ($value !== null && $this->binaryTypeIsEncodeNeeded($field->getTypeObject())) {
+        if ($this->binaryTypeIsEncodeNeeded($type)) {
             // always decode for Oracle platform to assert the value is always encoded,
             // on other platforms, binary values are stored natively
             if ($this->getDatabasePlatform() instanceof OraclePlatform || $this->binaryTypeValueIsEncoded($value)) {
-                $value = $this->binaryTypeValueDecode($value);
+                return true;
             }
         }
 
-        return $value;
+        return false;
     }
 }
diff --git a/src/Persistence/Sql/Expression.php b/src/Persistence/Sql/Expression.php
@@ -54,7 +54,7 @@ class Expression implements Expressionable, \ArrayAccess
      *
      * @var string
      */
-    protected $escape_char = '"';
+    protected $identifierEscapeChar = '"';
 
     /** @var string|null */
     private $renderParamBase;
@@ -189,7 +189,7 @@ public function expr($properties = [], $arguments = null)
             $e = new static($properties, $arguments);
         }
 
-        $e->escape_char = $this->escape_char;
+        $e->identifierEscapeChar = $this->identifierEscapeChar;
 
         return $e;
     }
@@ -260,9 +260,14 @@ protected function consume($expr, string $escapeMode = self::ESCAPE_PARAM)
             [$sql, $params] = $expr->render();
             foreach ($params as $k => $v) {
                 $this->renderParams[$k] = $v;
-                do {
+            }
+
+            if (count($params) > 0) {
+                $kWithoutColon = substr(array_key_last($params), 1);
+                while ($this->renderParamBase !== $kWithoutColon) {
                     ++$this->renderParamBase;
-                } while ($this->renderParamBase === $k);
+                }
+                ++$this->renderParamBase;
             }
         } finally {
             $expr->paramBase = $expressionParamBaseBackup;
@@ -317,8 +322,7 @@ protected function escapeParam($value): string
      */
     protected function escapeIdentifier(string $value): string
     {
-        // in all other cases we should escape
-        $c = $this->escape_char;
+        $c = $this->identifierEscapeChar;
 
         return $c . str_replace($c, $c . $c, $value) . $c;
     }
@@ -333,32 +337,26 @@ protected function escapeIdentifier(string $value): string
      */
     protected function escapeIdentifierSoft(string $value): string
     {
-        // in some cases we should not escape
-        if ($this->isUnescapablePattern($value)) {
+        if ($this->isUnescapableIdentifier($value)) {
             return $value;
         }
 
         if (str_contains($value, '.')) {
-            return implode('.', array_map(__METHOD__, explode('.', $value)));
+            return implode('.', array_map(fn ($v) => $this->escapeIdentifierSoft($v), explode('.', $value)));
         }
 
-        return $this->escape_char . trim($value) . $this->escape_char;
+        return $this->escapeIdentifier($value);
     }
 
     /**
      * Given the string parameter, it will detect some "deal-breaker" for our
      * soft escaping, such as "*" or "(".
-     * Those will typically indicate that expression is passed and shouldn't
-     * be escaped.
-     *
-     * @param self|string $value
      */
-    protected function isUnescapablePattern($value): bool
+    protected function isUnescapableIdentifier(string $value): bool
     {
-        return is_object($value)
-            || $value === '*'
+        return $value === '*'
             || str_contains($value, '(')
-            || str_contains($value, $this->escape_char);
+            || str_contains($value, $this->identifierEscapeChar);
     }
 
     private function _render(): array
@@ -370,9 +368,9 @@ private function _render(): array
         $res = preg_replace_callback(
             <<<'EOF'
                 ~
-                 '(?:[^'\\]+|\\.|'')*'\K
-                |"(?:[^"\\]+|\\.|"")*"\K
-                |`(?:[^`\\]+|\\.|``)*`\K
+                 '(?:[^'\\]+|\\.|'')*+'\K
+                |"(?:[^"\\]+|\\.|"")*+"\K
+                |`(?:[^`\\]+|\\.|``)*+`\K
                 |\[\w*\]
                 |\{\w*\}
                 |\{\{\w*\}\}
@@ -516,8 +514,12 @@ protected function updateRenderBeforeExecute(array $render): array
             $i = 0;
             $j = 0;
             $sql = preg_replace_callback(
-                '~(?:\'(?:\'\'|\\\\\'|[^\'])*\')?+\K(?:\?|:\w+)~s',
+                '~\'(?:\'\'|\\\\\'|[^\'])*+\'\K|(?:\?|:\w+)~s',
                 function ($matches) use ($params, &$numParams, &$i, &$j) {
+                    if ($matches[0] === '') {
+                        return '';
+                    }
+
                     $numParams[++$i] = $params[$matches[0] === '?' ? ++$j : $matches[0]];
 
                     return '?';

diff --git a/src/Persistence/Sql/Mssql/Expression.php b/src/Persistence/Sql/Mssql/Expression.php
@@ -10,5 +10,5 @@ class Expression extends BaseExpression
 {
     use ExpressionTrait;
 
-    protected $escape_char = ']';
+    protected $identifierEscapeChar = ']';
 }
diff --git a/src/Persistence/Sql/Mssql/ExpressionTrait.php b/src/Persistence/Sql/Mssql/ExpressionTrait.php
@@ -8,28 +8,18 @@
 
 trait ExpressionTrait
 {
-    private function fixOpenEscapeChar(string $v): string
-    {
-        return preg_replace('~(?:\'(?:\'\'|\\\\\'|[^\'])*\')?+\K\]([^\[\]\'"(){}]*?)\]~s', '[$1]', $v);
-    }
-
     protected function escapeIdentifier(string $value): string
     {
-        return $this->fixOpenEscapeChar(parent::escapeIdentifier($value));
-    }
-
-    protected function escapeIdentifierSoft(string $value): string
-    {
-        return $this->fixOpenEscapeChar(parent::escapeIdentifierSoft($value));
+        return preg_replace('~\]([^\[\]\'"(){}]*?\])~s', '[$1', parent::escapeIdentifier($value));
     }
 
     public function render(): array
     {
         [$sql, $params] = parent::render();
 
         // convert all SQL strings to NVARCHAR, eg 'text' to N'text'
-        $sql = preg_replace_callback('~(^|.)(\'(?:\'\'|\\\\\'|[^\'])*\')~s', function ($matches) {
-            return $matches[1] . (!in_array($matches[1], ['N', '\'', '\\'], true) ? 'N' : '') . $matches[2];
+        $sql = preg_replace_callback('~N?(\'(?:\'\'|\\\\\'|[^\'])*+\')~s', function ($matches) {
+            return 'N' . $matches[1];
         }, $sql);
 
         return [$sql, $params];

diff --git a/src/Persistence/Sql/Mssql/Query.php b/src/Persistence/Sql/Mssql/Query.php
@@ -10,7 +10,7 @@ class Query extends BaseQuery
 {
     use ExpressionTrait;
 
-    protected $escape_char = ']';
+    protected $identifierEscapeChar = ']';
 
     protected $expression_class = Expression::class;
 

diff --git a/src/Persistence/Sql/Mysql/Expression.php b/src/Persistence/Sql/Mysql/Expression.php
@@ -10,5 +10,5 @@ class Expression extends BaseExpression
 {
     use ExpressionTrait;
 
-    protected $escape_char = '`';
+    protected $identifierEscapeChar = '`';
 }
diff --git a/src/Persistence/Sql/Mysql/Query.php b/src/Persistence/Sql/Mysql/Query.php
@@ -10,10 +10,12 @@ class Query extends BaseQuery
 {
     use ExpressionTrait;
 
-    protected $escape_char = '`';
+    protected $identifierEscapeChar = '`';
 
     protected $expression_class = Expression::class;
 
+    protected $supportedOperators = ['=', '!=', '<', '>', '<=', '>=', 'like', 'not like', 'in', 'not in', 'regexp', 'not regexp'];
+
     protected $template_update = 'update [table][join] set [set] [where]';
 
     public function groupConcat($field, string $delimiter = ',')

diff --git a/src/Persistence/Sql/Oracle/ExpressionTrait.php b/src/Persistence/Sql/Oracle/ExpressionTrait.php
@@ -11,20 +11,20 @@ protected function convertLongStringToClobExpr(string $value): Expression
         $exprArgs = [];
         $buildConcatExprFx = function (array $parts) use (&$buildConcatExprFx, &$exprArgs): string {
             if (count($parts) > 1) {
-                $valueLeft = array_slice($parts, 0, intdiv(count($parts), 2));
-                $valueRight = array_slice($parts, count($valueLeft));
+                $partsLeft = array_slice($parts, 0, intdiv(count($parts), 2));
+                $partsRight = array_slice($parts, count($partsLeft));
 
-                return 'CONCAT(' . $buildConcatExprFx($valueLeft) . ', ' . $buildConcatExprFx($valueRight) . ')';
+                return 'CONCAT(' . $buildConcatExprFx($partsLeft) . ', ' . $buildConcatExprFx($partsRight) . ')';
             }
 
-            $exprArgs[] = count($parts) > 0 ? reset($parts) : '';
+            $exprArgs[] = reset($parts);
 
             return 'TO_CLOB([])';
         };
 
-        // Oracle SQL (multibyte) string literal is limited to 1332 bytes
+        // Oracle (multibyte) string literal is limited to 1332 bytes
         $parts = [];
-        foreach (mb_str_split($value, 10_000) as $shorterValue) {
+        foreach (mb_str_split($value, 10_000) ?: [''] as $shorterValue) { // @phpstan-ignore-line https://github.com/phpstan/phpstan/issues/7580
             $lengthBytes = strlen($shorterValue);
             $startBytes = 0;
             do {
@@ -46,16 +46,28 @@ protected function updateRenderBeforeExecute(array $render): array
         $newParamBase = $this->paramBase;
         $newParams = [];
         $sql = preg_replace_callback(
-            '~(?:\'(?:\'\'|\\\\\'|[^\'])*\')?+\K:\w+~s',
+            '~\'(?:\'\'|\\\\\'|[^\'])*+\'|:\w+~s',
             function ($matches) use ($params, &$newParams, &$newParamBase) {
-                $value = $params[$matches[0]];
+                if (str_starts_with($matches[0], '\'')) {
+                    $value = str_replace('\'\'', '\'', substr($matches[0], 1, -1));
+                    if (strlen($value) <= 4000) {
+                        return $matches[0];
+                    }
+                } else {
+                    $value = $params[$matches[0]];
+                }
+
                 if (is_string($value) && strlen($value) > 4000) {
                     $expr = $this->convertLongStringToClobExpr($value);
                     unset($value);
                     [$exprSql, $exprParams] = $expr->render();
                     $sql = preg_replace_callback(
-                        '~(?:\'(?:\'\'|\\\\\'|[^\'])*\')?+\K:\w+~s',
+                        '~\'(?:\'\'|\\\\\'|[^\'])*+\'\K|:\w+~s',
                         function ($matches) use ($exprParams, &$newParams, &$newParamBase) {
+                            if ($matches[0] === '') {
+                                return '';
+                            }
+
                             $name = ':' . $newParamBase;
                             ++$newParamBase;
                             $newParams[$name] = $exprParams[$matches[0]];

diff --git a/src/Persistence/Sql/Query.php b/src/Persistence/Sql/Query.php
@@ -28,7 +28,7 @@ class Query extends Expression
     public $wrapInParentheses = true;
 
     /** @var array<string> */
-    protected $supportedOperators = ['=', '!=', '<', '>', '<=', '>=', 'like', 'not like', 'in', 'not in', 'regexp', 'not regexp'];
+    protected $supportedOperators = ['=', '!=', '<', '>', '<=', '>=', 'like', 'not like', 'in', 'not in'];
 
     /** @var string */
     protected $template_select = '[with]select[option] [field] [from] [table][join][where][group][having][order][limit]';

diff --git a/tests/ExpressionSqlTest.php b/tests/ExpressionSqlTest.php
@@ -133,9 +133,7 @@ public function testExpressions(): void
         $m = new Model($this->db, ['table' => 'user']);
         $m->addFields(['name', 'surname', 'cached_name']);
 
-        if ($this->getDatabasePlatform() instanceof SqlitePlatform) {
-            $concatExpr = '[name] || " " || [surname]';
-        } elseif ($this->getDatabasePlatform() instanceof OraclePlatform) {
+        if ($this->getDatabasePlatform() instanceof SqlitePlatform || $this->getDatabasePlatform() instanceof OraclePlatform) {
             $concatExpr = '[name] || \' \' || [surname]';
         } else {
             $concatExpr = 'CONCAT([name], \' \', [surname])';