passing all tests except query plan tests

Author: angelamayxie

enginetest/enginetests.go

@@ -865,44 +865,8 @@ func TestOrderByGroupBy(t *testing.T, harness Harness) {
 			}
 		}
 		require.Equal(t, rowCount, 3)
-
-		// TODO: this should error; the order by doesn't count towards ONLY_FULL_GROUP_BY
-		_, rowIter, _, err = e.Query(ctx, "select id, team from members group by team order by id")
-		require.NoError(t, err)
-		rowCount = 0
-		for {
-			row, err = rowIter.Next(ctx)
-			if err == io.EOF {
-				break
-			}
-			rowCount++
-			require.NoError(t, err)
-
-			var val int64
-			switch v := row[0].(type) {
-			case int64:
-				val = v
-			case int32:
-				val = int64(v)
-			default:
-				panic(fmt.Sprintf("unexpected type %T", v))
-			}
-
-			team, ok, err := sql.Unwrap[string](ctx, row[1])
-			require.True(t, ok)
-			require.NoError(t, err)
-			switch team {
-			case "red":
-				require.True(t, val == 3 || val == 4)
-			case "orange":
-				require.True(t, val == 5 || val == 6 || val == 7)
-			case "purple":
-				require.True(t, val == 8)
-			default:
-				panic("received non-existent team")
-			}
-		}
-		require.Equal(t, rowCount, 3)
+		
+		AssertErr(t, e, harness, "select id, team from members group by team order by id", nil, analyzererrors.ErrValidationGroupBy)
 	})
 }
 

enginetest/queries/column_alias_queries.go

@@ -210,6 +210,10 @@ var ColumnAliasQueries = []ScriptTest{
 				Query:    "SELECT 1 as a, (select a) as b;",
 				Expected: []sql.Row{{1, 1}},
 			},
+			{
+				Query:    `SELECT 1 as a, (select a union select a) as b;`,
+				Expected: []sql.Row{{1, 1}},
+			},
 			{
 				Query:    "SELECT 1 as a, (select a) as b from dual;",
 				Expected: []sql.Row{{1, 1}},
@@ -226,6 +230,22 @@ var ColumnAliasQueries = []ScriptTest{
 				Query:    "SELECT 1 as a, (select a) from xy;",
 				Expected: []sql.Row{{1, 1}, {1, 1}, {1, 1}, {1, 1}},
 			},
+			{
+				// https://github.com/dolthub/dolt/issues/4256
+				Query: `SELECT *, (select i union select i) as a from mytable;`,
+				Expected: []sql.Row{
+					{1, "first row", 1},
+					{2, "second row", 2},
+					{3, "third row", 3}},
+			},
+			{
+				Query:    "select 1 as b, (select b group by b order by b) order by 1;",
+				Expected: []sql.Row{{1, 1}},
+			},
+			{
+				Query:       `select 1 as a, (select b), 0 as b;`,
+				ExpectedErr: sql.ErrColumnNotFound,
+			},
 		},
 	},
 	{
@@ -245,39 +265,15 @@ var ColumnAliasQueries = []ScriptTest{
 	},
 	{
 		Name: "various broken alias queries",
+		Skip: true,
 		Assertions: []ScriptTestAssertion{
-			{
-				// The second query in the union subquery returns "x" instead of mytable.i
-				// https://github.com/dolthub/dolt/issues/4256
-				Query: `SELECT *, (select i union select i) as a from mytable;`,
-				Expected: []sql.Row{
-					{1, "first row", 1},
-					{2, "second row", 2},
-					{3, "third row", 3}},
-			},
-			{
-				Query:    `SELECT 1 as a, (select a union select a) as b;`,
-				Expected: []sql.Row{{1, 1}},
-			},
-			{
-				// GMS executes this query, but it is not valid because of the forward ref of alias b.
-				// GMS should return an error about an invalid forward-ref.
-				Skip:        true,
-				Query:       `select 1 as a, (select b), 0 as b;`,
-				ExpectedErr: sql.ErrColumnNotFound,
-			},
 			{
 				// GMS returns "expression 'dt.two' doesn't appear in the group by expressions", but MySQL will execute
 				// this query.
 				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;",
 				// column names:  a, mod1, one, two
 				Expected: []sql.Row{{1, 2, 1, 2}},
 			},
-			{
-				// GMS returns `ambiguous column or alias name "b"` on both cases of `group by b` and `group by 1` inside subquery, but MySQL executes.
-				Query:    "select 1 as b, (select b group by b order by b) order by 1;",
-				Expected: []sql.Row{{1, 1}},
-			},
 		},
 	},
 }

enginetest/queries/information_schema_queries.go

