From 0b4b4cfbea511f547d61c95d7816c37c281f34c9 Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Tue, 17 Aug 2021 16:18:00 +0800 Subject: [PATCH] cherry pick #25094 to release-5.2 Signed-off-by: ti-srebot --- expression/integration_test.go | 29 ++++++++++++++++++++++++++++ expression/simple_rewriter.go | 6 ++++++ planner/core/logical_plan_builder.go | 12 ++++++++---- planner/core/logical_plan_test.go | 2 +- types/field_name.go | 2 ++ 5 files changed, 46 insertions(+), 5 deletions(-) diff --git a/expression/integration_test.go b/expression/integration_test.go index e2f620da24811..2add5903e6346 100644 --- a/expression/integration_test.go +++ b/expression/integration_test.go @@ -9715,6 +9715,35 @@ func (s *testIntegrationSuite) TestGlobalCacheCorrectness(c *C) { tk.MustExec("SET GLOBAL max_connections=151") } +func (s *testIntegrationSuite) TestRedundantColumnResolve(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t1, t2") + tk.MustExec("create table t1(a int not null)") + tk.MustExec("create table t2(a int not null)") + tk.MustExec("insert into t1 values(1)") + tk.MustExec("insert into t2 values(1)") + tk.MustQuery("select a, count(*) from t1 join t2 using (a) group by a").Check(testkit.Rows("1 1")) + tk.MustQuery("select a, count(*) from t1 natural join t2 group by a").Check(testkit.Rows("1 1")) + err := tk.ExecToErr("select a, count(*) from t1 join t2 on t1.a=t2.a group by a") + c.Assert(err.Error(), Equals, "[planner:1052]Column 'a' in field list is ambiguous") + tk.MustQuery("select t1.a, t2.a from t1 join t2 using (a) group by t1.a").Check(testkit.Rows("1 1")) + err = tk.ExecToErr("select t1.a, t2.a from t1 join t2 using(a) group by a") + c.Assert(err.Error(), Equals, "[planner:1052]Column 'a' in group statement is ambiguous") + tk.MustQuery("select t2.a from t1 join t2 using (a) group by t1.a").Check(testkit.Rows("1")) + tk.MustQuery("select t1.a from t1 join t2 using (a) group by t1.a").Check(testkit.Rows("1")) + tk.MustQuery("select t2.a from t1 join t2 using (a) group by t2.a").Check(testkit.Rows("1")) + // The test below cannot pass now since we do not infer functional dependencies from filters as MySQL, hence would fail in only_full_group_by check. + // tk.MustQuery("select t1.a from t1 join t2 using (a) group by t2.a").Check(testkit.Rows("1")) + tk.MustQuery("select count(*) from t1 join t2 using (a) group by t2.a").Check(testkit.Rows("1")) + tk.MustQuery("select t2.a from t1 join t2 using (a) group by a").Check(testkit.Rows("1")) + tk.MustQuery("select t1.a from t1 join t2 using (a) group by a").Check(testkit.Rows("1")) + tk.MustQuery("select * from t1 join t2 using(a)").Check(testkit.Rows("1")) + tk.MustQuery("select t1.a, t2.a from t1 join t2 using(a)").Check(testkit.Rows("1 1")) + tk.MustQuery("select * from t1 natural join t2").Check(testkit.Rows("1")) + tk.MustQuery("select t1.a, t2.a from t1 natural join t2").Check(testkit.Rows("1 1")) +} + func (s *testIntegrationSuite) TestControlFunctionWithEnumOrSet(c *C) { defer s.cleanEnv(c) diff --git a/expression/simple_rewriter.go b/expression/simple_rewriter.go index e0e9f41fd1bbf..72a2e0818373e 100644 --- a/expression/simple_rewriter.go +++ b/expression/simple_rewriter.go @@ -128,6 +128,12 @@ func FindFieldName(names types.NameSlice, astCol *ast.ColumnName) (int, error) { if idx == -1 { idx = i } else { + if names[idx].Redundant || name.Redundant { + if !name.Redundant { + idx = i + } + continue + } return -1, errNonUniq.GenWithStackByArgs(astCol.String(), "field list") } } diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index 6db52285b6953..d1aaacb258035 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -840,6 +840,7 @@ func (b *PlanBuilder) coalesceCommonColumns(p *LogicalJoin, leftPlan, rightPlan lColumns, rColumns := lsc.Columns, rsc.Columns lNames, rNames := leftPlan.OutputNames().Shallow(), rightPlan.OutputNames().Shallow() if joinTp == ast.RightJoin { + leftPlan, rightPlan = rightPlan, leftPlan lNames, rNames = rNames, lNames lColumns, rColumns = rsc.Columns, lsc.Columns } @@ -937,16 +938,18 @@ func (b *PlanBuilder) coalesceCommonColumns(p *LogicalJoin, leftPlan, rightPlan p.SetSchema(expression.NewSchema(schemaCols...)) p.names = names - if joinTp == ast.RightJoin { - leftPlan, rightPlan = rightPlan, leftPlan - } // We record the full `rightPlan.Schema` as `redundantSchema` in order to // record the redundant column in `rightPlan` and the output columns order // of the `rightPlan`. // For SQL like `select t1.*, t2.* from t1 left join t2 using(a)`, we can // retrieve the column order of `t2.*` from the `redundantSchema`. p.redundantSchema = expression.MergeSchema(p.redundantSchema, expression.NewSchema(rightPlan.Schema().Clone().Columns...)) - p.redundantNames = append(p.redundantNames.Shallow(), rightPlan.OutputNames().Shallow()...) + p.redundantNames = p.redundantNames.Shallow() + for _, name := range rightPlan.OutputNames() { + cpyName := *name + cpyName.Redundant = true + p.redundantNames = append(p.redundantNames, &cpyName) + } if joinTp == ast.RightJoin || joinTp == ast.LeftJoin { resetNotNullFlag(p.redundantSchema, 0, p.redundantSchema.Len()) } @@ -2545,6 +2548,7 @@ func (g *gbyResolver) Leave(inNode ast.Node) (ast.Node, bool) { var index int index, g.err = resolveFromSelectFields(v, g.fields, false) if g.err != nil { + g.err = ErrAmbiguous.GenWithStackByArgs(v.Name.Name.L, clauseMsg[groupByClause]) return inNode, false } if idx >= 0 { diff --git a/planner/core/logical_plan_test.go b/planner/core/logical_plan_test.go index 6259bc73fdc0f..dc67e612430a0 100644 --- a/planner/core/logical_plan_test.go +++ b/planner/core/logical_plan_test.go @@ -1456,7 +1456,7 @@ func (s *testPlanSuite) TestNameResolver(c *C) { {"select * from t", ""}, {"select t.* from t", ""}, {"select t2.* from t", "[planner:1051]Unknown table 't2'"}, - {"select b as a, c as a from t group by a", "[planner:1052]Column 'c' in field list is ambiguous"}, + {"select b as a, c as a from t group by a", "[planner:1052]Column 'a' in group statement is ambiguous"}, {"select 1 as a, b as a, c as a from t group by a", ""}, {"select a, b as a from t group by a+1", ""}, {"select c, a as c from t order by c+1", ""}, diff --git a/types/field_name.go b/types/field_name.go index 330c26041e544..918ca0fa3b87d 100644 --- a/types/field_name.go +++ b/types/field_name.go @@ -34,6 +34,8 @@ type FieldName struct { // update stmt can write `writeable` column implicitly but cannot use non-public columns explicit. // e.g. update t set a = 10 where b = 10; which `b` is in `writeOnly` state NotExplicitUsable bool + + Redundant bool } const emptyName = "EMPTY_NAME"