From c165b497879dcd146666374e241f610c8e2efcf6 Mon Sep 17 00:00:00 2001 From: Elsa <111482174+elsa0520@users.noreply.github.com> Date: Fri, 24 May 2024 12:29:48 +0800 Subject: [PATCH] planner: fix panic error when subquery + always true predicate in where clause (#53525) close pingcap/tidb#46962 --- .../casetest/physicalplantest/BUILD.bazel | 2 +- .../physicalplantest/physical_plan_test.go | 26 +++++++++++++ .../testdata/plan_suite_in.json | 8 ++++ .../testdata/plan_suite_out.json | 38 +++++++++++++++++++ pkg/planner/core/rule_build_key_info.go | 6 ++- 5 files changed, 78 insertions(+), 2 deletions(-) diff --git a/pkg/planner/core/casetest/physicalplantest/BUILD.bazel b/pkg/planner/core/casetest/physicalplantest/BUILD.bazel index 1b534a147b6a0..54f2d86948766 100644 --- a/pkg/planner/core/casetest/physicalplantest/BUILD.bazel +++ b/pkg/planner/core/casetest/physicalplantest/BUILD.bazel @@ -10,7 +10,7 @@ go_test( data = glob(["testdata/**"]), flaky = True, race = "on", - shard_count = 32, + shard_count = 33, deps = [ "//pkg/config", "//pkg/domain", diff --git a/pkg/planner/core/casetest/physicalplantest/physical_plan_test.go b/pkg/planner/core/casetest/physicalplantest/physical_plan_test.go index ee0fac0a8ade1..ab35ef9f19df4 100644 --- a/pkg/planner/core/casetest/physicalplantest/physical_plan_test.go +++ b/pkg/planner/core/casetest/physicalplantest/physical_plan_test.go @@ -1481,3 +1481,29 @@ func TestPointgetIndexChoosen(t *testing.T) { tk.MustQuery("explain format = 'brief' " + ts).Check(testkit.Rows(output[i].Plan...)) } } + +// Test issue #46962 plan +func TestAlwaysTruePredicateWithSubquery(t *testing.T) { + var ( + input []string + output []struct { + SQL string + Plan []string + Warning []string + } + ) + planSuiteData := GetPlanSuiteData() + planSuiteData.LoadTestCases(t, &input, &output) + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + + tk.MustExec("use test") + tk.MustExec(`CREATE TABLE t ( a int NOT NULL , b int NOT NULL ) `) + for i, ts := range input { + testdata.OnRecord(func() { + output[i].SQL = ts + output[i].Plan = testdata.ConvertRowsToStrings(tk.MustQuery(ts).Rows()) + }) + tk.MustQuery(ts).Check(testkit.Rows(output[i].Plan...)) + } +} diff --git a/pkg/planner/core/casetest/physicalplantest/testdata/plan_suite_in.json b/pkg/planner/core/casetest/physicalplantest/testdata/plan_suite_in.json index e37113c7afe3d..e036e59219ebb 100644 --- a/pkg/planner/core/casetest/physicalplantest/testdata/plan_suite_in.json +++ b/pkg/planner/core/casetest/physicalplantest/testdata/plan_suite_in.json @@ -611,5 +611,13 @@ "select * from t where b=1 and c='1' and d='1';", "select * from t where b in (1,2,3) and c in ('1');" ] + }, + { + "name": "TestAlwaysTruePredicateWithSubquery", + "cases" : [ + "SHOW ERRORS WHERE TRUE = ALL ( SELECT TRUE GROUP BY 1 LIMIT 1 ) IS NULL IS NOT NULL;", + "explain select * from t WHERE TRUE = ALL ( SELECT TRUE GROUP BY 1 LIMIT 1 ) IS NULL IS NOT NULL;", + "explain select * from t WHERE TRUE = ALL ( SELECT TRUE from t GROUP BY 1 LIMIT 1 ) is null is not null;" + ] } ] diff --git a/pkg/planner/core/casetest/physicalplantest/testdata/plan_suite_out.json b/pkg/planner/core/casetest/physicalplantest/testdata/plan_suite_out.json index 05c75cbb2b649..ed7a34e1757b6 100644 --- a/pkg/planner/core/casetest/physicalplantest/testdata/plan_suite_out.json +++ b/pkg/planner/core/casetest/physicalplantest/testdata/plan_suite_out.json @@ -3746,5 +3746,43 @@ "Warning": null } ] + }, + { + "Name": "TestAlwaysTruePredicateWithSubquery", + "Cases": [ + { + "SQL": "SHOW ERRORS WHERE TRUE = ALL ( SELECT TRUE GROUP BY 1 LIMIT 1 ) IS NULL IS NOT NULL;", + "Plan": null, + "Warning": null + }, + { + "SQL": "explain select * from t WHERE TRUE = ALL ( SELECT TRUE GROUP BY 1 LIMIT 1 ) IS NULL IS NOT NULL;", + "Plan": [ + "HashJoin_14 10000.00 root CARTESIAN inner join", + "├─StreamAgg_19(Build) 1.00 root funcs:count(1)->Column#13", + "│ └─Limit_22 1.00 root offset:0, count:1", + "│ └─HashAgg_23 1.00 root group by:1, ", + "│ └─TableDual_24 1.00 root rows:1", + "└─TableReader_17(Probe) 10000.00 root data:TableFullScan_16", + " └─TableFullScan_16 10000.00 cop[tikv] table:t keep order:false, stats:pseudo" + ], + "Warning": null + }, + { + "SQL": "explain select * from t WHERE TRUE = ALL ( SELECT TRUE from t GROUP BY 1 LIMIT 1 ) is null is not null;", + "Plan": [ + "HashJoin_14 10000.00 root CARTESIAN inner join", + "├─StreamAgg_19(Build) 1.00 root funcs:count(1)->Column#16", + "│ └─Limit_22 1.00 root offset:0, count:1", + "│ └─HashAgg_27 1.00 root group by:Column#17, funcs:firstrow(Column#18)->test.t.a, funcs:firstrow(Column#19)->test.t.b, funcs:firstrow(Column#20)->test.t._tidb_rowid", + "│ └─TableReader_28 1.00 root data:HashAgg_23", + "│ └─HashAgg_23 1.00 cop[tikv] group by:1, funcs:firstrow(test.t.a)->Column#18, funcs:firstrow(test.t.b)->Column#19, funcs:firstrow(test.t._tidb_rowid)->Column#20", + "│ └─TableFullScan_26 10000.00 cop[tikv] table:t keep order:false, stats:pseudo", + "└─TableReader_17(Probe) 10000.00 root data:TableFullScan_16", + " └─TableFullScan_16 10000.00 cop[tikv] table:t keep order:false, stats:pseudo" + ], + "Warning": null + } + ] } ] diff --git a/pkg/planner/core/rule_build_key_info.go b/pkg/planner/core/rule_build_key_info.go index acc4d9ba725b5..56a82548a81a6 100644 --- a/pkg/planner/core/rule_build_key_info.go +++ b/pkg/planner/core/rule_build_key_info.go @@ -47,7 +47,11 @@ func buildKeyInfo(lp base.LogicalPlan) { // BuildKeyInfo implements base.LogicalPlan BuildKeyInfo interface. func (la *LogicalAggregation) BuildKeyInfo(selfSchema *expression.Schema, childSchema []*expression.Schema) { - if la.IsPartialModeAgg() { + // According to the issue#46962, we can ignore the judgment of partial agg + // Sometimes, the agg inside of subquery and there is a true condition in where clause, the agg function is empty. + // For example, ``` select xxxx from xxx WHERE TRUE = ALL ( SELECT TRUE GROUP BY 1 LIMIT 1 ) IS NULL IS NOT NULL; + // In this case, the agg is complete mode and we can ignore this check. + if len(la.AggFuncs) != 0 && la.IsPartialModeAgg() { return } la.logicalSchemaProducer.BuildKeyInfo(selfSchema, childSchema)