From d9574cc5140363730e42649a54703fa80d3ec2eb Mon Sep 17 00:00:00 2001 From: Rasmus Schultz Date: Sun, 2 Jun 2024 11:48:15 +0000 Subject: [PATCH] remove the expr::group feature, which caused a 20x slowdown in benchmark - this change restores performance. the implications in terms of how many extra parens get added are actually quite reasonable - the conditional grouping feature seems to make very little difference on complex queries in practice. win/win :-) --- src/model/expr.php | 16 ++-------------- test/test-unit.php | 17 ++--------------- 2 files changed, 4 insertions(+), 29 deletions(-) diff --git a/src/model/expr.php b/src/model/expr.php index a9d3af9..f82d6a9 100644 --- a/src/model/expr.php +++ b/src/model/expr.php @@ -21,7 +21,7 @@ public static function any(array $exprs): string } return count($exprs) > 1 - ? implode(" OR ", array_map(self::group(...), $exprs)) + ? "(" . implode(") OR (", $exprs) . ")" : $exprs[0]; } @@ -37,19 +37,7 @@ public static function all(array $exprs): string } return count($exprs) > 1 - ? implode(" AND ", array_map(self::group(...), $exprs)) + ? "(" . implode(") AND (", $exprs) . ")" : $exprs[0]; } - - private const FULLY_GROUPED = '/^\((?:[^()]*|\((?:[^()]*|\([^()]*\))*\))*\)$/'; - - /** - * Group an expression in parentheses, if not already fully grouped - */ - public static function group(string $expr): string - { - return preg_match(self::FULLY_GROUPED, $expr) === 1 - ? $expr - : "($expr)"; - } } diff --git a/test/test-unit.php b/test/test-unit.php index cb8a355..c3a2db6 100644 --- a/test/test-unit.php +++ b/test/test-unit.php @@ -888,19 +888,6 @@ function () { } ); -test( - 'can conditionally group SQL expressions', - function () { - eq(expr::group('a OR b'), '(a OR b)', 'should group'); - eq(expr::group('(a) OR b'), '((a) OR b)', 'should group'); - eq(expr::group('(a) OR (b)'), '((a) OR (b))', 'should group'); - - eq(expr::group('(a OR b)'), '(a OR b)', 'should not regroup'); - eq(expr::group('((a) OR b)'), '((a) OR b)', 'should not regroup'); - eq(expr::group('((a) OR (b))'), '((a) OR (b))', 'should not regroup'); - } -); - test( 'can build and/or expressions', function () { @@ -930,8 +917,8 @@ function () { '(a OR b)', '(c OR d)' ]), - '(a OR b) AND (c OR d)', - 'should not regroup nested expressions' + '((a OR b)) AND ((c OR d))', + 'grouped expressions will be regrouped' ); eq(