From 20946baa1f193a1c0ef8978e4e85be476dbf526e Mon Sep 17 00:00:00 2001 From: imtbkcat Date: Tue, 21 Aug 2018 12:36:35 +0800 Subject: [PATCH 1/4] fix issue 7331 --- plan/logical_plan_builder.go | 2 ++ plan/logical_plan_test.go | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/plan/logical_plan_builder.go b/plan/logical_plan_builder.go index 5f18b30bff543..742dff42b786d 100644 --- a/plan/logical_plan_builder.go +++ b/plan/logical_plan_builder.go @@ -854,6 +854,8 @@ func matchField(f *ast.SelectField, col *ast.ColumnNameExpr, ignoreAsName bool) if f.AsName.L == "" || ignoreAsName { if curCol, isCol := f.Expr.(*ast.ColumnNameExpr); isCol { return curCol.Name.Name.L == col.Name.Name.L + } else if _, isFunc := f.Expr.(*ast.FuncCallExpr); isFunc { + return strings.ToLower(f.Text()) == col.Name.Name.L } // a expression without as name can't be matched. return false diff --git a/plan/logical_plan_test.go b/plan/logical_plan_test.go index c0615ffe6ed2e..d831699064374 100644 --- a/plan/logical_plan_test.go +++ b/plan/logical_plan_test.go @@ -1171,6 +1171,14 @@ func (s *testPlanSuite) TestValidate(c *C) { sql: "select a from t having sum(avg(a))", err: ErrInvalidGroupFuncUse, }, + { + sql: "select concat(c_str, d_str) from t group by `concat(c_str, d_str)`", + err: nil, + }, + { + sql: "select concat(c_str, d_str) from t group by `concat(c_str,d_str)`", + err: ErrUnknownColumn, + }, } for _, tt := range tests { sql := tt.sql From 2e6120f1c2fd49f0d0510d3d5f075843d8438a02 Mon Sep 17 00:00:00 2001 From: imtbkcat Date: Tue, 21 Aug 2018 14:33:41 +0800 Subject: [PATCH 2/4] add comment --- plan/logical_plan_builder.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plan/logical_plan_builder.go b/plan/logical_plan_builder.go index 742dff42b786d..5cdc45bf99b04 100644 --- a/plan/logical_plan_builder.go +++ b/plan/logical_plan_builder.go @@ -855,6 +855,11 @@ func matchField(f *ast.SelectField, col *ast.ColumnNameExpr, ignoreAsName bool) if curCol, isCol := f.Expr.(*ast.ColumnNameExpr); isCol { return curCol.Name.Name.L == col.Name.Name.L } else if _, isFunc := f.Expr.(*ast.FuncCallExpr); isFunc { + // Fix issue 7331 + // If ther are some function call in SelectField, we + // check if ColNameExpr in GroupByClause match these FunctionCallExpr + // Example: select concat(k1, k2) from t group by `concat(k1,k2)`, + // `concat(k1,k2)` matches FunctionCallExpr: concat(k1, k2) return strings.ToLower(f.Text()) == col.Name.Name.L } // a expression without as name can't be matched. From 5179fc986f35bc72e23d59fa1b566b28e0c36de1 Mon Sep 17 00:00:00 2001 From: imtbkcat Date: Tue, 21 Aug 2018 14:39:34 +0800 Subject: [PATCH 3/4] add comment --- plan/logical_plan_builder.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plan/logical_plan_builder.go b/plan/logical_plan_builder.go index 5cdc45bf99b04..7a3b2dd6a72d4 100644 --- a/plan/logical_plan_builder.go +++ b/plan/logical_plan_builder.go @@ -856,8 +856,8 @@ func matchField(f *ast.SelectField, col *ast.ColumnNameExpr, ignoreAsName bool) return curCol.Name.Name.L == col.Name.Name.L } else if _, isFunc := f.Expr.(*ast.FuncCallExpr); isFunc { // Fix issue 7331 - // If ther are some function call in SelectField, we - // check if ColNameExpr in GroupByClause match these FunctionCallExpr + // If there are some function call in SelectField, we + // check if ColumnNameExpr in GroupByClause match these FunctionCallExpr // Example: select concat(k1, k2) from t group by `concat(k1,k2)`, // `concat(k1,k2)` matches FunctionCallExpr: concat(k1, k2) return strings.ToLower(f.Text()) == col.Name.Name.L From 2e054e3117b716bd869c2f7d2969fd62d36eb3e1 Mon Sep 17 00:00:00 2001 From: imtbkcat Date: Tue, 21 Aug 2018 16:51:53 +0800 Subject: [PATCH 4/4] change comment --- plan/logical_plan_builder.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plan/logical_plan_builder.go b/plan/logical_plan_builder.go index 7a3b2dd6a72d4..db807b5315e11 100644 --- a/plan/logical_plan_builder.go +++ b/plan/logical_plan_builder.go @@ -856,10 +856,10 @@ func matchField(f *ast.SelectField, col *ast.ColumnNameExpr, ignoreAsName bool) return curCol.Name.Name.L == col.Name.Name.L } else if _, isFunc := f.Expr.(*ast.FuncCallExpr); isFunc { // Fix issue 7331 - // If there are some function call in SelectField, we - // check if ColumnNameExpr in GroupByClause match these FunctionCallExpr - // Example: select concat(k1, k2) from t group by `concat(k1,k2)`, - // `concat(k1,k2)` matches FunctionCallExpr: concat(k1, k2) + // If there are some function calls in SelectField, we check if + // ColumnNameExpr in GroupByClause matches one of these function calls. + // Example: select concat(k1,k2) from t group by `concat(k1,k2)`, + // `concat(k1,k2)` matches with function call concat(k1, k2). return strings.ToLower(f.Text()) == col.Name.Name.L } // a expression without as name can't be matched.