[Enhancement] Use sql_mode to be compatible with legacy group_concat …

…(backport #36150) (#36318) Co-authored-by: liuyehcf <[email protected]>
StarRocks · Dec 4, 2023 · 67fc754 · 67fc754
1 parent 7788f06
commit 67fc754
Show file tree

Hide file tree

Showing 4 changed files with 94 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
@@ -94,6 +94,7 @@
 import com.starrocks.common.Pair;
 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;
@@ -5420,6 +5421,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) {
@@ -5433,6 +5435,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;
         }
@@ -5456,10 +5463,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,48 @@ TomEnglish,TomMath
 TomEnglish,王武程咬金语文北京上海
 张三掩耳盗铃Math数学欧拉方程,张三此地无银三百两英文English
 李四大闹天空英语外语美誉
+-- !result
+-- name: testLegacyGroupConcat
+CREATE TABLE t1 (
+    id        tinyint(4)      NULL,
+    value   varchar(65533)  NULL
+) ENGINE=OLAP
+DISTRIBUTED BY HASH(id)
+PROPERTIES (
+ "replication_num" = "1"
+);
+-- result:
+-- !result
+INSERT INTO t1 VALUES
+(1,'fruit'),
+(1,'fruit'),
+(1,'fruit'),
+(2,'fruit'),
+(2,'fruit'),
+(2,'fruit');
+-- result:
+-- !result
+set group_concat_max_len = 1024;
+-- result:
+-- !result
+set sql_mode = 'GROUP_CONCAT_LEGACY';
+-- result:
+-- !result
+select id, group_concat( value ) from t1 group by id order by id;
+-- result:
+1	fruit, fruit, fruit
+2	fruit, fruit, fruit
+-- !result
+select id, group_concat( value, '-' ) from t1 group by id order by id;
+-- result:
+1	fruit-fruit-fruit
+2	fruit-fruit-fruit
+-- !result
+select group_concat( value ) from t1;
+-- result:
+fruit, fruit, fruit, fruit, fruit, fruit
+-- !result
+select group_concat( value, '-' ) from t1;
+-- result:
+fruit-fruit-fruit-fruit-fruit-fruit
 -- !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,29 @@ 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;
+
+-- name: testLegacyGroupConcat
+CREATE TABLE t1 (
+    id        tinyint(4)      NULL,
+    value   varchar(65533)  NULL
+) ENGINE=OLAP
+DISTRIBUTED BY HASH(id)
+PROPERTIES (
+ "replication_num" = "1"
+);
+
+INSERT INTO t1 VALUES
+(1,'fruit'),
+(1,'fruit'),
+(1,'fruit'),
+(2,'fruit'),
+(2,'fruit'),
+(2,'fruit');
+
+set group_concat_max_len = 1024;
+set sql_mode = 'GROUP_CONCAT_LEGACY';
+select id, group_concat( value ) from t1 group by id order by id;
+select id, group_concat( value, '-' ) from t1 group by id order by id;
+select group_concat( value ) from t1;
+select group_concat( value, '-' ) from t1;