Skip to content

Commit

Permalink
executor: refine the check of onlyFullGroupBy when groupByItem is par…
Browse files Browse the repository at this point in the history
…enthesesExpr or UnaryPlusExpr (pingcap#13655)
  • Loading branch information
XuHuaiyu authored and XiaTianliang committed Dec 21, 2019
1 parent 2c333de commit 2525e52
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
14 changes: 14 additions & 0 deletions executor/aggregate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,20 @@ func (s *testSuiteAgg) TestOnlyFullGroupBy(c *C) {
c.Assert(terror.ErrorEqual(err, plannercore.ErrAmbiguous), IsTrue, Commentf("err %v", err))
}

func (s *testSuiteAgg) TestIssue13652(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")
tk.MustExec("set sql_mode = 'ONLY_FULL_GROUP_BY'")
tk.MustExec("drop table if exists t")
tk.MustExec("create table t(a real)")
tk.MustQuery("select a from t group by (a)")
tk.MustQuery("select a from t group by ((a))")
tk.MustQuery("select a from t group by +a")
tk.MustQuery("select a from t group by ((+a))")
_, err := tk.Exec("select a from t group by (-a)")
c.Assert(err.Error(), Equals, "[planner:1055]Expression #1 of SELECT list is not in GROUP BY clause and contains nonaggregated column 'test.t.a' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by")
}

func (s *testSuiteAgg) TestHaving(c *C) {
tk := testkit.NewTestKitWithInit(c, s.store)

Expand Down
5 changes: 3 additions & 2 deletions planner/core/logical_plan_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -1910,14 +1910,15 @@ func (b *PlanBuilder) checkOnlyFullGroupByWithGroupClause(p LogicalPlan, sel *as
gbyColNames := make(map[*types.FieldName]struct{}, len(sel.Fields.Fields))
gbyExprs := make([]ast.ExprNode, 0, len(sel.Fields.Fields))
for _, byItem := range sel.GroupBy.Items {
if colExpr, ok := byItem.Expr.(*ast.ColumnNameExpr); ok {
expr := getInnerFromParenthesesAndUnaryPlus(byItem.Expr)
if colExpr, ok := expr.(*ast.ColumnNameExpr); ok {
idx, err := expression.FindFieldName(p.OutputNames(), colExpr.Name)
if err != nil || idx < 0 {
continue
}
gbyColNames[p.OutputNames()[idx]] = struct{}{}
} else {
gbyExprs = append(gbyExprs, byItem.Expr)
gbyExprs = append(gbyExprs, expr)
}
}

Expand Down

0 comments on commit 2525e52

Please sign in to comment.