Skip to content

Commit eb59fab

Browse files
author
James Cor
committed
fix group by concat with order by subqueries
1 parent cf1535b commit eb59fab

File tree

4 files changed

+100
-26
lines changed

4 files changed

+100
-26
lines changed

enginetest/memory_engine_test.go

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -202,24 +202,40 @@ func TestSingleQueryPrepared(t *testing.T) {
202202

203203
// Convenience test for debugging a single query. Unskip and set to the desired query.
204204
func TestSingleScript(t *testing.T) {
205-
t.Skip()
205+
//t.Skip()
206206
var scripts = []queries.ScriptTest{
207207
{
208-
Name: "AS OF propagates to nested CALLs",
209-
SetUpScript: []string{},
208+
Name: "Group Concat with Subquery in ORDER BY",
209+
Dialect: "mysql",
210+
SetUpScript: []string{
211+
"CREATE TABLE test_data (id INT PRIMARY KEY, name VARCHAR(50), age INT, category VARCHAR(10))",
212+
`INSERT INTO test_data VALUES
213+
(1, 'Alice', 25, 'A'),
214+
(2, 'Bob', 30, 'B'),
215+
(3, 'Charlie', 22, 'A'),
216+
(4, 'Diana', 28, 'C'),
217+
(5, 'Eve', 35, 'B'),
218+
(6, 'Frank', 26, 'A')`,
219+
},
210220
Assertions: []queries.ScriptTestAssertion{
221+
//{
222+
// Query: "SELECT group_concat(name order by age) from test_data",
223+
// Expected: []sql.Row{},
224+
//},
211225
{
212-
Query: "create procedure create_proc() create table t (i int primary key, j int);",
213-
Expected: []sql.Row{
214-
{types.NewOkResult(0)},
215-
},
216-
},
217-
{
218-
Query: "call create_proc()",
219-
Expected: []sql.Row{
220-
{types.NewOkResult(0)},
221-
},
226+
//Skip: true,
227+
Query: "SELECT group_concat(name ORDER BY (SELECT sum(t2.age) FROM test_data t2 WHERE t2.age < test_data.age)) FROM test_data",
228+
Expected: []sql.Row{},
222229
},
230+
//{
231+
// Skip: true,
232+
// Query: "SELECT group_concat(name ORDER BY (SELECT AVG(age) FROM test_data t2 WHERE t2.category = test_data.category), id) FROM test_data;",
233+
// Expected: []sql.Row{{"Alice,Charlie,Frank,Diana,Bob,Eve"}},
234+
//},
235+
//{
236+
// Query: "SELECT category, group_concat(name ORDER BY (SELECT MAX(age) FROM test_data t2 WHERE t2.id <= test_data.id)) FROM test_data GROUP BY category ORDER BY category",
237+
// Expected: []sql.Row{{"A", "Alice,Charlie,Frank"}, {"B", "Bob,Eve"}, {"C", "Diana"}},
238+
//},
223239
},
224240
},
225241
}
@@ -232,8 +248,8 @@ func TestSingleScript(t *testing.T) {
232248
panic(err)
233249
}
234250

235-
//engine.EngineAnalyzer().Debug = true
236-
//engine.EngineAnalyzer().Verbose = true
251+
engine.EngineAnalyzer().Debug = true
252+
engine.EngineAnalyzer().Verbose = true
237253

238254
enginetest.TestScriptWithEngine(t, engine, harness, test)
239255
}

enginetest/queries/script_queries.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2775,12 +2775,12 @@ CREATE TABLE tab3 (
27752775
},
27762776
Assertions: []ScriptTestAssertion{
27772777
{
2778-
Skip: true,
2778+
//Skip: true,
27792779
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",
27802780
Expected: []sql.Row{{"A", "Charlie,Alice,Frank"}, {"B", "Bob,Eve"}, {"C", "Diana"}},
27812781
},
27822782
{
2783-
Skip: true,
2783+
//Skip: true,
27842784
Query: "SELECT group_concat(name ORDER BY (SELECT AVG(age) FROM test_data t2 WHERE t2.category = test_data.category), id) FROM test_data;",
27852785
Expected: []sql.Row{{"Alice,Charlie,Frank,Diana,Bob,Eve"}},
27862786
},
@@ -2804,22 +2804,22 @@ CREATE TABLE tab3 (
28042804
},
28052805
Assertions: []ScriptTestAssertion{
28062806
{
2807-
Skip: true,
2807+
//Skip: true,
28082808
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",
28092809
Expected: []sql.Row{{1, "Laptop,Keyboard,Mouse,Monitor"}, {2, "Chair,Desk"}},
28102810
},
28112811
{
2812-
Skip: true,
2812+
//Skip: true,
28132813
Query: "SELECT GROUP_CONCAT(name ORDER BY (SELECT COUNT(*) FROM products p2 WHERE p2.price < products.price), id) FROM products",
28142814
Expected: []sql.Row{{"Mouse,Keyboard,Chair,Monitor,Desk,Laptop"}},
28152815
},
28162816
{
2817-
Skip: true,
2817+
//Skip: true,
28182818
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",
28192819
Expected: []sql.Row{{1, "2,1"}, {2, "3"}},
28202820
},
28212821
{
2822-
Skip: true,
2822+
//Skip: true,
28232823
Query: "SELECT GROUP_CONCAT(name ORDER BY (SELECT priority FROM categories WHERE categories.id = products.category_id), price) FROM products",
28242824
Expected: []sql.Row{{"Mouse,Keyboard,Monitor,Laptop,Chair,Desk"}},
28252825
},
@@ -2905,13 +2905,13 @@ CREATE TABLE tab3 (
29052905
},
29062906
{
29072907
// Test with subquery using LIMIT
2908-
Skip: true,
2908+
//Skip: true,
29092909
Query: "SELECT GROUP_CONCAT(data ORDER BY (SELECT weight FROM perf_test p2 WHERE p2.id = perf_test.id LIMIT 1)) FROM perf_test",
29102910
Expected: []sql.Row{{"C,A,E,B,D"}},
29112911
},
29122912
{
29132913
// Test with very small decimal differences in ORDER BY subquery
2914-
Skip: true,
2914+
//Skip: true,
29152915
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",
29162916
Expected: []sql.Row{{"C,A,E,B,D"}},
29172917
},

sql/analyzer/fix_exec_indexes.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package analyzer
1616

1717
import (
1818
"fmt"
19+
"github.com/dolthub/go-mysql-server/sql/expression/function/aggregation"
1920
"strings"
2021

2122
"github.com/dolthub/go-mysql-server/sql"
@@ -578,9 +579,21 @@ func (s *idxScope) visitSelf(n sql.Node) error {
578579
}
579580
if ne, ok := n.(sql.Expressioner); ok {
580581
scope := append(s.parentScopes, s.childScopes...)
582+
// default nodes can't see lateral join nodes, unless we're in lateral
583+
// join and lateral scopes are promoted to parent status
581584
for _, e := range ne.Expressions() {
582-
// default nodes can't see lateral join nodes, unless we're in lateral
583-
// join and lateral scopes are promoted to parent status
585+
// groupConcat is special as it appends results to outer scope row.
586+
if gc, isGc := e.(*aggregation.GroupConcat); isGc {
587+
selExprs := gc.GetSelectExprs()
588+
selScope := &idxScope{}
589+
for _, expr := range selExprs {
590+
selScope.columns = append(selScope.columns, expr.String())
591+
if gf, isGf := expr.(*expression.GetField); isGf {
592+
selScope.ids = append(selScope.ids, gf.Id())
593+
}
594+
}
595+
scope = append(scope, selScope)
596+
}
584597
s.expressions = append(s.expressions, fixExprToScope(e, scope...))
585598
}
586599
}
@@ -734,6 +747,10 @@ func fixExprToScope(e sql.Expression, scopes ...*idxScope) sql.Expression {
734747
// this error for the case of DEFAULT in a `plan.Values`, since we analyze the insert source in isolation (we
735748
// don't have the destination schema, and column references in default values are determined in the build phase)
736749

750+
if e.Name() == "sum(t2.age)" {
751+
print()
752+
}
753+
737754
idx, _ := newScope.getIdxId(e.Id(), e.String())
738755
if idx >= 0 {
739756
return e.WithIndex(idx), transform.NewTree, nil

sql/expression/function/aggregation/group_concat.go

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,40 @@ func (g *GroupConcat) String() string {
153153
return sb.String()
154154
}
155155

156+
func (g *GroupConcat) DebugString() string {
157+
sb := strings.Builder{}
158+
sb.WriteString("group_concat(")
159+
if g.distinct != "" {
160+
sb.WriteString(fmt.Sprintf("distinct %s", g.distinct))
161+
}
162+
163+
if g.selectExprs != nil {
164+
var exprs = make([]string, len(g.selectExprs))
165+
for i, expr := range g.selectExprs {
166+
exprs[i] = sql.DebugString(expr)
167+
}
168+
169+
sb.WriteString(strings.Join(exprs, ", "))
170+
}
171+
172+
if len(g.sf) > 0 {
173+
sb.WriteString(" order by ")
174+
for i, ob := range g.sf {
175+
if i > 0 {
176+
sb.WriteString(", ")
177+
}
178+
sb.WriteString(sql.DebugString(ob))
179+
}
180+
}
181+
182+
sb.WriteString(" separator ")
183+
sb.WriteString(fmt.Sprintf("'%s'", g.separator))
184+
185+
sb.WriteString(")")
186+
187+
return sb.String()
188+
}
189+
156190
// Type implements the Expression interface.
157191
// cc: https://dev.mysql.com/doc/refman/8.0/en/aggregate-functions.html#function_group-concat for explanations
158192
// on return type.
@@ -195,6 +229,12 @@ func (g *GroupConcat) WithChildren(children ...sql.Expression) (sql.Expression,
195229
return NewGroupConcat(g.distinct, g.sf.FromExpressions(orderByExpr...), g.separator, children[sortFieldMarker:], g.maxLen), nil
196230
}
197231

232+
// GetSelectExprs returns the select expressions
233+
// TODO: just expose the member variable
234+
func (g *GroupConcat) GetSelectExprs() []sql.Expression {
235+
return g.selectExprs
236+
}
237+
198238
type groupConcatBuffer struct {
199239
gc *GroupConcat
200240
rows []sql.Row
@@ -257,7 +297,8 @@ func (g *groupConcatBuffer) Update(ctx *sql.Context, originalRow sql.Row) error
257297

258298
// Append the current value to the end of the row. We want to preserve the row's original structure for
259299
// for sort ordering in the final step.
260-
g.rows = append(g.rows, append(originalRow, nil, vs))
300+
// TODO: why nil spacer?
301+
g.rows = append(g.rows, append(originalRow, vs))
261302

262303
return nil
263304
}

0 commit comments

Comments
 (0)