@@ -46,7 +46,7 @@ var InfoSchemaQueries = []QueryTest{
      table_name, index_name, comment, non_unique, GROUP_CONCAT(column_name ORDER BY seq_in_index) AS COLUMNS 
    FROM information_schema.statistics 
    WHERE table_schema='mydb' AND table_name='mytable' AND index_name!="PRIMARY" 
-   GROUP BY index_name;`,
+   GROUP BY index_name, comment, non_unique;`,
 		ExpectedColumns: sql.Schema{
 			{
 				Name: "TABLE_NAME",
@@ -274,7 +274,7 @@ var InfoSchemaQueries = []QueryTest{
 		WHERE FILE_TYPE = 'UNDO LOG'
 			AND FILE_NAME IS NOT NULL
 			AND LOGFILE_GROUP_NAME IS NOT NULL
-		GROUP BY LOGFILE_GROUP_NAME, FILE_NAME, ENGINE, TOTAL_EXTENTS, INITIAL_SIZE
+		GROUP BY LOGFILE_GROUP_NAME, FILE_NAME, ENGINE, TOTAL_EXTENTS, INITIAL_SIZE, EXTRA
 		ORDER BY LOGFILE_GROUP_NAME
 		`,
 		Expected: nil,

enginetest/queries/json_scripts.go

@@ -516,7 +516,7 @@ var JsonScripts = []ScriptTest{
 		},
 		Assertions: []ScriptTestAssertion{
 			{
-				Query: "SELECT pk, JSON_ARRAYAGG(field) FROM (SELECT * FROM j ORDER BY pk) as sub GROUP BY field ORDER BY pk",
+				Query: "SELECT pk, JSON_ARRAYAGG(field) FROM (SELECT * FROM j ORDER BY pk) as sub GROUP BY pk, field ORDER BY pk",
 				Expected: []sql.Row{
 					{1, types.MustJSON(`[{"key1": {"key": "value"}}]`)},
 					{2, types.MustJSON(`[{"key1": "value1", "key2": "value2"}]`)},

enginetest/queries/sysbench_plans.go

@@ -253,7 +253,6 @@ func validateGroupBy(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scop
 	checkParent := false
 	var project *plan.Project
 	var having *plan.Having
-	var filter *plan.Filter
 	transform.Inspect(n, func(n sql.Node) bool {
 		defer func() {
 			parent = n
@@ -290,17 +289,16 @@ func validateGroupBy(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scop
 			exprs := make([]sql.Expression, 0)
 			exprs = append(exprs, n.GroupByExprs...)
 			if having != nil {
-				if eq, ok := having.Cond.(*expression.Equals); ok {
-					exprs = append(exprs, eq.Children()...)
-				}
+				exprs = append(exprs, getEqualsDependencies(having.Cond)...)
 			}
-			if filter != nil {
+			possibleJoin := n.Child
+			if filter, ok := n.Child.(*plan.Filter); ok {
+				possibleJoin = filter.Child
+				exprs = append(exprs, getEqualsDependencies(filter.Expression)...)
 			}
-			if join, ok := n.Child.(*plan.JoinNode); ok {
+			if join, ok := possibleJoin.(*plan.JoinNode); ok {
 				isJoin = true
-				if eq, ok := join.Filter.(*expression.Equals); ok {
-					exprs = append(exprs, eq.Children()...)
-				}
+				exprs = append(exprs, getEqualsDependencies(join.Filter)...)
 			}
 			for _, expr := range exprs {
 				sql.Inspect(expr, func(expr sql.Expression) bool {
@@ -310,6 +308,9 @@ func validateGroupBy(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scop
 					}
 					groupBys[exprStr] = true
 
+					if nameable, ok := expr.(sql.Nameable); ok {
+						groupBys[strings.ToLower(nameable.Name())] = true
+					}
 					_, isAlias := expr.(*expression.Alias)
 					return isAlias
 				})
@@ -334,8 +335,6 @@ func validateGroupBy(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scop
 			}
 		case *plan.Project:
 			project = n
-		case *plan.Filter:
-			filter = n
 		case *plan.Having:
 			having = n
 		}
@@ -345,6 +344,26 @@ func validateGroupBy(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scop
 	return n, transform.SameTree, err
 }
 
+func getEqualsDependencies(expr sql.Expression) []sql.Expression {
+	exprs := make([]sql.Expression, 0)
+	sql.Inspect(expr, func(expr sql.Expression) bool {
+		switch expr := expr.(type) {
+		case *expression.And:
+			return true
+		case *expression.Equals:
+			for _, e := range expr.Children() {
+				if and, ok := e.(*expression.And); ok {
+					exprs = append(exprs, getEqualsDependencies(and)...)
+				} else if _, ok := e.(*expression.Literal); !ok {
+					exprs = append(exprs, e)
+				}
+			}
+		}
+		return false
+	})
+	return exprs
+}
+
 func getSelectExprs(project *plan.Project, selectDeps []sql.Expression, groupBys map[string]bool) []sql.Expression {
 	if project == nil {
 		return selectDeps
@@ -392,6 +411,12 @@ func expressionReferencesOnlyGroupBys(groupBys map[string]bool, expr sql.Express
 				return false
 			}
 
+			if nameable, ok := expr.(sql.Nameable); ok {
+				if groupBys[strings.ToLower(nameable.Name())] {
+					return false
+				}
+			}
+
 			if len(expr.Children()) == 0 {
 				valid = false
 				return false

sql/analyzer/validation_rules.go

@@ -140,7 +140,7 @@ func (g *GroupBy) String() string {
 	}
 
 	_ = pr.WriteChildren(
-		fmt.Sprintf("Select Dependencies(%s)", strings.Join(selectDeps, ", ")),
+		fmt.Sprintf("SelectDeps(%s)", strings.Join(selectDeps, ", ")),
 		fmt.Sprintf("Grouping(%s)", strings.Join(grouping, ", ")),
 		g.Child.String(),
 	)

sql/plan/group_by.go

@@ -502,11 +502,6 @@ func (s *Subquery) IsNullable() bool {
 }
 
 func (s *Subquery) String() string {
-	//pr := sql.NewTreePrinter()
-	//_ = pr.WriteNode("Subquery")
-	//children := []string{fmt.Sprintf("cacheable: %t", s.canCacheResults()), s.Query.String()}
-	//_ = pr.WriteChildren(children...)
-	//return pr.String()
 	return fmt.Sprintf("Subquery(%s)", s.QueryString)
 }