Implement GROUP BY () without columns (#1218)

manticore-projects · web-flow · commit 999db01658c3 · 2021-06-03T07:55:35.000+02:00
* Implement GROUP BY () without columns * Migrate GroupByElement to ExpressionList Also solves issue #1210 automatically * Solves issue #1168, add a test for it.
diff --git a/src/main/java/net/sf/jsqlparser/expression/operators/relational/ExpressionList.java b/src/main/java/net/sf/jsqlparser/expression/operators/relational/ExpressionList.java
@@ -24,7 +24,20 @@
 public class ExpressionList implements ItemsList {
 
     private List<Expression> expressions;
-    private boolean withBrackets = true;
+    private boolean usingBrackets = true;
+
+    public boolean isUsingBrackets() {
+        return usingBrackets;
+    }
+
+    public void setUsingBrackets(boolean usingBrackets) {
+        this.usingBrackets = usingBrackets;
+    }
+    
+    public ExpressionList withUsingBrackets(boolean usingBrackets) {
+        setUsingBrackets(usingBrackets);
+        return this;
+    }
 
     public ExpressionList() {
     }
@@ -56,9 +69,9 @@ public void setExpressions(List<Expression> expressions) {
         this.expressions = expressions;
     }
 
+    @Deprecated
     public ExpressionList withBrackets(boolean brackets) {
-        this.withBrackets = brackets;
-        return this;
+        return withUsingBrackets(brackets);
     }
 
     @Override
@@ -68,7 +81,7 @@ public void accept(ItemsListVisitor itemsListVisitor) {
 
     @Override
     public String toString() {
-        return PlainSelect.getStringList(expressions, true, withBrackets);
+        return PlainSelect.getStringList(expressions, true, usingBrackets);
     }
 
     public ExpressionList addExpressions(Collection<? extends Expression> expressions) {
diff --git a/src/main/java/net/sf/jsqlparser/statement/select/GroupByElement.java b/src/main/java/net/sf/jsqlparser/statement/select/GroupByElement.java
@@ -18,24 +18,51 @@
 import net.sf.jsqlparser.expression.operators.relational.ExpressionList;
 
 public class GroupByElement {
-
-    private List<Expression> groupByExpressions = new ArrayList<>();
+    // ExpressionList has 'usingBrackets = true' and so we need to switch it off explicitly
+    private ExpressionList groupByExpressions = new ExpressionList().withUsingBrackets(false);
     private List groupingSets = new ArrayList();
 
+    public boolean isUsingBrackets() {
+        return groupByExpressions.isUsingBrackets();
+    }
+
+    public void setUsingBrackets(boolean usingBrackets) {
+        this.groupByExpressions.setUsingBrackets(usingBrackets);
+    }
+
+    public GroupByElement withUsingBrackets(boolean usingBrackets) {
+        this.groupByExpressions.setUsingBrackets(usingBrackets);
+        return this;
+    }
+
     public void accept(GroupByVisitor groupByVisitor) {
         groupByVisitor.visit(this);
     }
-
-    public List<Expression> getGroupByExpressions() {
+    
+    public ExpressionList getGroupByExpressionList() {
         return groupByExpressions;
     }
+    
+    public void setGroupByExpressionList(ExpressionList groupByExpressions) {
+        this.groupByExpressions=groupByExpressions;
+    }
+    
+    @Deprecated
+    public List<Expression> getGroupByExpressions() {
+        return groupByExpressions.getExpressions();
+    }
 
+    @Deprecated
     public void setGroupByExpressions(List<Expression> groupByExpressions) {
-        this.groupByExpressions = groupByExpressions;
+        this.groupByExpressions.setExpressions(groupByExpressions);
     }
 
+    @Deprecated
     public void addGroupByExpression(Expression groupByExpression) {
-        groupByExpressions.add(groupByExpression);
+        if (groupByExpressions.getExpressions()==null) {
+            groupByExpressions.setExpressions(new ArrayList());
+        }
+        groupByExpressions.getExpressions().add(groupByExpression);
     }
 
     public List getGroupingSets() {
@@ -60,8 +87,14 @@ public String toString() {
         StringBuilder b = new StringBuilder();
         b.append("GROUP BY ");
 
-        if (groupByExpressions.size() > 0) {
-            b.append(PlainSelect.getStringList(groupByExpressions));
+        if (groupByExpressions.getExpressions()!=null && groupByExpressions.getExpressions().size() > 0) {
+            if (groupByExpressions.isUsingBrackets()) {
+                b.append("( ");
+            }
+            b.append(PlainSelect.getStringList(groupByExpressions.getExpressions()));
+            if (groupByExpressions.isUsingBrackets()) {
+                b.append(" )");
+            }
         } else if (groupingSets.size() > 0) {
             b.append("GROUPING SETS (");
             boolean first = true;
@@ -79,6 +112,10 @@ public String toString() {
                 }
             }
             b.append(")");
+        } else {
+            if (groupByExpressions.isUsingBrackets()) {
+                b.append("()");
+            }
         }
 
         return b.toString();
@@ -95,13 +132,15 @@ public GroupByElement withGroupingSets(List groupingSets) {
     }
 
     public GroupByElement addGroupByExpressions(Expression... groupByExpressions) {
-        List<Expression> collection = Optional.ofNullable(getGroupByExpressions()).orElseGet(ArrayList::new);
+        List<Expression> collection
+                = Optional.ofNullable(getGroupByExpressions()).orElseGet(ArrayList::new);
         Collections.addAll(collection, groupByExpressions);
         return this.withGroupByExpressions(collection);
     }
 
     public GroupByElement addGroupByExpressions(Collection<? extends Expression> groupByExpressions) {
-        List<Expression> collection = Optional.ofNullable(getGroupByExpressions()).orElseGet(ArrayList::new);
+        List<Expression> collection
+                = Optional.ofNullable(getGroupByExpressions()).orElseGet(ArrayList::new);
         collection.addAll(groupByExpressions);
         return this.withGroupByExpressions(collection);
     }
diff --git a/src/main/java/net/sf/jsqlparser/util/deparser/GroupByDeParser.java b/src/main/java/net/sf/jsqlparser/util/deparser/GroupByDeParser.java
@@ -10,6 +10,7 @@
 package net.sf.jsqlparser.util.deparser;
 
 import java.util.Iterator;
+import java.util.List;
 
 import net.sf.jsqlparser.expression.Expression;
 import net.sf.jsqlparser.expression.ExpressionVisitor;
@@ -34,12 +35,21 @@ public GroupByDeParser(ExpressionVisitor expressionVisitor, StringBuilder buffer
     @SuppressWarnings({"PMD.CyclomaticComplexity", "PMD.NPathComplexity"})
     public void deParse(GroupByElement groupBy) {
         buffer.append("GROUP BY ");
-        for (Iterator<Expression> iter = groupBy.getGroupByExpressions().iterator(); iter.hasNext();) {
-            iter.next().accept(expressionVisitor);
-            if (iter.hasNext()) {
-                buffer.append(", ");
+        if (groupBy.isUsingBrackets()) {
+            buffer.append("( ");
+        }
+        List<Expression> expressions = groupBy.getGroupByExpressionList().getExpressions();
+        if (expressions != null) {
+            for (Iterator<Expression> iter = expressions.iterator(); iter.hasNext();) {
+                iter.next().accept(expressionVisitor);
+                if (iter.hasNext()) {
+                    buffer.append(", ");
+                }
             }
         }
+        if (groupBy.isUsingBrackets()) {
+            buffer.append(" )");
+        }
         if (!groupBy.getGroupingSets().isEmpty()) {
             buffer.append("GROUPING SETS (");
             boolean first = true;
diff --git a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt
@@ -2325,9 +2325,15 @@ GroupByElement GroupByColumnReferences():
 }
 {
     <K_GROUP> <K_BY>
-    (
-        columnReference=SimpleExpression() {groupBy.addGroupByExpression(columnReference); }
-        ("," columnReference=SimpleExpression()  {groupBy.addGroupByExpression(columnReference); } )*
+    (     LOOKAHEAD(2) (
+              "(" ")" { groupBy.withUsingBrackets(true); }
+          )
+          | LOOKAHEAD(3) (
+              "(" list = SimpleExpressionList(true)  ")" { groupBy.setGroupByExpressionList(list); }
+          )
+          |
+          ( list = SimpleExpressionList(false) { groupBy.setGroupByExpressionList(list); }
+          )
     |
         <K_GROUPING> <K_SETS> "("
             ( LOOKAHEAD(2) "(" ")" { groupBy.addGroupingSet(new ExpressionList()); }
diff --git a/src/test/java/net/sf/jsqlparser/statement/select/SelectTest.java b/src/test/java/net/sf/jsqlparser/statement/select/SelectTest.java
@@ -4577,4 +4577,31 @@ public void testWithValueListWithOutExtraBrackets1135() throws JSQLParserExcepti
     public void testKeywordSynonymIssue1211() throws JSQLParserException {
         assertSqlCanBeParsedAndDeparsed("select businessDate as \"bd\", synonym as \"synonym\" from sc.tab", true);
     }
+
+    @Test
+    public void testGroupedByIssue1176() throws JSQLParserException {
+        assertSqlCanBeParsedAndDeparsed(
+                "select id_instrument, count(*)\n" + "from cfe.instrument\n" + "group by (id_instrument)",
+                true);
+        assertSqlCanBeParsedAndDeparsed("select count(*)\n" + "from cfe.instrument\n" + "group by ()",
+                true);
+    }
+    
+    @Test
+    public void testGroupedByWithExtraBracketsIssue1210() throws JSQLParserException {
+        assertSqlCanBeParsedAndDeparsed(
+                "select a,b,c from table group by rollup(a,b,c)",
+                true);
+        assertSqlCanBeParsedAndDeparsed("select a,b,c from table group by rollup((a,b,c))",
+                true);
+        
+        
+    }
+    
+    @Test
+    public void testGroupedByWithExtraBracketsIssue1168() throws JSQLParserException {
+        assertSqlCanBeParsedAndDeparsed(
+                "select sum(a) as amount, b, c from TEST_TABLE group by rollup ((a,b),c)",
+                true);
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -2325,9 +2325,15 @@ GroupByElement GroupByColumnReferences():`
`2325`	`2325`	`}`
`2326`	`2326`	`{`
`2327`	`2327`	`<K_GROUP> <K_BY>`
`2328`		`- (`
`2329`		`- columnReference=SimpleExpression() {groupBy.addGroupByExpression(columnReference); }`
`2330`		`- ("," columnReference=SimpleExpression() {groupBy.addGroupByExpression(columnReference); } )*`
	`2328`	`+ ( LOOKAHEAD(2) (`
	`2329`	`+ "(" ")" { groupBy.withUsingBrackets(true); }`
	`2330`	`+ )`
	`2331`	`+ \| LOOKAHEAD(3) (`
	`2332`	`+ "(" list = SimpleExpressionList(true) ")" { groupBy.setGroupByExpressionList(list); }`
	`2333`	`+ )`
	`2334`	`+ \|`
	`2335`	`+ ( list = SimpleExpressionList(false) { groupBy.setGroupByExpressionList(list); }`
	`2336`	`+ )`
`2331`	`2337`	`\|`
`2332`	`2338`	`<K_GROUPING> <K_SETS> "("`
`2333`	`2339`	`( LOOKAHEAD(2) "(" ")" { groupBy.addGroupingSet(new ExpressionList()); }`