From 47e4b5bf312aa9219f2589145c4e145590e0785f Mon Sep 17 00:00:00 2001 From: wjHuang Date: Thu, 17 Mar 2022 10:43:53 +0800 Subject: [PATCH] *: revert #27021 to fix a bug that selection can not be pushed down when having clause above aggregation (#33168) close pingcap/tidb#33166 --- executor/aggregate_test.go | 12 ------------ planner/core/logical_plan_builder.go | 2 +- planner/core/logical_plans.go | 3 --- planner/core/rule_predicate_push_down.go | 15 +++++---------- .../testdata/ordered_result_mode_suite_out.json | 12 ++++++------ .../core/testdata/plan_suite_unexported_out.json | 12 ++++++------ statistics/testdata/stats_suite_out.json | 6 +++--- 7 files changed, 21 insertions(+), 41 deletions(-) diff --git a/executor/aggregate_test.go b/executor/aggregate_test.go index 9be71b793ee3f..922bfa72fdc00 100644 --- a/executor/aggregate_test.go +++ b/executor/aggregate_test.go @@ -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() diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index 60b0d4bec0655..c8e38780eecba 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -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 { diff --git a/planner/core/logical_plans.go b/planner/core/logical_plans.go index 6df233ccdbac7..a5cedfe83c610 100644 --- a/planner/core/logical_plans.go +++ b/planner/core/logical_plans.go @@ -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. diff --git a/planner/core/rule_predicate_push_down.go b/planner/core/rule_predicate_push_down.go index 9896b844f74f8..fc7154d9063d2 100644 --- a/planner/core/rule_predicate_push_down.go +++ b/planner/core/rule_predicate_push_down.go @@ -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. @@ -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 { diff --git a/planner/core/testdata/ordered_result_mode_suite_out.json b/planner/core/testdata/ordered_result_mode_suite_out.json index 8016c33bed6f4..2047c8542e776 100644 --- a/planner/core/testdata/ordered_result_mode_suite_out.json +++ b/planner/core/testdata/ordered_result_mode_suite_out.json @@ -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" ] }, { diff --git a/planner/core/testdata/plan_suite_unexported_out.json b/planner/core/testdata/plan_suite_unexported_out.json index 44b29b2594347..218880d0aa713 100644 --- a/planner/core/testdata/plan_suite_unexported_out.json +++ b/planner/core/testdata/plan_suite_unexported_out.json @@ -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", @@ -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", @@ -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.", @@ -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.", @@ -483,12 +483,12 @@ "test.t.f" ] ], - "4": [ + "5": [ [ "test.t.f" ] ], - "5": [ + "6": [ [ "test.t.f" ] diff --git a/statistics/testdata/stats_suite_out.json b/statistics/testdata/stats_suite_out.json index df4c8498d845c..d07b336ccbfbd 100644 --- a/statistics/testdata/stats_suite_out.json +++ b/statistics/testdata/stats_suite_out.json @@ -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, @@ -736,7 +736,7 @@ { "Start": 200, "End": 400, - "Count": 1230.0288209899081 + "Count": 1211.5288209899081 }, { "Start": 200,