Skip to content

Commit

Permalink
*: revert #27021 to fix a bug that selection can not be pushed down w…
Browse files Browse the repository at this point in the history 
…hen having clause above aggregation (#33168)

close #33166
  • Loading branch information
@wjhuang2016
wjhuang2016 authored Mar 17, 2022
1 parent 46c43fe commit 47e4b5b
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 41 deletions.
12 changes: 0 additions & 12 deletions executor/aggregate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -927,18 +927,6 @@ func TestHaving(t *testing.T) {
tk.MustQuery("select 1 from t group by c1 having sum(abs(c2 + c3)) = c1").Check(testkit.Rows("1"))
}

func TestIssue26496(t *testing.T) {
store, clean := testkit.CreateMockStore(t)
defer clean()
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")

tk.MustExec("drop table if exists UK_NSPRE_19416")
tk.MustExec("CREATE TABLE `UK_NSPRE_19416` ( `COL1` binary(20) DEFAULT NULL, `COL2` varchar(20) DEFAULT NULL, `COL4` datetime DEFAULT NULL, `COL3` bigint(20) DEFAULT NULL, `COL5` float DEFAULT NULL, UNIQUE KEY `UK_COL1` (`COL1`)) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")
tk.MustExec("insert into `UK_NSPRE_19416`(col1) values (0xc5b428e2ebc1b78f0b183899a8df55c88a333f86), (0x004dad637b37cc4a9742484ab93f97ede2ab8bd5), (0x550c4a4390ba14fd6d382dd29063e10210c99381)")
tk.MustQuery("select t1.col1, count(t2.col1) from UK_NSPRE_19416 as t1 left join UK_NSPRE_19416 as t2 on t1.col1 = t2.col1 where t1.col1 in (0x550C4A4390BA14FD6D382DD29063E10210C99381, 0x004DAD637B37CC4A9742484AB93F97EDE2AB8BD5, 0xC5B428E2EBC1B78F0B183899A8DF55C88A333F86) group by t1.col1, t2.col1 having t1.col1 in (0x9B4B48FEBA9225BACF8F9ADEAEE810AEC26DC7A2, 0x25A6C4FAD832F8E0267AAA504CFAE767565C8B84, 0xE26E5B0080EC5A8156DACE67D13B239500E540E6)").Check(nil)
}

func TestAggEliminator(t *testing.T) {
store, clean := testkit.CreateMockStore(t)
defer clean()
Expand Down
2 changes: 1 addition & 1 deletion planner/core/logical_plan_builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -964,7 +964,7 @@ func (b *PlanBuilder) buildSelection(ctx context.Context, p LogicalPlan, where a

conditions := splitWhere(where)
expressions := make([]expression.Expression, 0, len(conditions))
selection := LogicalSelection{buildByHaving: aggMapper != nil}.Init(b.ctx, b.getSelectOffset())
selection := LogicalSelection{}.Init(b.ctx, b.getSelectOffset())
for _, cond := range conditions {
expr, np, err := b.rewrite(ctx, cond, p, aggMapper, false)
if err != nil {
Expand Down
3 changes: 0 additions & 3 deletions planner/core/logical_plans.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -457,9 +457,6 @@ type LogicalSelection struct {
// but after we converted to CNF(Conjunctive normal form), it can be
// split into a list of AND conditions.
Conditions []expression.Expression

// having selection can't be pushed down, because it must above the aggregation.
buildByHaving bool
}

// ExtractCorrelatedCols implements LogicalPlan interface.
Expand Down
15 changes: 5 additions & 10 deletions planner/core/rule_predicate_push_down.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,10 @@ func (p *LogicalSelection) PredicatePushDown(predicates []expression.Expression,
var child LogicalPlan
var retConditions []expression.Expression
var originConditions []expression.Expression
if p.buildByHaving {
retConditions, child = p.children[0].PredicatePushDown(predicates, opt)
retConditions = append(retConditions, p.Conditions...)
} else {
canBePushDown, canNotBePushDown := splitSetGetVarFunc(p.Conditions)
originConditions = canBePushDown
retConditions, child = p.children[0].PredicatePushDown(append(canBePushDown, predicates...), opt)
retConditions = append(retConditions, canNotBePushDown...)
}
canBePushDown, canNotBePushDown := splitSetGetVarFunc(p.Conditions)
originConditions = canBePushDown
retConditions, child = p.children[0].PredicatePushDown(append(canBePushDown, predicates...), opt)
retConditions = append(retConditions, canNotBePushDown...)
if len(retConditions) > 0 {
p.Conditions = expression.PropagateConstant(p.ctx, retConditions)
// Return table dual when filter is constant false or null.
Expand Down Expand Up @@ -696,7 +691,7 @@ func appendSelectionPredicatePushDownTraceStep(p *LogicalSelection, conditions [
reason := func() string {
return ""
}
if len(conditions) > 0 && !p.buildByHaving {
if len(conditions) > 0 {
reason = func() string {
buffer := bytes.NewBufferString("The conditions[")
for i, cond := range conditions {
Expand Down
12 changes: 6 additions & 6 deletions planner/core/testdata/ordered_result_mode_suite_out.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -417,12 +417,12 @@
},
{
"Plan": [
"Selection_8 6400.00 root lt(Column#6, 20)",
"└─Sort_9 8000.00 root Column#5, Column#6, Column#7",
" └─HashAgg_15 8000.00 root group by:test.t1.d, funcs:min(Column#11)->Column#5, funcs:max(Column#12)->Column#6, funcs:sum(Column#13)->Column#7",
" └─TableReader_16 8000.00 root data:HashAgg_11",
" └─HashAgg_11 8000.00 cop[tikv] group by:test.t1.d, funcs:min(test.t1.a)->Column#11, funcs:max(test.t1.b)->Column#12, funcs:sum(test.t1.c)->Column#13",
" └─TableFullScan_14 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo"
"Sort_9 6400.00 root Column#5, Column#6, Column#7",
"└─Selection_11 6400.00 root lt(Column#6, 20)",
" └─HashAgg_16 8000.00 root group by:test.t1.d, funcs:min(Column#11)->Column#5, funcs:max(Column#12)->Column#6, funcs:sum(Column#13)->Column#7",
" └─TableReader_17 8000.00 root data:HashAgg_12",
" └─HashAgg_12 8000.00 cop[tikv] group by:test.t1.d, funcs:min(test.t1.a)->Column#11, funcs:max(test.t1.b)->Column#12, funcs:sum(test.t1.c)->Column#13",
" └─TableFullScan_15 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo"
]
},
{
Expand Down
12 changes: 6 additions & 6 deletions planner/core/testdata/plan_suite_unexported_out.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"Name": "TestEagerAggregation",
"Cases": [
"DataScan(t)->Aggr(sum(test.t.a),sum(plus(test.t.a, 1)),count(test.t.a))->Projection",
"DataScan(t)->Aggr(sum(plus(test.t.a, test.t.b)),sum(plus(test.t.a, test.t.c)),count(test.t.a))->Projection->Sel([gt(Column#13, 0)])->Sort->Projection",
"DataScan(t)->Aggr(sum(plus(test.t.a, test.t.b)),sum(plus(test.t.a, test.t.c)),count(test.t.a))->Sel([gt(Column#13, 0)])->Projection->Sort->Projection",
"Join{DataScan(a)->Aggr(sum(test.t.a),firstrow(test.t.c))->DataScan(b)}(test.t.c,test.t.c)->Aggr(sum(Column#26))->Projection",
"Join{DataScan(a)->DataScan(b)->Aggr(sum(test.t.a),firstrow(test.t.c))}(test.t.c,test.t.c)->Aggr(sum(Column#26))->Projection",
"Join{DataScan(a)->DataScan(b)->Aggr(sum(test.t.a),firstrow(test.t.c))}(test.t.c,test.t.c)->Aggr(sum(Column#26),firstrow(test.t.a))->Projection",
Expand Down Expand Up @@ -89,7 +89,7 @@
"DataScan(t)->Aggr(sum(test.t.b),firstrow(test.t.a))->Sel([gt(cast(test.t.a, decimal(20,0) BINARY), Column#13)])->Projection->Projection",
"DataScan(t)->Aggr(sum(test.t.b),firstrow(test.t.a))->Sel([gt(test.t.a, 1)])->Projection->Projection",
"Dual->Sel([gt(test.t.a, 1)])->Projection",
"DataScan(t)->Aggr(count(test.t.a),firstrow(test.t.a))->Projection->Sel([lt(Column#13, 1)])",
"DataScan(t)->Aggr(count(test.t.a),firstrow(test.t.a))->Sel([lt(Column#13, 1)])->Projection",
"Join{DataScan(t1)->DataScan(t2)}(test.t.a,test.t.a)->Projection",
"Dual->Projection",
"DataScan(t)->Projection->Projection->Window(min(test.t.a)->Column#14)->Sel([lt(test.t.a, 10) eq(test.t.b, Column#14)])->Projection->Projection",
Expand Down Expand Up @@ -195,7 +195,7 @@
"TableReader(Table(t))->Window(sum(cast(test.t.a, decimal(10,0) BINARY))->Column#14 over())->Sort->Projection",
"TableReader(Table(t))->Window(sum(cast(test.t.a, decimal(10,0) BINARY))->Column#14 over(partition by test.t.a))->Sort->Projection",
"TableReader(Table(t)->StreamAgg)->StreamAgg->Window(sum(Column#13)->Column#15 over())->Sort->Projection",
"Apply{IndexReader(Index(t.f)[[NULL,+inf]])->IndexReader(Index(t.f)[[NULL,+inf]]->Sel([gt(test.t.a, test.t.a)]))->Window(sum(cast(test.t.a, decimal(10,0) BINARY))->Column#38 over())->MaxOneRow}->Sel([Column#38])->Projection",
"Apply{IndexReader(Index(t.f)[[NULL,+inf]])->IndexReader(Index(t.f)[[NULL,+inf]]->Sel([gt(test.t.a, test.t.a)]))->Window(sum(cast(test.t.a, decimal(10,0) BINARY))->Column#38 over())->MaxOneRow->Sel([Column#38])}->Projection",
"[planner:3594]You cannot use the alias 'w' of an expression containing a window function in this context.'",
"[planner:1247]Reference 'sum_a' not supported (reference to window function)",
"[planner:3579]Window name 'w2' is not defined.",
Expand Down Expand Up @@ -268,7 +268,7 @@
"TableReader(Table(t))->Window(sum(cast(test.t.a, decimal(10,0) BINARY))->Column#14 over())->Sort->Projection",
"TableReader(Table(t))->Window(sum(cast(test.t.a, decimal(10,0) BINARY))->Column#14 over(partition by test.t.a))->Sort->Projection",
"TableReader(Table(t)->StreamAgg)->StreamAgg->Window(sum(Column#13)->Column#15 over())->Sort->Projection",
"Apply{IndexReader(Index(t.f)[[NULL,+inf]])->IndexReader(Index(t.f)[[NULL,+inf]]->Sel([gt(test.t.a, test.t.a)]))->Window(sum(cast(test.t.a, decimal(10,0) BINARY))->Column#38 over())->MaxOneRow}->Sel([Column#38])->Projection",
"Apply{IndexReader(Index(t.f)[[NULL,+inf]])->IndexReader(Index(t.f)[[NULL,+inf]]->Sel([gt(test.t.a, test.t.a)]))->Window(sum(cast(test.t.a, decimal(10,0) BINARY))->Column#38 over())->MaxOneRow->Sel([Column#38])}->Projection",
"[planner:3594]You cannot use the alias 'w' of an expression containing a window function in this context.'",
"[planner:1247]Reference 'sum_a' not supported (reference to window function)",
"[planner:3579]Window name 'w2' is not defined.",
Expand Down Expand Up @@ -483,12 +483,12 @@
"test.t.f"
]
],
"4": [
"5": [
[
"test.t.f"
]
],
"5": [
"6": [
[
"test.t.f"
]
Expand Down
6 changes: 3 additions & 3 deletions statistics/testdata/stats_suite_out.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -706,12 +706,12 @@
{
"Start": 300,
"End": 899,
"Count": 4500
"Count": 4498.5
},
{
"Start": 800,
"End": 1000,
"Count": 1210.196869573942
"Count": 1201.196869573942
},
{
"Start": 900,
Expand All @@ -736,7 +736,7 @@
{
"Start": 200,
"End": 400,
"Count": 1230.0288209899081
"Count": 1211.5288209899081
},
{
"Start": 200,
Expand Down

0 comments on commit 47e4b5b

Please sign in to comment.