Skip to content

Commit

Permalink
planner: fix planner can't error for union-all query when new-only-fu…
Browse files Browse the repository at this point in the history
…ll-group-check is enabled (pingcap#59212)

close pingcap#59211
  • Loading branch information
AilinKid authored Feb 12, 2025
1 parent 453ccd7 commit ed9b7d5
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 3 deletions.
4 changes: 4 additions & 0 deletions pkg/planner/core/operator/logicalop/logical_projection.go
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,10 @@ func (p *LogicalProjection) ExtractFD() *fd.FDSet {
for idx, expr := range p.Exprs {
switch x := expr.(type) {
case *expression.Column:
// column projected as a new column again.
if int(x.UniqueID) != outputColsUniqueIDsArray[idx] {
fds.AddEquivalence(intset.NewFastIntSet(int(x.UniqueID)), intset.NewFastIntSet(outputColsUniqueIDsArray[idx]))
}
continue
case *expression.CorrelatedColumn:
// t1(a,b,c), t2(m,n)
Expand Down
29 changes: 28 additions & 1 deletion pkg/planner/core/operator/logicalop/logical_union_all.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@ import (

"github.com/pingcap/tidb/pkg/expression"
"github.com/pingcap/tidb/pkg/planner/core/base"
fd "github.com/pingcap/tidb/pkg/planner/funcdep"
"github.com/pingcap/tidb/pkg/planner/property"
"github.com/pingcap/tidb/pkg/planner/util"
"github.com/pingcap/tidb/pkg/planner/util/optimizetrace"
"github.com/pingcap/tidb/pkg/planner/util/optimizetrace/logicaltrace"
"github.com/pingcap/tidb/pkg/planner/util/utilfuncp"
"github.com/pingcap/tidb/pkg/util/intset"
"github.com/pingcap/tidb/pkg/util/plancodec"
)

Expand Down Expand Up @@ -191,7 +193,32 @@ func (p *LogicalUnionAll) ExhaustPhysicalPlans(prop *property.PhysicalProperty)

// CanPushToCop inherits BaseLogicalPlan.LogicalPlan.<21st> implementation.

// ExtractFD inherits BaseLogicalPlan.LogicalPlan.<22nd> implementation.
// ExtractFD implement base.LogicalPlan.<22nd> interface.
func (p *LogicalUnionAll) ExtractFD() *fd.FDSet {
// basically extract the children's fdSet.
childFDs := make([]*fd.FDSet, 0, len(p.children))
for _, child := range p.children {
childFD := child.ExtractFD()
childFDs = append(childFDs, childFD)
}
// check the output columns' not-null property.
res := &fd.FDSet{}
notNullCols := intset.NewFastIntSet()
for _, col := range p.Schema().Columns {
notNullCols.Insert(int(col.UniqueID))
}
for _, childFD := range childFDs {
notNullCols.IntersectionWith(childFD.NotNullCols)
}
res.MakeNotNull(notNullCols)
// check the equivalency between children.
equivs := fd.FindCommonEquivClasses(childFDs)
for _, equiv := range equivs {
res.AddEquivalenceUnion(equiv)
}
p.fdSet = res
return res
}

// GetBaseLogicalPlan inherits BaseLogicalPlan.LogicalPlan.<23rd> implementation.

Expand Down
2 changes: 2 additions & 0 deletions pkg/planner/core/stringer.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ func fdToString(in base.LogicalPlan, strs []string, idxs []int) ([]string, []int
strs = append(strs, "{"+x.FDs().String()+"}")
case *logicalop.LogicalJoin:
strs = append(strs, "{"+x.FDs().String()+"}")
case *logicalop.LogicalUnionAll:
strs = append(strs, "{"+x.FDs().String()+"}")
default:
}
return strs, idxs
Expand Down
14 changes: 13 additions & 1 deletion pkg/planner/funcdep/extract_fd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ func TestFDSet_ExtractFD(t *testing.T) {
}
}

func TestFDSet_ExtractFDForApply(t *testing.T) {
func TestFDSet_ExtractFDForApplyAndUnion(t *testing.T) {
store := testkit.CreateMockStore(t)
par := parser.New()
par.SetParserConfig(parser.ParserConfig{EnableWindowFunction: true, EnableStrictDoubleTypeCheck: true})
Expand All @@ -249,6 +249,8 @@ func TestFDSet_ExtractFDForApply(t *testing.T) {
tk.MustExec("CREATE TABLE X (a INT PRIMARY KEY, b INT, c INT, d INT, e INT)")
tk.MustExec("CREATE UNIQUE INDEX uni ON X (b, c)")
tk.MustExec("CREATE TABLE Y (m INT, n INT, p INT, q INT, PRIMARY KEY (m, n))")
tk.MustExec("create table t1 (a int not null, b int, c int)")
tk.MustExec("create table t2 (e int not null, f int, g int)")

tests := []struct {
sql string
Expand Down Expand Up @@ -308,6 +310,16 @@ func TestFDSet_ExtractFDForApply(t *testing.T) {
// p=1 is semi join's right condition which should **NOT** be conserved.
fd: "{(1)-->(2-5), (2,3)~~>(1,4,5)} >>> {(1)-->(2-5), (2,3)~~>(1,4,5)}",
},
{
sql: "select * from t1 union all select * from t2",
best: "UnionAll{DataScan(t1)->Projection->DataScan(t2)->Projection}",
fd: "{}",
},
{
sql: "select * from t1 where a=b union all select * from t2 where e=f",
best: "UnionAll{DataScan(t1)->Projection->DataScan(t2)->Projection}",
fd: "{(9,10)==(9,10)}",
},
}

ctx := context.TODO()
Expand Down
11 changes: 10 additions & 1 deletion pkg/planner/funcdep/fd_graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,10 @@ func (s *FDSet) addEquivalence(eqs intset.FastIntSet) {
// {} --> {a} + {a,b,c} --> {a,b,c} leading to extension {} --> {a,b,c}
s.AddConstants(eqClosure)
}
// the new equiv closure should share the same not-null attribute with the existed ones.
if s.NotNullCols.Intersects(eqClosure) {
s.MakeNotNull(eqClosure)
}
}

// AddEquivalence take two column id as parameters, establish a strict equivalence between
Expand All @@ -402,6 +406,12 @@ func (s *FDSet) AddEquivalence(from, to intset.FastIntSet) {
s.addEquivalence(from.Union(to))
}

// AddEquivalenceUnion add an equivalence union to fdSet.
func (s *FDSet) AddEquivalenceUnion(union intset.FastIntSet) {
// no check for trivial equivalence, since it's a union of multiple equivalent columns.
s.addEquivalence(union)
}

// AddConstants adds a strict FD to the source which indicates that each of the given column
// have the same constant value for all rows, or same null value for all rows if it's nullable.
//
Expand Down Expand Up @@ -1173,7 +1183,6 @@ func (s *FDSet) makeEquivMap(detCols, projectedCols intset.FastIntSet) map[int]i
// String returns format string of this FDSet.
func (s *FDSet) String() string {
var builder strings.Builder

for i := range s.fdEdges {
if i != 0 {
builder.WriteString(", ")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,12 @@ drop table if exists t1;
create table t1(a int not null, b int not null, index(a));
select b from t1 group by a;
Error 1055 (42000): Expression #1 of SELECT list is not in GROUP BY clause and contains nonaggregated column 'planner__funcdep__only_full_group_by.t1.b' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by
drop table if exists t;
create table t (a int, b int, c int);
insert into t values (1, 2, 3);
select a,max(b) as max_b from (select * from t union all select * from t) x ;
Error 8123 (HY000): In aggregated query without GROUP BY, expression #1 of SELECT list contains nonaggregated column 'x.a'; this is incompatible with sql_mode=only_full_group_by
select a,max(b) as max_b from (select * from t union all select 1,2,3 ) x ;
Error 8123 (HY000): In aggregated query without GROUP BY, expression #1 of SELECT list contains nonaggregated column 'x.a'; this is incompatible with sql_mode=only_full_group_by
select a,max(b) as max_b from (select * from t where a=1 union all select 1,2,3 ) x ;
Error 8123 (HY000): In aggregated query without GROUP BY, expression #1 of SELECT list contains nonaggregated column 'x.a'; this is incompatible with sql_mode=only_full_group_by
10 changes: 10 additions & 0 deletions tests/integrationtest/t/planner/funcdep/only_full_group_by.test
Original file line number Diff line number Diff line change
Expand Up @@ -157,3 +157,13 @@ create table t1(a int not null, b int not null, index(a));
-- error 1055
select b from t1 group by a;

# TestIssue59211
drop table if exists t;
create table t (a int, b int, c int);
insert into t values (1, 2, 3);
-- error 8123
select a,max(b) as max_b from (select * from t union all select * from t) x ;
-- error 8123
select a,max(b) as max_b from (select * from t union all select 1,2,3 ) x ;
-- error 8123
select a,max(b) as max_b from (select * from t where a=1 union all select 1,2,3 ) x ;

0 comments on commit ed9b7d5

Please sign in to comment.