plan: fix a bug when 'only_full_group_by' is set in sql_mode (#6734)

when 'only_full_group_by' is set in sql_mode, raise error for non-aggregated column in query without GROUP BY.

mysql> create table t(a bigint, b bigint, c bigint);                                                                                           Query OK, 0 rows affected (0.02 sec)

mysql> select a, max(b) from t;
ERROR 1140 (42000): In aggregated query without GROUP BY, expression #1 of SELECT list contains nonaggregated column 'dc_test.t.a'; this is incompatible with sql_mode=only_full_group_by

executor/aggregate_test.go

@@ -425,6 +425,14 @@ func (s *testSuite) TestOnlyFullGroupBy(c *C) {
 	c.Assert(terror.ErrorEqual(err, plan.ErrFieldNotInGroupBy), IsTrue)
 	_, err = tk.Exec("select -b from t group by c")
 	c.Assert(terror.ErrorEqual(err, plan.ErrFieldNotInGroupBy), IsTrue)
+	_, err = tk.Exec("select a, max(b) from t")
+	c.Assert(terror.ErrorEqual(err, plan.ErrMixOfGroupFuncAndFields), IsTrue)
+	_, err = tk.Exec("select sum(a)+b from t")
+	c.Assert(terror.ErrorEqual(err, plan.ErrMixOfGroupFuncAndFields), IsTrue)
+	_, err = tk.Exec("select count(b), c from t")
+	c.Assert(terror.ErrorEqual(err, plan.ErrMixOfGroupFuncAndFields), IsTrue)
+	_, err = tk.Exec("select distinct a, b, count(a) from t")
+	c.Assert(terror.ErrorEqual(err, plan.ErrMixOfGroupFuncAndFields), IsTrue)
 	// test compatible with sql_mode = ONLY_FULL_GROUP_BY
 	tk.MustQuery("select a from t group by a,b,c")
 	tk.MustQuery("select b from t group by b")
@@ -444,6 +452,9 @@ func (s *testSuite) TestOnlyFullGroupBy(c *C) {
 	tk.MustQuery("select b like '%a' from t group by b")
 	tk.MustQuery("select c REGEXP '1.*' from t group by c")
 	tk.MustQuery("select -b from t group by b")
+	tk.MustQuery("select max(a+b) from t")
+	tk.MustQuery("select avg(a)+1 from t")
+	tk.MustQuery("select count(c), 5 from t")
 	// test functinal depend on primary key
 	tk.MustQuery("select * from t group by a")
 	// test functional depend on unique not null columns

plan/errors.go

@@ -31,7 +31,6 @@ const (
 	codeUnknownColumn                = mysql.ErrBadField
 	codeUnknownTable                 = mysql.ErrUnknownTable
 	codeWrongArguments               = mysql.ErrWrongArguments
-	codeWrongNumberOfColumnsInSelect = mysql.ErrWrongNumberOfColumnsInSelect
 	codeBadGeneratedColumn           = mysql.ErrBadGeneratedColumn
 	codeFieldNotInGroupBy            = mysql.ErrFieldNotInGroupBy
 	codeBadTable                     = mysql.ErrBadTable
@@ -46,7 +45,9 @@ const (
 	codeDupFieldName                 = mysql.ErrDupFieldName
 	codeNonUpdatableTable            = mysql.ErrNonUpdatableTable
 	codeInternal                     = mysql.ErrInternal
+	codeMixOfGroupFuncAndFields      = mysql.ErrMixOfGroupFuncAndFields
 	codeNonUniqTable                 = mysql.ErrNonuniqTable
+	codeWrongNumberOfColumnsInSelect = mysql.ErrWrongNumberOfColumnsInSelect
 )
 
 // error definitions.
@@ -78,6 +79,7 @@ var (
 	ErrDupFieldName                 = terror.ClassOptimizer.New(codeDupFieldName, mysql.MySQLErrName[mysql.ErrDupFieldName])
 	ErrNonUpdatableTable            = terror.ClassOptimizer.New(codeNonUpdatableTable, mysql.MySQLErrName[mysql.ErrNonUpdatableTable])
 	ErrInternal                     = terror.ClassOptimizer.New(codeInternal, mysql.MySQLErrName[mysql.ErrInternal])
+	ErrMixOfGroupFuncAndFields      = terror.ClassOptimizer.New(codeMixOfGroupFuncAndFields, "In aggregated query without GROUP BY, expression #%d of SELECT list contains nonaggregated column '%s'; this is incompatible with sql_mode=only_full_group_by")
 	ErrNonUniqTable                 = terror.ClassOptimizer.New(codeNonUniqTable, mysql.MySQLErrName[mysql.ErrNonuniqTable])
 )
 
@@ -88,7 +90,6 @@ func init() {
 		codeUnknownColumn:                mysql.ErrBadField,
 		codeUnknownTable:                 mysql.ErrBadTable,
 		codeWrongArguments:               mysql.ErrWrongArguments,
-		codeWrongNumberOfColumnsInSelect: mysql.ErrWrongNumberOfColumnsInSelect,
 		codeBadGeneratedColumn:           mysql.ErrBadGeneratedColumn,
 		codeFieldNotInGroupBy:            mysql.ErrFieldNotInGroupBy,
 		codeBadTable:                     mysql.ErrBadTable,
@@ -103,7 +104,9 @@ func init() {
 		codeDupFieldName:                 mysql.ErrDupFieldName,
 		codeNonUpdatableTable:            mysql.ErrUnknownTable,
 		codeInternal:                     mysql.ErrInternal,
+		codeMixOfGroupFuncAndFields:      mysql.ErrMixOfGroupFuncAndFields,
 		codeNonUniqTable:                 mysql.ErrNonuniqTable,
+		codeWrongNumberOfColumnsInSelect: mysql.ErrWrongNumberOfColumnsInSelect,
 	}
 	terror.ErrClassToMySQLCodes[terror.ClassOptimizer] = mysqlErrCodeMap
 }

plan/logical_plan_builder.go

@@ -1338,6 +1338,14 @@ func checkExprInGroupBy(p LogicalPlan, expr ast.ExprNode, offset int, loc string
 }
 
 func (b *planBuilder) checkOnlyFullGroupBy(p LogicalPlan, fields []*ast.SelectField, orderBy *ast.OrderByClause, gby *ast.GroupByClause, from ast.ResultSetNode, where ast.ExprNode) {
+	if gby != nil {
+		b.checkOnlyFullGroupByWithGroupClause(p, fields, orderBy, gby, from, where)
+		return
+	}
+	b.checkOnlyFullGroupByWithOutGroupClause(p, fields)
+}
+
+func (b *planBuilder) checkOnlyFullGroupByWithGroupClause(p LogicalPlan, fields []*ast.SelectField, orderBy *ast.OrderByClause, gby *ast.GroupByClause, from ast.ResultSetNode, where ast.ExprNode) {
 	gbyCols := make(map[*expression.Column]struct{}, len(fields))
 	gbyExprs := make([]ast.ExprNode, 0, len(fields))
 	schema := p.Schema()
@@ -1394,6 +1402,52 @@ func (b *planBuilder) checkOnlyFullGroupBy(p LogicalPlan, fields []*ast.SelectFi
 	}
 }
 
+func (b *planBuilder) checkOnlyFullGroupByWithOutGroupClause(p LogicalPlan, fields []*ast.SelectField) {
+	resolver := colResolverForOnlyFullGroupBy{}
+	for idx, field := range fields {
+		resolver.exprIdx = idx
+		field.Accept(&resolver)
+		if err := resolver.Check(); err != nil {
+			b.err = errors.Trace(err)
+			return
+		}
+	}
+}
+
+// colResolverForOnlyFullGroupBy visits Expr tree to find out if an Expr tree is an aggregation function.
+// If so, find out the first column name that not in an aggregation function.
+type colResolverForOnlyFullGroupBy struct {
+	firstNonAggCol    *ast.ColumnName
+	exprIdx           int
+	firstNonAggColIdx int
+	hasAggFunc        bool
+}
+
+func (c *colResolverForOnlyFullGroupBy) Enter(node ast.Node) (ast.Node, bool) {
+	switch t := node.(type) {
+	case *ast.AggregateFuncExpr:
+		c.hasAggFunc = true
+		return node, true
+	case *ast.ColumnNameExpr:
+		if c.firstNonAggCol == nil {
+			c.firstNonAggCol, c.firstNonAggColIdx = t.Name, c.exprIdx
+		}
+		return node, true
+	}
+	return node, false
+}
+
+func (c *colResolverForOnlyFullGroupBy) Leave(node ast.Node) (ast.Node, bool) {
+	return node, true
+}
+
+func (c *colResolverForOnlyFullGroupBy) Check() error {
+	if c.hasAggFunc && c.firstNonAggCol != nil {
+		return ErrMixOfGroupFuncAndFields.GenByArgs(c.firstNonAggColIdx+1, c.firstNonAggCol.Name.O)
+	}
+	return nil
+}
+
 type colResolver struct {
 	p    LogicalPlan
 	cols map[*expression.Column]struct{}
@@ -1564,10 +1618,11 @@ func (b *planBuilder) buildSelect(sel *ast.SelectStmt) LogicalPlan {
 		if b.err != nil {
 			return nil
 		}
-		if b.ctx.GetSessionVars().SQLMode.HasOnlyFullGroupBy() {
-			b.checkOnlyFullGroupBy(p, sel.Fields.Fields, sel.OrderBy, sel.GroupBy, sel.From.TableRefs, sel.Where)
-		}
 	}
+	if b.ctx.GetSessionVars().SQLMode.HasOnlyFullGroupBy() && sel.From != nil {
+		b.checkOnlyFullGroupBy(p, sel.Fields.Fields, sel.OrderBy, sel.GroupBy, sel.From.TableRefs, sel.Where)
+	}
+
 	// We must resolve having and order by clause before build projection,
 	// because when the query is "select a+1 as b from t having sum(b) < 0", we must replace sum(b) to sum(a+1),
 	// which only can be done before building projection and extracting Agg functions.