Store GROUP_CONCAT separator as second argument

h2/src/main/org/h2/command/Parser.java

@@ -3055,25 +3055,31 @@ public class Parser {
             break;
         case GROUP_CONCAT: {
             boolean distinct = readDistinctAgg();
-            r = new Aggregate(AggregateType.GROUP_CONCAT, new Expression[] { readExpression() }, currentSelect,
-                    distinct);
+            Expression arg = readExpression(), separator = null;
+            ArrayList<SelectOrderBy> orderByList = null;
             if (equalsToken("STRING_AGG", aggregateName)) {
                 // PostgreSQL compatibility: string_agg(expression, delimiter)
                 read(COMMA);
-                r.setGroupConcatSeparator(readExpression());
+                separator = readExpression();
                 if (readIf(ORDER)) {
                     read("BY");
-                    r.setOrderByList(parseSimpleOrderList());
+                    orderByList = parseSimpleOrderList();
                 }
             } else {
                 if (readIf(ORDER)) {
                     read("BY");
-                    r.setOrderByList(parseSimpleOrderList());
+                    orderByList = parseSimpleOrderList();
                 }
                 if (readIf("SEPARATOR")) {
-                    r.setGroupConcatSeparator(readExpression());
+                    separator = readExpression();
                 }
             }
+            r = new Aggregate(AggregateType.GROUP_CONCAT,
+                    separator == null ? new Expression[] { arg } : new Expression[] { arg, separator },
+                            currentSelect, distinct);
+            if (orderByList != null) {
+                r.setOrderByList(orderByList);
+            }
             break;
         }
         case ARRAY_AGG: {

h2/src/main/org/h2/expression/aggregate/Aggregate.java

@@ -54,7 +54,6 @@ public class Aggregate extends AbstractAggregate {
     private final AggregateType aggregateType;
 
     private final Expression[] args;
-    private Expression groupConcatSeparator;
     private ArrayList<SelectOrderBy> orderByList;
     private SortOrder orderBySort;
     private TypeInfo type;
@@ -149,16 +148,6 @@ public class Aggregate extends AbstractAggregate {
         this.orderByList = orderByList;
     }
 
-    /**
-     * Set the separator for the GROUP_CONCAT() aggregate.
-     *
-     * @param separator
-     *            the separator expression
-     */
-    public void setGroupConcatSeparator(Expression separator) {
-        this.groupConcatSeparator = separator;
-    }
-
     /**
      * Returns the type of this aggregate.
      *
@@ -437,7 +426,7 @@ public class Aggregate extends AbstractAggregate {
             sortWithOrderBy(array);
         }
         StatementBuilder buff = new StatementBuilder();
-        String sep = groupConcatSeparator == null ? "," : groupConcatSeparator.getValue(session).getString();
+        String sep = args.length < 2 ? "," : args[1].getValue(session).getString();
         for (Value val : array) {
             String s;
             if (val.getValueType() == Value.ARRAY) {
@@ -536,9 +525,6 @@ public class Aggregate extends AbstractAggregate {
                 o.expression.mapColumns(resolver, level, innerState);
             }
         }
-        if (groupConcatSeparator != null) {
-            groupConcatSeparator.mapColumns(resolver, level, innerState);
-        }
         super.mapColumnsAnalysis(resolver, level, innerState);
     }
 
@@ -557,9 +543,6 @@ public class Aggregate extends AbstractAggregate {
             }
             orderBySort = createOrder(session, orderByList, 1);
         }
-        if (groupConcatSeparator != null) {
-            groupConcatSeparator = groupConcatSeparator.optimize(session);
-        }
         switch (aggregateType) {
         case GROUP_CONCAT:
             type = TypeInfo.TYPE_STRING_DEFAULT;
@@ -652,9 +635,6 @@ public class Aggregate extends AbstractAggregate {
                 o.expression.setEvaluatable(tableFilter, b);
             }
         }
-        if (groupConcatSeparator != null) {
-            groupConcatSeparator.setEvaluatable(tableFilter, b);
-        }
         super.setEvaluatable(tableFilter, b);
     }
 
@@ -665,9 +645,9 @@ public class Aggregate extends AbstractAggregate {
         }
         args[0].getSQL(builder);
         Window.appendOrderBy(builder, orderByList);
-        if (groupConcatSeparator != null) {
+        if (args.length >= 2) {
             builder.append(" SEPARATOR ");
-            groupConcatSeparator.getSQL(builder);
+            args[1].getSQL(builder);
         }
         builder.append(')');
         return appendTailConditions(builder);
@@ -834,9 +814,6 @@ public class Aggregate extends AbstractAggregate {
                 return false;
             }
         }
-        if (groupConcatSeparator != null && !groupConcatSeparator.isEverything(visitor)) {
-            return false;
-        }
         if (orderByList != null) {
             for (SelectOrderBy o : orderByList) {
                 if (!o.expression.isEverything(visitor)) {
@@ -853,9 +830,6 @@ public class Aggregate extends AbstractAggregate {
         for (Expression arg : args) {
             cost += arg.getCost();
         }
-        if (groupConcatSeparator != null) {
-            cost += groupConcatSeparator.getCost();
-        }
         if (orderByList != null) {
             for (SelectOrderBy o : orderByList) {
                 cost += o.expression.getCost();