fix group_concat with order by subquery clauses (#3041)

Author: jycor

enginetest/queries/script_queries.go

@@ -2775,12 +2775,10 @@ CREATE TABLE tab3 (
 		},
 		Assertions: []ScriptTestAssertion{
 			{
-				Skip:     true,
 				Query:    "SELECT category, group_concat(name ORDER BY (SELECT COUNT(*) FROM test_data t2 WHERE t2.category = test_data.category AND t2.age < test_data.age)) FROM test_data GROUP BY category ORDER BY category",
 				Expected: []sql.Row{{"A", "Charlie,Alice,Frank"}, {"B", "Bob,Eve"}, {"C", "Diana"}},
 			},
 			{
-				Skip:     true,
 				Query:    "SELECT group_concat(name ORDER BY (SELECT AVG(age) FROM test_data t2 WHERE t2.category = test_data.category), id) FROM test_data;",
 				Expected: []sql.Row{{"Alice,Charlie,Frank,Diana,Bob,Eve"}},
 			},
@@ -2804,22 +2802,18 @@ CREATE TABLE tab3 (
 		},
 		Assertions: []ScriptTestAssertion{
 			{
-				Skip:     true,
 				Query:    "SELECT category_id, GROUP_CONCAT(name ORDER BY (SELECT rating FROM suppliers WHERE suppliers.id = products.supplier_id) DESC, id ASC) FROM products GROUP BY category_id ORDER BY category_id",
 				Expected: []sql.Row{{1, "Laptop,Keyboard,Mouse,Monitor"}, {2, "Chair,Desk"}},
 			},
 			{
-				Skip:     true,
 				Query:    "SELECT GROUP_CONCAT(name ORDER BY (SELECT COUNT(*) FROM products p2 WHERE p2.price < products.price), id) FROM products",
 				Expected: []sql.Row{{"Mouse,Keyboard,Chair,Monitor,Desk,Laptop"}},
 			},
 			{
-				Skip:     true,
 				Query:    "SELECT category_id, GROUP_CONCAT(DISTINCT supplier_id ORDER BY (SELECT rating FROM suppliers WHERE suppliers.id = products.supplier_id)) FROM products GROUP BY category_id",
 				Expected: []sql.Row{{1, "2,1"}, {2, "3"}},
 			},
 			{
-				Skip:     true,
 				Query:    "SELECT GROUP_CONCAT(name ORDER BY (SELECT priority FROM categories WHERE categories.id = products.category_id), price) FROM products",
 				Expected: []sql.Row{{"Mouse,Keyboard,Monitor,Laptop,Chair,Desk"}},
 			},
@@ -2861,21 +2855,31 @@ CREATE TABLE tab3 (
 		Assertions: []ScriptTestAssertion{
 			{
 				// Test with subquery returning NULL values
-				Skip:     true,
-				Query:    "SELECT category, GROUP_CONCAT(name ORDER BY (SELECT CASE WHEN complex_test.value > 80 THEN NULL ELSE complex_test.value END), name) FROM complex_test GROUP BY category ORDER BY category",
-				Expected: []sql.Row{{"X", "Alpha,Gamma"}, {"Y", "Epsilon,Beta"}, {"Z", "Delta"}},
+				Query: "SELECT category, GROUP_CONCAT(name ORDER BY (SELECT CASE WHEN complex_test.value > 80 THEN NULL ELSE complex_test.value END), name) FROM complex_test GROUP BY category ORDER BY category",
+				Expected: []sql.Row{
+					{"X", "Alpha,Gamma"},
+					{"Y", "Epsilon,Beta"},
+					{"Z", "Delta"},
+				},
 			},
 			{
 				// Test with correlated subquery using multiple tables
-				Skip:     true,
 				Query:    "SELECT GROUP_CONCAT(name ORDER BY (SELECT COUNT(*) FROM complex_test c2 WHERE c2.category = complex_test.category AND c2.value > complex_test.value), name) FROM complex_test",
 				Expected: []sql.Row{{"Alpha,Delta,Epsilon,Beta,Gamma"}},
 			},
+			{
+				// Test with subquery using multiple columns errors
+				Query:       "SELECT category, GROUP_CONCAT(name ORDER BY (SELECT AVG(value), name FROM complex_test c2 WHERE c2.id <= complex_test.id HAVING AVG(value) > 50) DESC) FROM complex_test GROUP BY category ORDER BY category",
+				ExpectedErr: sql.ErrInvalidOperandColumns,
+			},
 			{
 				// Test with subquery using aggregate functions with HAVING
-				Skip:     true,
-				Query:    "SELECT category, GROUP_CONCAT(name ORDER BY (SELECT AVG(value), name FROM complex_test c2 WHERE c2.id <= complex_test.id HAVING AVG(value) > 50) DESC) FROM complex_test GROUP BY category ORDER BY category",
-				Expected: []sql.Row{{"X", "Alpha,Gamma"}, {"Y", "Beta,Epsilon"}, {"Z", "Delta"}},
+				Query: "SELECT category, GROUP_CONCAT(name ORDER BY (SELECT AVG(value) FROM complex_test c2 WHERE c2.id <= complex_test.id HAVING AVG(value) > 50) DESC) FROM complex_test GROUP BY category ORDER BY category",
+				Expected: []sql.Row{
+					{"X", "Alpha,Gamma"},
+					{"Y", "Beta,Epsilon"},
+					{"Z", "Delta"},
+				},
 			},
 			{
 				// Test with DISTINCT and complex subquery
@@ -2884,9 +2888,8 @@ CREATE TABLE tab3 (
 			},
 			{
 				// Test with nested subqueries
-				Skip:     true,
-				Query:    "SELECT GROUP_CONCAT(name ORDER BY (SELECT COUNT(*) FROM complex_test c2 WHERE c2.value > (SELECT MIN(value) FROM complex_test c3 WHERE c3.category = complex_test.category))) FROM complex_test",
-				Expected: []sql.Row{{"Gamma,Alpha,Epsilon,Beta,Delta"}},
+				Query:    "SELECT GROUP_CONCAT(name ORDER BY (SELECT SUM(value) FROM complex_test c2 WHERE c2.value != (SELECT MIN(value) FROM complex_test c3 where c3.id = complex_test.id))) FROM complex_test;",
+				Expected: []sql.Row{{"Alpha,Epsilon,Gamma,Beta,Delta"}},
 			},
 		},
 	},
@@ -2905,13 +2908,11 @@ CREATE TABLE tab3 (
 			},
 			{
 				// Test with subquery using LIMIT
-				Skip:     true,
 				Query:    "SELECT GROUP_CONCAT(data ORDER BY (SELECT weight FROM perf_test p2 WHERE p2.id = perf_test.id LIMIT 1)) FROM perf_test",
 				Expected: []sql.Row{{"C,A,E,B,D"}},
 			},
 			{
 				// Test with very small decimal differences in ORDER BY subquery
-				Skip:     true,
 				Query:    "SELECT GROUP_CONCAT(data ORDER BY (SELECT weight + 0.001 * perf_test.id FROM perf_test p2 WHERE p2.id = perf_test.id)) FROM perf_test",
 				Expected: []sql.Row{{"C,A,E,B,D"}},
 			},

sql/aggregates.go

@@ -115,7 +115,7 @@ type WindowFrame interface {
 	StartNFollowing() Expression
 	// EndNPreceding returns whether a frame end preceding Expression or nil
 	EndNPreceding() Expression
-	// EndNPreceding returns whether a frame end following Expression or nil
+	// EndNFollowing returns whether a frame end following Expression or nil
 	EndNFollowing() Expression
 }
 
@@ -135,3 +135,9 @@ type AggregationBuffer interface {
 type WindowAggregation interface {
 	WindowAdaptableExpression
 }
+
+// OrderedAggregation are aggregate functions that modify the current working row with additional result columns.
+type OrderedAggregation interface {
+	// OutputExpressions gets a list of return expressions.
+	OutputExpressions() []Expression
+}

sql/analyzer/fix_exec_indexes.go

@@ -578,9 +578,23 @@ func (s *idxScope) visitSelf(n sql.Node) error {
 		}
 		if ne, ok := n.(sql.Expressioner); ok {
 			scope := append(s.parentScopes, s.childScopes...)
+			// default nodes can't see lateral join nodes, unless we're in lateral
+			// join and lateral scopes are promoted to parent status
 			for _, e := range ne.Expressions() {
-				// default nodes can't see lateral join nodes, unless we're in lateral
-				// join and lateral scopes are promoted to parent status
+				// OrderedAggregations are special as they append results to the outer scope row
+				// We need to account for this extra column in the rows when assigning indexes
+				// Example: gms/expression/function/aggregation/group_concat.go:groupConcatBuffer.Update()
+				if ordAgg, isOrdAgg := e.(sql.OrderedAggregation); isOrdAgg {
+					selExprs := ordAgg.OutputExpressions()
+					selScope := &idxScope{}
+					for _, expr := range selExprs {
+						selScope.columns = append(selScope.columns, expr.String())
+						if gf, isGf := expr.(*expression.GetField); isGf {
+							selScope.ids = append(selScope.ids, gf.Id())
+						}
+					}
+					scope = append(scope, selScope)
+				}
 				s.expressions = append(s.expressions, fixExprToScope(e, scope...))
 			}
 		}

sql/expression/function/aggregation/group_concat.go

@@ -40,6 +40,7 @@ type GroupConcat struct {
 var _ sql.FunctionExpression = &GroupConcat{}
 var _ sql.Aggregation = &GroupConcat{}
 var _ sql.WindowAdaptableExpression = (*GroupConcat)(nil)
+var _ sql.OrderedAggregation = (*GroupConcat)(nil)
 
 func NewEmptyGroupConcat() sql.Expression {
 	return &GroupConcat{}
@@ -153,6 +154,40 @@ func (g *GroupConcat) String() string {
 	return sb.String()
 }
 
+func (g *GroupConcat) DebugString() string {
+	sb := strings.Builder{}
+	sb.WriteString("group_concat(")
+	if g.distinct != "" {
+		sb.WriteString(fmt.Sprintf("distinct %s", g.distinct))
+	}
+
+	if g.selectExprs != nil {
+		var exprs = make([]string, len(g.selectExprs))
+		for i, expr := range g.selectExprs {
+			exprs[i] = sql.DebugString(expr)
+		}
+
+		sb.WriteString(strings.Join(exprs, ", "))
+	}
+
+	if len(g.sf) > 0 {
+		sb.WriteString(" order by ")
+		for i, ob := range g.sf {
+			if i > 0 {
+				sb.WriteString(", ")
+			}
+			sb.WriteString(sql.DebugString(ob))
+		}
+	}
+
+	sb.WriteString(" separator ")
+	sb.WriteString(fmt.Sprintf("'%s'", g.separator))
+
+	sb.WriteString(")")
+
+	return sb.String()
+}
+
 // Type implements the Expression interface.
 // cc: https://dev.mysql.com/doc/refman/8.0/en/aggregate-functions.html#function_group-concat for explanations
 // on return type.
@@ -195,6 +230,11 @@ func (g *GroupConcat) WithChildren(children ...sql.Expression) (sql.Expression,
 	return NewGroupConcat(g.distinct, g.sf.FromExpressions(orderByExpr...), g.separator, children[sortFieldMarker:], g.maxLen), nil
 }
 
+// OutputExpressions implements the OrderedAggregation interface.
+func (g *GroupConcat) OutputExpressions() []sql.Expression {
+	return g.selectExprs
+}
+
 type groupConcatBuffer struct {
 	gc          *GroupConcat
 	rows        []sql.Row
@@ -257,7 +297,7 @@ func (g *groupConcatBuffer) Update(ctx *sql.Context, originalRow sql.Row) error
 
 	// Append the current value to the end of the row. We want to preserve the row's original structure for
 	// for sort ordering in the final step.
-	g.rows = append(g.rows, append(originalRow, nil, vs))
+	g.rows = append(g.rows, append(originalRow, vs))
 
 	return nil
 }