Better format for groupby

.bazelrc

@@ -20,7 +20,8 @@
 # widely accepted by compilers. This may lead to strange behavior or compiler
 # errors in earlier compilers.
 build --cxxopt="-std=c++1z"
-build --sandbox_debug --verbose_failures --cxxopt="-DNDEBUG"
+build --sandbox_debug --verbose_failures
+build --cxxopt="-DNDEBUG"
 build --workspace_status_command "tools/workspace_status.sh"
 # By default, we don't suppress any warnings, to get clang-specific warning
 # suppression you can invoke with --config=clang

zetasql/parser/unparser.cc

@@ -1623,7 +1623,26 @@ void Unparser::visitASTGroupBy(const ASTGroupBy* node, void* data) {
   println("BY");
   {
     Formatter::Indenter indenter(&formatter_);
-    UnparseVectorWithSeparator(node->grouping_items(), data, ",");
+    std::vector<const ASTIntLiteral*> int_grouping_items;
+    std::vector<const ASTGroupingItem*> other_grouping_items;
+    for (const auto grouping_item : node->grouping_items()) {
+      const auto& expr = grouping_item->expression();
+      if (expr == nullptr) {
+        other_grouping_items.push_back(grouping_item);
+        continue;
+      }
+      const auto& int_item = expr->GetAsOrNull<ASTIntLiteral>();
+      if (int_item == nullptr) {
+        other_grouping_items.push_back(grouping_item);
+        continue;
+      }
+      int_grouping_items.push_back(int_item);
+    }
+    UnparseVectorWithSeparator(absl::Span<const ASTIntLiteral* const>(int_grouping_items), data, ",");
+    if (int_grouping_items.size() > 0 && other_grouping_items.size() > 0) {
+      println(",");
+    }
+    UnparseVectorWithSeparator(absl::Span<const ASTGroupingItem* const>(other_grouping_items), data, ",\n");
   }
 }
 

zetasql/public/sql_formatter_test.cc

@@ -195,7 +195,10 @@ TEST(SqlFormatterTest, SeparatorAndGroupBy) {
       "  AND col2 > 10\n"
       "  AND col3 IS NOT NULL\n"
       "GROUP BY\n"
-      "  0, x, y, z;\n");
+      "  0, 1,\n"
+      "  x,\n"
+      "  y,\n"
+      "  z;\n");
   std::string formatted_sql;
   ZETASQL_ASSERT_OK(FormatSql(query_string, &formatted_sql));
   EXPECT_EQ(query_string,