From ef3cf5ce731bdabd9e4dbb010830948529de4c30 Mon Sep 17 00:00:00 2001 From: xuhuaiyu <391585975@qq.com> Date: Sat, 29 Sep 2018 16:29:42 +0800 Subject: [PATCH 1/3] ast, plan: return error when the arg of VALUES() function is not a column --- ast/expressions.go | 5 ++++- plan/logical_plan_builder.go | 4 ++++ plan/logical_plan_test.go | 1 + 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/ast/expressions.go b/ast/expressions.go index 24e510ff9e5a0..f839f2985d885 100644 --- a/ast/expressions.go +++ b/ast/expressions.go @@ -931,7 +931,10 @@ func (n *ValuesExpr) Accept(v Visitor) (Node, bool) { if !ok { return n, false } - n.Column = node.(*ColumnNameExpr) + n.Column, ok = node.(*ColumnNameExpr) + if !ok { + n.Column = nil + } return v.Leave(n) } diff --git a/plan/logical_plan_builder.go b/plan/logical_plan_builder.go index dcb0bb5e81171..949904239ea15 100644 --- a/plan/logical_plan_builder.go +++ b/plan/logical_plan_builder.go @@ -1140,6 +1140,10 @@ func (g *gbyResolver) Leave(inNode ast.Node) (ast.Node, bool) { return inNode, false } return ret, true + case *ast.ValuesExpr: + if v.Column == nil { + g.err = ErrUnknownColumn.GenByArgs("", "VALUES() function") + } } return inNode, true } diff --git a/plan/logical_plan_test.go b/plan/logical_plan_test.go index 6a0f0133f79bc..ba07b1bf274a2 100644 --- a/plan/logical_plan_test.go +++ b/plan/logical_plan_test.go @@ -1752,6 +1752,7 @@ func (s *testPlanSuite) TestNameResolver(c *C) { {"select a from t group by t11.c1", "[planner:1054]Unknown column 't11.c1' in 'group statement'"}, {"delete a from (select * from t ) as a, t", "[planner:1288]The target table a of the DELETE is not updatable"}, {"delete b from (select * from t ) as a, t", "[planner:1109]Unknown table 'b' in MULTI DELETE"}, + {"select '' as fakeCol from t group by values(fakeCol)", "[planner:1054]Unknown column '' in 'VALUES() function'"}, } for _, t := range tests { From 7fcac680a4623ef67e0450d6fbf8d50f35dd1f40 Mon Sep 17 00:00:00 2001 From: xuhuaiyu <391585975@qq.com> Date: Sat, 29 Sep 2018 16:50:56 +0800 Subject: [PATCH 2/3] fix ci --- 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 45a08561555f9..9ebf16ede572e 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -1166,7 +1166,7 @@ func (g *gbyResolver) Leave(inNode ast.Node) (ast.Node, bool) { return ret, true case *ast.ValuesExpr: if v.Column == nil { - g.err = ErrUnknownColumn.GenByArgs("", "VALUES() function") + g.err = ErrUnknownColumn.GenWithStackByArgs("", "VALUES() function") } } return inNode, true From 68eeef3effc0c7bb1cbf5c87a987943f2fb223b5 Mon Sep 17 00:00:00 2001 From: xuhuaiyu <391585975@qq.com> Date: Sun, 30 Sep 2018 11:33:13 +0800 Subject: [PATCH 3/3] address comment --- ast/expressions.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ast/expressions.go b/ast/expressions.go index 6d98483e9d32b..e359b121c7444 100644 --- a/ast/expressions.go +++ b/ast/expressions.go @@ -927,10 +927,9 @@ func (n *ValuesExpr) Accept(v Visitor) (Node, bool) { if !ok { return n, false } + // `node` may be *ast.ValueExpr, to avoid panic, we write `ok` but do not use + // it. n.Column, ok = node.(*ColumnNameExpr) - if !ok { - n.Column = nil - } return v.Leave(n) }