From 3f5ab5fd4d52c8c7826b8f4f3c7e0104bcfbd8dd Mon Sep 17 00:00:00 2001 From: Weizhen Wang <wangweizhen@pingcap.com> Date: Wed, 17 Jul 2024 12:45:00 +0800 Subject: [PATCH 1/4] This is an automated cherry-pick of #54450 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> --- pkg/executor/cte_test.go | 50 ++++++++++++++++++++++++++++++ pkg/executor/test/cte/BUILD.bazel | 22 +++++++++++++ pkg/planner/core/find_best_task.go | 2 +- pkg/server/main_test.go | 5 +++ 4 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 pkg/executor/test/cte/BUILD.bazel diff --git a/pkg/executor/cte_test.go b/pkg/executor/cte_test.go index 68d6c322e4f22..6550b7cd96999 100644 --- a/pkg/executor/cte_test.go +++ b/pkg/executor/cte_test.go @@ -226,3 +226,53 @@ func TestCTEIterationMemTracker(t *testing.T) { }() tk.MustQuery(fmt.Sprintf("explain analyze with recursive cte1 as (select c1 from t1 union all select c1 + 1 c1 from cte1 where c1 < %d) select * from cte1", maxIter)) } + +func TestCTETableInvaildTask(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("CREATE TABLE p ( groupid bigint(20) DEFAULT NULL, KEY k1 (groupid));") + tk.MustExec(`CREATE TABLE g (groupid bigint(20) DEFAULT NULL,parentid bigint(20) NOT NULL,KEY k1 (parentid),KEY k2 (groupid,parentid));`) + tk.MustExec(`set tidb_opt_enable_hash_join=off;`) + tk.MustQuery(`explain WITH RECURSIVE w(gid) AS ( + SELECT + groupId + FROM + p + UNION + SELECT + g.groupId + FROM + g + JOIN w ON g.parentId = w.gid +) +SELECT + 1 +FROM + g +WHERE + g.groupId IN ( + SELECT + gid + FROM + w + );`).Check(testkit.Rows( + "Projection_54 9990.00 root 1->Column#17", + "└─IndexJoin_59 9990.00 root inner join, inner:IndexReader_58, outer key:test.p.groupid, inner key:test.g.groupid, equal cond:eq(test.p.groupid, test.g.groupid)", + " ├─HashAgg_75(Build) 12800.00 root group by:test.p.groupid, funcs:firstrow(test.p.groupid)->test.p.groupid", + " │ └─Selection_72 12800.00 root not(isnull(test.p.groupid))", + " │ └─CTEFullScan_73 16000.00 root CTE:w data:CTE_0", + " └─IndexReader_58(Probe) 9990.00 root index:Selection_57", + " └─Selection_57 9990.00 cop[tikv] not(isnull(test.g.groupid))", + " └─IndexRangeScan_56 10000.00 cop[tikv] table:g, index:k2(groupid, parentid) range: decided by [eq(test.g.groupid, test.p.groupid)], keep order:false, stats:pseudo", + "CTE_0 16000.00 root Recursive CTE", + "├─IndexReader_24(Seed Part) 10000.00 root index:IndexFullScan_23", + "│ └─IndexFullScan_23 10000.00 cop[tikv] table:p, index:k1(groupid) keep order:false, stats:pseudo", + "└─IndexHashJoin_34(Recursive Part) 10000.00 root inner join, inner:IndexLookUp_31, outer key:test.p.groupid, inner key:test.g.parentid, equal cond:eq(test.p.groupid, test.g.parentid)", + " ├─Selection_51(Build) 8000.00 root not(isnull(test.p.groupid))", + " │ └─CTETable_52 10000.00 root Scan on CTE_0", + " └─IndexLookUp_31(Probe) 10000.00 root ", + " ├─IndexRangeScan_29(Build) 10000.00 cop[tikv] table:g, index:k1(parentid) range: decided by [eq(test.g.parentid, test.p.groupid)], keep order:false, stats:pseudo", + " └─TableRowIDScan_30(Probe) 10000.00 cop[tikv] table:g keep order:false, stats:pseudo")) + tk.MustQuery(`show warnings`).Check(testkit.Rows()) +} diff --git a/pkg/executor/test/cte/BUILD.bazel b/pkg/executor/test/cte/BUILD.bazel new file mode 100644 index 0000000000000..5b7b3f39ea0c4 --- /dev/null +++ b/pkg/executor/test/cte/BUILD.bazel @@ -0,0 +1,22 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_test") + +go_test( + name = "cte_test", + timeout = "short", + srcs = [ + "cte_test.go", + "main_test.go", + ], + flaky = True, + shard_count = 8, + deps = [ + "//pkg/config", + "//pkg/parser/terror", + "//pkg/testkit", + "//pkg/testkit/testsetup", + "//pkg/types", + "@com_github_pingcap_failpoint//:failpoint", + "@com_github_stretchr_testify//require", + "@org_uber_go_goleak//:goleak", + ], +) diff --git a/pkg/planner/core/find_best_task.go b/pkg/planner/core/find_best_task.go index b3fc8a283d845..f33c8a8ad4cd4 100644 --- a/pkg/planner/core/find_best_task.go +++ b/pkg/planner/core/find_best_task.go @@ -2873,7 +2873,7 @@ func (p *LogicalCTE) findBestTask(prop *property.PhysicalProperty, counter *Plan func (p *LogicalCTETable) findBestTask(prop *property.PhysicalProperty, _ *PlanCounterTp, _ *physicalOptimizeOp) (t task, cntPlan int64, err error) { if !prop.IsSortItemEmpty() { - return nil, 1, nil + return base.InvalidTask, 0, nil } pcteTable := PhysicalCTETable{IDForStorage: p.idForStorage}.Init(p.SCtx(), p.StatsInfo()) diff --git a/pkg/server/main_test.go b/pkg/server/main_test.go index 68958c93b51af..ddc4e2e5a83a2 100644 --- a/pkg/server/main_test.go +++ b/pkg/server/main_test.go @@ -62,6 +62,11 @@ func TestMain(m *testing.M) { testDataMap.LoadTestSuiteData("testdata", "optimizer_suite") opts := []goleak.Option{ +<<<<<<< HEAD +======= + goleak.IgnoreTopFunction("github.com/dgraph-io/ristretto.(*defaultPolicy).processItems"), + goleak.IgnoreTopFunction("github.com/dgraph-io/ristretto.(*Cache).processItems"), +>>>>>>> 156f23aacb9 (planer: fix invalid pointer caused by a recursive CTE query (#54450)) goleak.IgnoreTopFunction("github.com/golang/glog.(*fileSink).flushDaemon"), goleak.IgnoreTopFunction("github.com/bazelbuild/rules_go/go/tools/bzltestutil.RegisterTimeoutHandler.func1"), goleak.IgnoreTopFunction("github.com/lestrrat-go/httprc.runFetchWorker"), From d7eeae4422d97d2627cc5c73064f7da07d7d10df Mon Sep 17 00:00:00 2001 From: Weizhen Wang <wangweizhen@pingcap.com> Date: Fri, 19 Jul 2024 15:07:52 +0800 Subject: [PATCH 2/4] update Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com> --- pkg/server/main_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/server/main_test.go b/pkg/server/main_test.go index ddc4e2e5a83a2..68958c93b51af 100644 --- a/pkg/server/main_test.go +++ b/pkg/server/main_test.go @@ -62,11 +62,6 @@ func TestMain(m *testing.M) { testDataMap.LoadTestSuiteData("testdata", "optimizer_suite") opts := []goleak.Option{ -<<<<<<< HEAD -======= - goleak.IgnoreTopFunction("github.com/dgraph-io/ristretto.(*defaultPolicy).processItems"), - goleak.IgnoreTopFunction("github.com/dgraph-io/ristretto.(*Cache).processItems"), ->>>>>>> 156f23aacb9 (planer: fix invalid pointer caused by a recursive CTE query (#54450)) goleak.IgnoreTopFunction("github.com/golang/glog.(*fileSink).flushDaemon"), goleak.IgnoreTopFunction("github.com/bazelbuild/rules_go/go/tools/bzltestutil.RegisterTimeoutHandler.func1"), goleak.IgnoreTopFunction("github.com/lestrrat-go/httprc.runFetchWorker"), From 7536ff24077df189fad1edc71d10cf7de1b6078e Mon Sep 17 00:00:00 2001 From: Weizhen Wang <wangweizhen@pingcap.com> Date: Fri, 19 Jul 2024 15:08:25 +0800 Subject: [PATCH 3/4] update Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com> --- pkg/executor/test/cte/BUILD.bazel | 22 ---------------------- 1 file changed, 22 deletions(-) delete mode 100644 pkg/executor/test/cte/BUILD.bazel diff --git a/pkg/executor/test/cte/BUILD.bazel b/pkg/executor/test/cte/BUILD.bazel deleted file mode 100644 index 5b7b3f39ea0c4..0000000000000 --- a/pkg/executor/test/cte/BUILD.bazel +++ /dev/null @@ -1,22 +0,0 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_test") - -go_test( - name = "cte_test", - timeout = "short", - srcs = [ - "cte_test.go", - "main_test.go", - ], - flaky = True, - shard_count = 8, - deps = [ - "//pkg/config", - "//pkg/parser/terror", - "//pkg/testkit", - "//pkg/testkit/testsetup", - "//pkg/types", - "@com_github_pingcap_failpoint//:failpoint", - "@com_github_stretchr_testify//require", - "@org_uber_go_goleak//:goleak", - ], -) From 0e4c68ab4aa1e8f6a1f81d9c49b0315009b64cd0 Mon Sep 17 00:00:00 2001 From: Weizhen Wang <wangweizhen@pingcap.com> Date: Fri, 19 Jul 2024 16:47:19 +0800 Subject: [PATCH 4/4] update Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com> --- pkg/planner/core/find_best_task.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/planner/core/find_best_task.go b/pkg/planner/core/find_best_task.go index f33c8a8ad4cd4..713398ee61695 100644 --- a/pkg/planner/core/find_best_task.go +++ b/pkg/planner/core/find_best_task.go @@ -2873,7 +2873,7 @@ func (p *LogicalCTE) findBestTask(prop *property.PhysicalProperty, counter *Plan func (p *LogicalCTETable) findBestTask(prop *property.PhysicalProperty, _ *PlanCounterTp, _ *physicalOptimizeOp) (t task, cntPlan int64, err error) { if !prop.IsSortItemEmpty() { - return base.InvalidTask, 0, nil + return invalidTask, 0, nil } pcteTable := PhysicalCTETable{IDForStorage: p.idForStorage}.Init(p.SCtx(), p.StatsInfo())