fix GROUPING SETS formating

src/Parsers/ASTExpressionList.cpp

@@ -17,38 +17,24 @@ void ASTExpressionList::formatImpl(const FormatSettings & settings, FormatState
     if (frame.expression_list_prepend_whitespace)
         settings.ostr << ' ';
 
-    if (frame.need_parens)
+    for (ASTs::const_iterator it = children.begin(); it != children.end(); ++it)
     {
-        settings.ostr << "(";
+        if (it != children.begin())
-        for (ASTs::const_iterator it = children.begin(); it != children.end(); ++it)
         {
-            if (it != children.begin())
+            if (separator)
-            {
+                settings.ostr << separator;
-                if (separator)
+            settings.ostr << ' ';
-                    settings.ostr << separator;
+        }
-                settings.ostr << ' ';
-            }
 
+        if (frame.surround_each_list_element_with_parens)
             settings.ostr << "(";
-            FormatStateStacked frame_nested = frame;
+
-            frame_nested.need_parens = false;
+        FormatStateStacked frame_nested = frame;
-            (*it)->formatImpl(settings, state, frame_nested);
+        frame_nested.surround_each_list_element_with_parens = false;
+        (*it)->formatImpl(settings, state, frame_nested);
+
+        if (frame.surround_each_list_element_with_parens)
             settings.ostr << ")";
-        }
-        settings.ostr << ")";
-    }
-    else
-    {
-        for (ASTs::const_iterator it = children.begin(); it != children.end(); ++it)
-        {
-            if (it != children.begin())
-            {
-                if (separator)
-                    settings.ostr << separator;
-                settings.ostr << ' ';
-            }
-            (*it)->formatImpl(settings, state, frame);
-        }
     }
 }
 
@@ -64,52 +50,28 @@ void ASTExpressionList::formatImplMultiline(const FormatSettings & settings, For
 
     ++frame.indent;
 
-    if (frame.need_parens)
+    for (ASTs::const_iterator it = children.begin(); it != children.end(); ++it)
     {
-        for (ASTs::const_iterator it = children.begin(); it != children.end(); ++it)
+        if (it != children.begin())
         {
-            if (it != children.begin())
+            if (separator)
-            {
+                settings.ostr << separator;
-                if (separator)
+        }
-                    settings.ostr << separator;
-            }
 
-            if (children.size() > 1 || frame.expression_list_always_start_on_new_line)
+        if (children.size() > 1 || frame.expression_list_always_start_on_new_line)
-                settings.ostr << indent_str;
+            settings.ostr << indent_str;
 
-            if (it == children.begin())
+        FormatStateStacked frame_nested = frame;
-            {
+        frame_nested.expression_list_always_start_on_new_line = false;
-                settings.ostr << "(";
+        frame_nested.surround_each_list_element_with_parens = false;
-            }
 
-            FormatStateStacked frame_nested = frame;
+        if (frame.surround_each_list_element_with_parens)
-            frame_nested.expression_list_always_start_on_new_line = false;
-            frame_nested.expression_list_prepend_whitespace = false;
-            frame_nested.need_parens = false;
             settings.ostr << "(";
-            (*it)->formatImpl(settings, state, frame_nested);
+
+        (*it)->formatImpl(settings, state, frame_nested);
+
+        if (frame.surround_each_list_element_with_parens)
             settings.ostr << ")";
-        }
-        settings.ostr << ")";
-    }
-    else
-    {
-        for (ASTs::const_iterator it = children.begin(); it != children.end(); ++it)
-        {
-            if (it != children.begin())
-            {
-                if (separator)
-                    settings.ostr << separator;
-            }
-
-            if (children.size() > 1 || frame.expression_list_always_start_on_new_line)
-                settings.ostr << indent_str;
-
-            FormatStateStacked frame_nested = frame;
-            frame_nested.expression_list_always_start_on_new_line = false;
-
-            (*it)->formatImpl(settings, state, frame_nested);
-        }
     }
 }
 

src/Parsers/ASTSelectQuery.cpp

@@ -130,13 +130,14 @@ void ASTSelectQuery::formatImpl(const FormatSettings & s, FormatState & state, F
 
     if (group_by_with_grouping_sets)
     {
-        bool tmp_need_parens = frame.need_parens;
+        frame.surround_each_list_element_with_parens = true;
-        frame.need_parens = true;
         s.ostr << (s.hilite ? hilite_keyword : "") << s.nl_or_ws << indent_str << (s.one_line ? "" : "    ") << "GROUPING SETS" << (s.hilite ? hilite_none : "");
+        s.ostr << " (";
         s.one_line
         ? groupBy()->formatImpl(s, state, frame)
         : groupBy()->as<ASTExpressionList &>().formatImplMultiline(s, state, frame);
-        frame.need_parens = tmp_need_parens;
+        s.ostr << ")";
+        frame.surround_each_list_element_with_parens = false;
     }
 
     if (group_by_with_totals)

src/Parsers/IAST.h

@@ -208,6 +208,7 @@ public:
         bool need_parens = false;
         bool expression_list_always_start_on_new_line = false;  /// Line feed and indent before expression list even if it's of single element.
         bool expression_list_prepend_whitespace = false; /// Prepend whitespace (if it is required)
+        bool surround_each_list_element_with_parens = false;
         const IAST * current_select = nullptr;
     };