Merge pull request ClickHouse#68455 from petern48/fix_trim_empty_string

Fix trim empty string
MikhailBurdukov · Sep 23, 2024 · 8b4a9b6 · 8b4a9b6
2 parents fdaf7e8 + 4edbf03
commit 8b4a9b6
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 14 deletions.
diff --git a/src/Parsers/ExpressionListParsers.cpp b/src/Parsers/ExpressionListParsers.cpp
@@ -1667,8 +1667,18 @@ class TrimLayer : public Layer
                 if (!mergeElement())
                     return false;
 
-                to_remove = makeASTFunction("regexpQuoteMeta", elements[0]);
-                elements.clear();
+                /// Trimming an empty string is a no-op.
+                ASTLiteral * ast_literal = typeid_cast<ASTLiteral *>(elements[0].get());
+                if (ast_literal && ast_literal->value.getType() == Field::Types::String && ast_literal->value.safeGet<String>().empty())
+                {
+                    noop = true;
+                }
+                else
+                {
+                    to_remove = makeASTFunction("regexpQuoteMeta", elements[0]);
+                    elements.clear();
+                }
+
                 state = 2;
             }
         }
@@ -1680,15 +1690,20 @@ class TrimLayer : public Layer
                 if (!mergeElement())
                     return false;
 
-                ASTPtr pattern_node;
-
+                if (noop)
+                {
+                    /// The operation does nothing.
+                }
                 if (char_override)
                 {
+                    ASTPtr pattern_node;
+
                     auto pattern_func_node = std::make_shared<ASTFunction>();
                     auto pattern_list_args = std::make_shared<ASTExpressionList>();
                     if (trim_left && trim_right)
                     {
-                        pattern_list_args->children = {
+                        pattern_list_args->children =
+                        {
                             std::make_shared<ASTLiteral>("^["),
                             to_remove,
                             std::make_shared<ASTLiteral>("]+|["),
@@ -1701,7 +1716,8 @@ class TrimLayer : public Layer
                     {
                         if (trim_left)
                         {
-                            pattern_list_args->children = {
+                            pattern_list_args->children =
+                            {
                                 std::make_shared<ASTLiteral>("^["),
                                 to_remove,
                                 std::make_shared<ASTLiteral>("]+")
@@ -1710,7 +1726,8 @@ class TrimLayer : public Layer
                         else
                         {
                             /// trim_right == false not possible
-                            pattern_list_args->children = {
+                            pattern_list_args->children =
+                            {
                                 std::make_shared<ASTLiteral>("["),
                                 to_remove,
                                 std::make_shared<ASTLiteral>("]+$")
@@ -1724,6 +1741,9 @@ class TrimLayer : public Layer
                     pattern_func_node->children.push_back(pattern_func_node->arguments);
 
                     pattern_node = std::move(pattern_func_node);
+
+                    elements.push_back(pattern_node);
+                    elements.push_back(std::make_shared<ASTLiteral>(""));
                 }
                 else
                 {
@@ -1740,12 +1760,6 @@ class TrimLayer : public Layer
                     }
                 }
 
-                if (char_override)
-                {
-                    elements.push_back(pattern_node);
-                    elements.push_back(std::make_shared<ASTLiteral>(""));
-                }
-
                 finished = true;
             }
         }
@@ -1756,14 +1770,18 @@ class TrimLayer : public Layer
 protected:
     bool getResultImpl(ASTPtr & node) override
     {
-        node = makeASTFunction(function_name, std::move(elements));
+        if (noop)
+            node = std::move(elements.at(1));
+        else
+            node = makeASTFunction(function_name, std::move(elements));
         return true;
     }
 
 private:
     bool trim_left;
     bool trim_right;
     bool char_override = false;
+    bool noop = false;
 
     ASTPtr to_remove;
     String function_name;

diff --git a/tests/queries/0_stateless/03224_trim_empty_string.reference b/tests/queries/0_stateless/03224_trim_empty_string.reference
@@ -0,0 +1,6 @@
+foo
+foo
+foo
+" foo "
+" foo "
+" foo "
diff --git a/tests/queries/0_stateless/03224_trim_empty_string.sql b/tests/queries/0_stateless/03224_trim_empty_string.sql
@@ -0,0 +1,7 @@
+SELECT trim(LEADING '' FROM 'foo');
+SELECT trim(TRAILING '' FROM 'foo');
+SELECT trim(BOTH '' FROM 'foo');
+
+SELECT trim(LEADING '' FROM ' foo ') FORMAT CSV;
+SELECT trim(TRAILING '' FROM ' foo ') FORMAT CSV;
+SELECT trim(BOTH '' FROM ' foo ') FORMAT CSV;