From 920b4b9d84cb319006ac77ab94597b7e17d9ad18 Mon Sep 17 00:00:00 2001 From: HuaiyuXu <391585975@qq.com> Date: Thu, 21 Nov 2019 12:39:07 +0800 Subject: [PATCH 1/2] executor: refine the check of onlyFullGroupBy when groupByItem is parenthesesExpr or UnaryPlusExpr (#13655) --- executor/aggregate_test.go | 14 ++++++++++++++ planner/core/logical_plan_builder.go | 3 ++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/executor/aggregate_test.go b/executor/aggregate_test.go index fca05420430ad..d5f1b80a25944 100644 --- a/executor/aggregate_test.go +++ b/executor/aggregate_test.go @@ -566,6 +566,20 @@ func (s *testSuite1) TestOnlyFullGroupBy(c *C) { c.Assert(terror.ErrorEqual(err, plannercore.ErrAmbiguous), IsTrue, Commentf("err %v", err)) } +func (s *testSuite1) 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 *testSuite1) TestHaving(c *C) { tk := testkit.NewTestKitWithInit(c, s.store) diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index f462f55c5cc76..9037e1b9f0a11 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -1734,6 +1734,7 @@ func (b *PlanBuilder) checkOnlyFullGroupByWithGroupClause(p LogicalPlan, sel *as gbyExprs := make([]ast.ExprNode, 0, len(sel.Fields.Fields)) schema := p.Schema() for _, byItem := range sel.GroupBy.Items { + expr := getInnerFromParenthesesAndUnaryPlus(byItem.Expr) if colExpr, ok := byItem.Expr.(*ast.ColumnNameExpr); ok { col, err := schema.FindColumn(colExpr.Name) if err != nil || col == nil { @@ -1741,7 +1742,7 @@ func (b *PlanBuilder) checkOnlyFullGroupByWithGroupClause(p LogicalPlan, sel *as } gbyCols[col] = struct{}{} } else { - gbyExprs = append(gbyExprs, byItem.Expr) + gbyExprs = append(gbyExprs, expr) } } From 4f4df38d0397cc73225436e075d8d95ed6e059c4 Mon Sep 17 00:00:00 2001 From: xuhuaiyu <391585975@qq.com> Date: Tue, 17 Dec 2019 10:56:02 +0800 Subject: [PATCH 2/2] address comment --- planner/core/logical_plan_builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index 9037e1b9f0a11..fe5c16ea33ad4 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -1735,7 +1735,7 @@ func (b *PlanBuilder) checkOnlyFullGroupByWithGroupClause(p LogicalPlan, sel *as schema := p.Schema() for _, byItem := range sel.GroupBy.Items { expr := getInnerFromParenthesesAndUnaryPlus(byItem.Expr) - if colExpr, ok := byItem.Expr.(*ast.ColumnNameExpr); ok { + if colExpr, ok := expr.(*ast.ColumnNameExpr); ok { col, err := schema.FindColumn(colExpr.Name) if err != nil || col == nil { continue