[Enhancement] Use sql_mode to be compatible with legacy group_concat

Signed-off-by: liuyehcf <[email protected]>
StarRocks · Nov 30, 2023 · 8def78e · 8def78e
1 parent 9cb912d
commit 8def78e
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 6 deletions.
diff --git a/fe/fe-core/src/main/java/com/starrocks/qe/SqlModeHelper.java b/fe/fe-core/src/main/java/com/starrocks/qe/SqlModeHelper.java
@@ -92,6 +92,7 @@ public class SqlModeHelper {
     public static final long MODE_SORT_NULLS_LAST = 1L << 33;
     public static final long MODE_LAST = 1L << 34;
     public static final long MODE_ERROR_IF_OVERFLOW = 1L << 35;
+    public static final long MODE_GROUP_CONCAT_LEGACY = 1L << 36;
 
     public static final long MODE_ALLOWED_MASK =
             (MODE_REAL_AS_FLOAT | MODE_PIPES_AS_CONCAT | MODE_ANSI_QUOTES |
@@ -103,7 +104,8 @@ public class SqlModeHelper {
                     MODE_NO_ZERO_DATE | MODE_INVALID_DATES | MODE_ERROR_FOR_DIVISION_BY_ZERO |
                     MODE_HIGH_NOT_PRECEDENCE | MODE_NO_ENGINE_SUBSTITUTION |
                     MODE_PAD_CHAR_TO_FULL_LENGTH | MODE_TRADITIONAL | MODE_ANSI |
-                    MODE_TIME_TRUNCATE_FRACTIONAL | MODE_SORT_NULLS_LAST) | MODE_ERROR_IF_OVERFLOW;
+                    MODE_TIME_TRUNCATE_FRACTIONAL | MODE_SORT_NULLS_LAST) | MODE_ERROR_IF_OVERFLOW |
+                    MODE_GROUP_CONCAT_LEGACY;
 
     private static final Map<String, Long> SQL_MODE_SET = Maps.newTreeMap(String.CASE_INSENSITIVE_ORDER);
 
@@ -137,6 +139,7 @@ public class SqlModeHelper {
         SQL_MODE_SET.put("TIME_TRUNCATE_FRACTIONAL", MODE_TIME_TRUNCATE_FRACTIONAL);
         SQL_MODE_SET.put("SORT_NULLS_LAST", MODE_SORT_NULLS_LAST);
         SQL_MODE_SET.put("ERROR_IF_OVERFLOW", MODE_ERROR_IF_OVERFLOW);
+        SQL_MODE_SET.put("GROUP_CONCAT_LEGACY", MODE_GROUP_CONCAT_LEGACY);
 
         COMBINE_MODE_SET.put("ANSI", (MODE_REAL_AS_FLOAT | MODE_PIPES_AS_CONCAT |
                 MODE_ANSI_QUOTES | MODE_IGNORE_SPACE | MODE_ONLY_FULL_GROUP_BY));

diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java b/fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java
@@ -97,6 +97,7 @@
 import com.starrocks.common.profile.Tracers;
 import com.starrocks.common.util.DateUtils;
 import com.starrocks.mysql.MysqlPassword;
+import com.starrocks.qe.ConnectContext;
 import com.starrocks.qe.SqlModeHelper;
 import com.starrocks.sql.analyzer.AnalyzerUtils;
 import com.starrocks.sql.analyzer.RelationId;
@@ -5713,6 +5714,7 @@ public ParseNode visitAggregationFunctionCall(StarRocksParser.AggregationFunctio
         NodePosition pos = createPos(context);
         String functionName;
         boolean isGroupConcat = false;
+        boolean isLegacyGroupConcat = false;
         if (context.aggregationFunction().COUNT() != null) {
             functionName = FunctionSet.COUNT;
         } else if (context.aggregationFunction().AVG() != null) {
@@ -5726,6 +5728,11 @@ public ParseNode visitAggregationFunctionCall(StarRocksParser.AggregationFunctio
         } else if (context.aggregationFunction().GROUP_CONCAT() != null) {
             functionName = FunctionSet.GROUP_CONCAT;
             isGroupConcat = true;
+            ConnectContext session = ConnectContext.get();
+            if (session != null && session.getSessionVariable() != null) {
+                long sqlMode = session.getSessionVariable().getSqlMode();
+                isLegacyGroupConcat = SqlModeHelper.check(sqlMode, SqlModeHelper.MODE_GROUP_CONCAT_LEGACY);
+            }
         } else {
             functionName = FunctionSet.MAX;
         }
@@ -5749,10 +5756,19 @@ public ParseNode visitAggregationFunctionCall(StarRocksParser.AggregationFunctio
         }
         List<Expr> exprs = visit(context.aggregationFunction().expression(), Expr.class);
         if (isGroupConcat && !exprs.isEmpty() && context.aggregationFunction().SEPARATOR() == null) {
-            Expr sepExpr;
-            String sep = ",";
-            sepExpr = new StringLiteral(sep, pos);
-            exprs.add(sepExpr);
+            if (isLegacyGroupConcat) {
+                if (exprs.size() == 1) {
+                    Expr sepExpr;
+                    String sep = ", ";
+                    sepExpr = new StringLiteral(sep, pos);
+                    exprs.add(sepExpr);
+                }
+            } else {
+                Expr sepExpr;
+                String sep = ",";
+                sepExpr = new StringLiteral(sep, pos);
+                exprs.add(sepExpr);
+            }
         }
         if (!orderByElements.isEmpty()) {
             int exprSize = exprs.size();

diff --git a/test/sql/test_agg_function/R/test_group_concat b/test/sql/test_agg_function/R/test_group_concat
@@ -1650,4 +1650,52 @@ TomEnglish,TomMath
 TomEnglish,王武程咬金语文北京上海
 张三掩耳盗铃Math数学欧拉方程,张三此地无银三百两英文English
 李四大闹天空英语外语美誉
+-- !result
+set group_concat_max_len = 1024;
+-- result:
+-- !result
+set sql_mode = 'GROUP_CONCAT_LEGACY';
+-- result:
+-- !result
+select group_concat( name ) from ss group by id order by 1;
+-- result:
+None
+May, Ti, 欧阳诸葛方程
+Ti
+Tom, Tom
+Tom, Tom, 王武程咬金
+张三此地无银三百两, 张三掩耳盗铃
+李四大闹天空
+-- !result
+select group_concat( name , "-") from ss group by id order by 1;
+-- result:
+None
+May-Ti-欧阳诸葛方程
+Ti
+Tom-Tom
+Tom-Tom-王武程咬金
+张三此地无银三百两-张三掩耳盗铃
+李四大闹天空
+-- !result
+select group_concat( subject , "-") from ss group by id order by 1;
+-- result:
+None
+English-Math
+English-数学大不列颠
+English-语文北京上海
+物理Phy
+英文English-Math数学欧拉方程
+英语外语美誉
+-- !result
+select group_concat( name ) from ss;
+-- result:
+May, Ti, 欧阳诸葛方程, Tom, Tom, 王武程咬金, Tom, Tom, 张三此地无银三百两, 张三掩耳盗铃, Ti, 李四大闹天空
+-- !result
+select group_concat( name , "-") from ss;
+-- result:
+Tom-Tom-王武程咬金-May-Ti-欧阳诸葛方程-Ti-李四大闹天空-Tom-Tom-张三此地无银三百两-张三掩耳盗铃
+-- !result
+select group_concat( subject , "-") from ss;
+-- result:
+English-数学大不列颠-English-Math-英文English-Math数学欧拉方程-English-语文北京上海-物理Phy-英语外语美誉
 -- !result
diff --git a/test/sql/test_agg_function/T/test_group_concat b/test/sql/test_agg_function/T/test_group_concat
@@ -301,4 +301,14 @@ select group_concat(name,subject order by 1,2) from ss group by id order by 1;
 set group_concat_max_len = 9;
 select group_concat(name,subject order by 1,2) from ss group by id order by 1;
 set group_concat_max_len = 121;
-select group_concat(name,subject order by 1,2) from ss group by id order by 1;
+select group_concat(name,subject order by 1,2) from ss group by id order by 1;
+
+-- legacy mode
+set group_concat_max_len = 1024;
+set sql_mode = 'GROUP_CONCAT_LEGACY';
+select group_concat( name ) from ss group by id order by 1;
+select group_concat( name , "-") from ss group by id order by 1;
+select group_concat( subject , "-") from ss group by id order by 1;
+select group_concat( name ) from ss;
+select group_concat( name , "-") from ss;
+select group_concat( subject , "-") from ss;