Skip to content

Commit 9b75556

Browse files
authored
Merge pull request #3166 from dolthub/angela/groupby
Unskip GroupBy validation on Project, Having, and Sort nodes
2 parents c848ce9 + 5943a32 commit 9b75556

31 files changed

+880
-4064
lines changed

enginetest/enginetests.go

Lines changed: 1 addition & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -866,43 +866,7 @@ func TestOrderByGroupBy(t *testing.T, harness Harness) {
866866
}
867867
require.Equal(t, rowCount, 3)
868868

869-
// TODO: this should error; the order by doesn't count towards ONLY_FULL_GROUP_BY
870-
_, rowIter, _, err = e.Query(ctx, "select id, team from members group by team order by id")
871-
require.NoError(t, err)
872-
rowCount = 0
873-
for {
874-
row, err = rowIter.Next(ctx)
875-
if err == io.EOF {
876-
break
877-
}
878-
rowCount++
879-
require.NoError(t, err)
880-
881-
var val int64
882-
switch v := row[0].(type) {
883-
case int64:
884-
val = v
885-
case int32:
886-
val = int64(v)
887-
default:
888-
panic(fmt.Sprintf("unexpected type %T", v))
889-
}
890-
891-
team, ok, err := sql.Unwrap[string](ctx, row[1])
892-
require.True(t, ok)
893-
require.NoError(t, err)
894-
switch team {
895-
case "red":
896-
require.True(t, val == 3 || val == 4)
897-
case "orange":
898-
require.True(t, val == 5 || val == 6 || val == 7)
899-
case "purple":
900-
require.True(t, val == 8)
901-
default:
902-
panic("received non-existent team")
903-
}
904-
}
905-
require.Equal(t, rowCount, 3)
869+
AssertErr(t, e, harness, "select id, team from members group by team order by id", nil, analyzererrors.ErrValidationGroupBy)
906870
})
907871
}
908872

enginetest/join_planning_tests.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ order by 1;`,
512512
},
513513
{
514514
// group by doesn't transform
515-
q: "select * from xy where y-1 in (select u from uv group by v having v = 2 order by 1) order by 1;",
515+
q: "select * from xy where y-1 in (select u from uv group by u having u = 2 order by 1) order by 1;",
516516
types: []plan.JoinType{plan.JoinTypeSemi},
517517
exp: []sql.Row{{3, 3}},
518518
},

enginetest/queries/column_alias_queries.go

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,10 @@ var ColumnAliasQueries = []ScriptTest{
210210
Query: "SELECT 1 as a, (select a) as b;",
211211
Expected: []sql.Row{{1, 1}},
212212
},
213+
{
214+
Query: `SELECT 1 as a, (select a union select a) as b;`,
215+
Expected: []sql.Row{{1, 1}},
216+
},
213217
{
214218
Query: "SELECT 1 as a, (select a) as b from dual;",
215219
Expected: []sql.Row{{1, 1}},
@@ -226,6 +230,22 @@ var ColumnAliasQueries = []ScriptTest{
226230
Query: "SELECT 1 as a, (select a) from xy;",
227231
Expected: []sql.Row{{1, 1}, {1, 1}, {1, 1}, {1, 1}},
228232
},
233+
{
234+
// https://github.com/dolthub/dolt/issues/4256
235+
Query: `SELECT *, (select i union select i) as a from mytable;`,
236+
Expected: []sql.Row{
237+
{1, "first row", 1},
238+
{2, "second row", 2},
239+
{3, "third row", 3}},
240+
},
241+
{
242+
Query: "select 1 as b, (select b group by b order by b) order by 1;",
243+
Expected: []sql.Row{{1, 1}},
244+
},
245+
{
246+
Query: `select 1 as a, (select b), 0 as b;`,
247+
ExpectedErr: sql.ErrColumnNotFound,
248+
},
229249
},
230250
},
231251
{
@@ -245,39 +265,15 @@ var ColumnAliasQueries = []ScriptTest{
245265
},
246266
{
247267
Name: "various broken alias queries",
268+
Skip: true,
248269
Assertions: []ScriptTestAssertion{
249-
{
250-
// The second query in the union subquery returns "x" instead of mytable.i
251-
// https://github.com/dolthub/dolt/issues/4256
252-
Query: `SELECT *, (select i union select i) as a from mytable;`,
253-
Expected: []sql.Row{
254-
{1, "first row", 1},
255-
{2, "second row", 2},
256-
{3, "third row", 3}},
257-
},
258-
{
259-
Query: `SELECT 1 as a, (select a union select a) as b;`,
260-
Expected: []sql.Row{{1, 1}},
261-
},
262-
{
263-
// GMS executes this query, but it is not valid because of the forward ref of alias b.
264-
// GMS should return an error about an invalid forward-ref.
265-
Skip: true,
266-
Query: `select 1 as a, (select b), 0 as b;`,
267-
ExpectedErr: sql.ErrColumnNotFound,
268-
},
269270
{
270271
// GMS returns "expression 'dt.two' doesn't appear in the group by expressions", but MySQL will execute
271272
// this query.
272273
Query: "select 1 as a, one + 1 as mod1, dt.* from mytable as t1, (select 1, 2 from mytable) as dt (one, two) where dt.one > 0 group by one;",
273274
// column names: a, mod1, one, two
274275
Expected: []sql.Row{{1, 2, 1, 2}},
275276
},
276-
{
277-
// GMS returns `ambiguous column or alias name "b"` on both cases of `group by b` and `group by 1` inside subquery, but MySQL executes.
278-
Query: "select 1 as b, (select b group by b order by b) order by 1;",
279-
Expected: []sql.Row{{1, 1}},
280-
},
281277
},
282278
},
283279
}

enginetest/queries/imdb_plans.go

Lines changed: 226 additions & 226 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

enginetest/queries/information_schema_queries.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ var InfoSchemaQueries = []QueryTest{
4646
table_name, index_name, comment, non_unique, GROUP_CONCAT(column_name ORDER BY seq_in_index) AS COLUMNS
4747
FROM information_schema.statistics
4848
WHERE table_schema='mydb' AND table_name='mytable' AND index_name!="PRIMARY"
49-
GROUP BY index_name;`,
49+
GROUP BY index_name, comment, non_unique;`,
5050
ExpectedColumns: sql.Schema{
5151
{
5252
Name: "TABLE_NAME",
@@ -274,7 +274,7 @@ var InfoSchemaQueries = []QueryTest{
274274
WHERE FILE_TYPE = 'UNDO LOG'
275275
AND FILE_NAME IS NOT NULL
276276
AND LOGFILE_GROUP_NAME IS NOT NULL
277-
GROUP BY LOGFILE_GROUP_NAME, FILE_NAME, ENGINE, TOTAL_EXTENTS, INITIAL_SIZE
277+
GROUP BY LOGFILE_GROUP_NAME, FILE_NAME, ENGINE, TOTAL_EXTENTS, INITIAL_SIZE, EXTRA
278278
ORDER BY LOGFILE_GROUP_NAME
279279
`,
280280
Expected: nil,

0 commit comments

Comments
 (0)