From 9cfc4e337f12a56835239b5c4f18f1f8846d30a9 Mon Sep 17 00:00:00 2001 From: Rustin Liu Date: Tue, 17 Oct 2023 22:09:29 -0500 Subject: [PATCH 1/2] This is an automated cherry-pick of #47454 Signed-off-by: ti-chi-bot --- parser/ast/expressions.go | 2 +- planner/core/plan_test.go | 63 ++++++++++++++++++++++++++++++++++ planner/core/point_get_plan.go | 59 ++++++++++++++++++++++++++++--- 3 files changed, 118 insertions(+), 6 deletions(-) diff --git a/parser/ast/expressions.go b/parser/ast/expressions.go index 66f40eb205952..70345d91ef4ef 100644 --- a/parser/ast/expressions.go +++ b/parser/ast/expressions.go @@ -960,7 +960,7 @@ type ParamMarkerExpr interface { SetOrder(int) } -// ParenthesesExpr is the parentheses expression. +// ParenthesesExpr is the parentheses' expression. type ParenthesesExpr struct { exprNode // Expr is the expression in parentheses. diff --git a/planner/core/plan_test.go b/planner/core/plan_test.go index 81a1d58ebd8e1..72f6514d69917 100644 --- a/planner/core/plan_test.go +++ b/planner/core/plan_test.go @@ -874,10 +874,73 @@ func TestIssue40857(t *testing.T) { require.Empty(t, tk.Session().LastMessage()) } +<<<<<<< HEAD:planner/core/plan_test.go // https://github.com/pingcap/tidb/issues/35527. func TestTableDualAsSubQuery(t *testing.T) { store, clean := testkit.CreateMockStore(t) defer clean() +======= +func TestCloneFineGrainedShuffleStreamCount(t *testing.T) { + window := &core.PhysicalWindow{} + newPlan, err := window.Clone() + require.NoError(t, err) + newWindow, ok := newPlan.(*core.PhysicalWindow) + require.Equal(t, ok, true) + require.Equal(t, window.TiFlashFineGrainedShuffleStreamCount, newWindow.TiFlashFineGrainedShuffleStreamCount) + + window.TiFlashFineGrainedShuffleStreamCount = 8 + newPlan, err = window.Clone() + require.NoError(t, err) + newWindow, ok = newPlan.(*core.PhysicalWindow) + require.Equal(t, ok, true) + require.Equal(t, window.TiFlashFineGrainedShuffleStreamCount, newWindow.TiFlashFineGrainedShuffleStreamCount) + + sort := &core.PhysicalSort{} + newPlan, err = sort.Clone() + require.NoError(t, err) + newSort, ok := newPlan.(*core.PhysicalSort) + require.Equal(t, ok, true) + require.Equal(t, sort.TiFlashFineGrainedShuffleStreamCount, newSort.TiFlashFineGrainedShuffleStreamCount) + + sort.TiFlashFineGrainedShuffleStreamCount = 8 + newPlan, err = sort.Clone() + require.NoError(t, err) + newSort, ok = newPlan.(*core.PhysicalSort) + require.Equal(t, ok, true) + require.Equal(t, sort.TiFlashFineGrainedShuffleStreamCount, newSort.TiFlashFineGrainedShuffleStreamCount) +} + +func TestIssue40535(t *testing.T) { + store := testkit.CreateMockStore(t) + var cfg kv.InjectionConfig + tk := testkit.NewTestKit(t, kv.NewInjectedStore(store, &cfg)) + tk.MustExec("use test;") + tk.MustExec("drop table if exists t1; drop table if exists t2;") + tk.MustExec("CREATE TABLE `t1`(`c1` bigint(20) NOT NULL DEFAULT '-2312745469307452950', `c2` datetime DEFAULT '5316-02-03 06:54:49', `c3` tinyblob DEFAULT NULL, PRIMARY KEY (`c1`) /*T![clustered_index] CLUSTERED */) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin;") + tk.MustExec("CREATE TABLE `t2`(`c1` set('kn8pu','7et','vekx6','v3','liwrh','q14','1met','nnd5i','5o0','8cz','l') DEFAULT '7et,vekx6,liwrh,q14,1met', `c2` float DEFAULT '1.683167', KEY `k1` (`c2`,`c1`), KEY `k2` (`c2`)) ENGINE=InnoDB DEFAULT CHARSET=gbk COLLATE=gbk_chinese_ci;") + tk.MustExec("(select /*+ agg_to_cop()*/ locate(t1.c3, t1.c3) as r0, t1.c3 as r1 from t1 where not( IsNull(t1.c1)) order by r0,r1) union all (select concat_ws(',', t2.c2, t2.c1) as r0, t2.c1 as r1 from t2 order by r0, r1) order by 1 limit 273;") + require.Empty(t, tk.Session().LastMessage()) +} + +func TestIssue47445(t *testing.T) { + store, _ := testkit.CreateMockStoreAndDomain(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test;") + tk.MustExec("CREATE TABLE golang1 ( `fcbpdt` CHAR (8) COLLATE utf8_general_ci NOT NULL, `fcbpsq` VARCHAR (20) COLLATE utf8_general_ci NOT NULL, `procst` char (4) COLLATE utf8_general_ci DEFAULT NULL,`cipstx` VARCHAR (105) COLLATE utf8_general_ci DEFAULT NULL, `cipsst` CHAR (4) COLLATE utf8_general_ci DEFAULT NULL, `dyngtg` VARCHAR(4) COLLATE utf8_general_ci DEFAULT NULL, `blncdt` VARCHAR (8) COLLATE utf8_general_ci DEFAULT NULL, PRIMARY KEY ( fcbpdt, fcbpsq ))") + tk.MustExec("insert into golang1 values('20230925','12023092502158016','abc','','','','')") + tk.MustExec("create table golang2 (`sysgrp` varchar(20) NOT NULL,`procst` varchar(8) NOT NULL,`levlid` int(11) NOT NULL,PRIMARY key (procst));") + tk.MustExec("insert into golang2 VALUES('COMMON','ACSC',90)") + tk.MustExec("insert into golang2 VALUES('COMMON','abc',8)") + tk.MustExec("insert into golang2 VALUES('COMMON','CH02',6)") + tk.MustExec("UPDATE golang1 a SET procst =(CASE WHEN ( SELECT levlid FROM golang2 b WHERE b.sysgrp = 'COMMON' AND b.procst = 'ACSC' ) > ( SELECT levlid FROM golang2 c WHERE c.sysgrp = 'COMMON' AND c.procst = a.procst ) THEN 'ACSC' ELSE a.procst END ), cipstx = 'CI010000', cipsst = 'ACSC', dyngtg = 'EAYT', blncdt= '20230925' WHERE fcbpdt = '20230925' AND fcbpsq = '12023092502158016'") + tk.MustQuery("select * from golang1").Check(testkit.Rows("20230925 12023092502158016 ACSC CI010000 ACSC EAYT 20230925")) + tk.MustExec("UPDATE golang1 a SET procst= (SELECT 1 FROM golang2 c WHERE c.procst = a.procst) WHERE fcbpdt = '20230925' AND fcbpsq = '12023092502158016'") + tk.MustQuery("select * from golang1").Check(testkit.Rows("20230925 12023092502158016 1 CI010000 ACSC EAYT 20230925")) +} + +func TestExplainValuesStatement(t *testing.T) { + store, _ := testkit.CreateMockStoreAndDomain(t) +>>>>>>> 1c185556710 (planner: do not convert update to point get if the expr has sub-query (#47454)):pkg/planner/core/plan_test.go tk := testkit.NewTestKit(t, store) tk.MustExec("use test") tk.MustExec("CREATE VIEW v0(c0) AS SELECT NULL;") diff --git a/planner/core/point_get_plan.go b/planner/core/point_get_plan.go index 5d309606afc06..e05bd9d6c3dd9 100644 --- a/planner/core/point_get_plan.go +++ b/planner/core/point_get_plan.go @@ -19,6 +19,11 @@ import ( "sort" "strconv" "strings" +<<<<<<< HEAD:planner/core/point_get_plan.go +======= + "sync" + "unsafe" +>>>>>>> 1c185556710 (planner: do not convert update to point get if the expr has sub-query (#47454)):pkg/planner/core/point_get_plan.go "github.com/pingcap/errors" "github.com/pingcap/tidb/config" @@ -1365,13 +1370,57 @@ func findInPairs(colName string, pairs []nameValuePair) int { return -1 } -func tryUpdatePointPlan(ctx sessionctx.Context, updateStmt *ast.UpdateStmt) Plan { - // avoid using the point_get when assignment_list contains the subquery in the UPDATE. - for _, list := range updateStmt.List { - if _, ok := list.Expr.(*ast.SubqueryExpr); ok { - return nil +// Use cache to avoid allocating memory every time. +var subQueryCheckerPool = &sync.Pool{New: func() any { return &subQueryChecker{} }} + +type subQueryChecker struct { + hasSubQuery bool +} + +func (s *subQueryChecker) Enter(in ast.Node) (node ast.Node, skipChildren bool) { + if s.hasSubQuery { + return in, true + } + + if _, ok := in.(*ast.SubqueryExpr); ok { + s.hasSubQuery = true + return in, true + } + + return in, false +} + +func (s *subQueryChecker) Leave(in ast.Node) (ast.Node, bool) { + // Before we enter the sub-query, we should keep visiting its children. + return in, !s.hasSubQuery +} + +func isExprHasSubQuery(expr ast.Node) bool { + checker := subQueryCheckerPool.Get().(*subQueryChecker) + defer func() { + // Do not forget to reset the flag. + checker.hasSubQuery = false + subQueryCheckerPool.Put(checker) + }() + expr.Accept(checker) + return checker.hasSubQuery +} + +func checkIfAssignmentListHasSubQuery(list []*ast.Assignment) bool { + for _, a := range list { + if isExprHasSubQuery(a) { + return true } } + return false +} + +func tryUpdatePointPlan(ctx sessionctx.Context, updateStmt *ast.UpdateStmt) Plan { + // Avoid using the point_get when assignment_list contains the sub-query in the UPDATE. + if checkIfAssignmentListHasSubQuery(updateStmt.List) { + return nil + } + selStmt := &ast.SelectStmt{ Fields: &ast.FieldList{}, From: updateStmt.TableRefs, From 9ab17bcb777a31292c9e38d8cebe5513f80bd75a Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Fri, 26 Jan 2024 17:01:11 +0800 Subject: [PATCH 2/2] fix Signed-off-by: hi-rustin --- planner/core/plan_test.go | 61 ++++------------------------------ planner/core/point_get_plan.go | 4 --- 2 files changed, 7 insertions(+), 58 deletions(-) diff --git a/planner/core/plan_test.go b/planner/core/plan_test.go index 72f6514d69917..fca7a77bf67f9 100644 --- a/planner/core/plan_test.go +++ b/planner/core/plan_test.go @@ -874,56 +874,19 @@ func TestIssue40857(t *testing.T) { require.Empty(t, tk.Session().LastMessage()) } -<<<<<<< HEAD:planner/core/plan_test.go // https://github.com/pingcap/tidb/issues/35527. func TestTableDualAsSubQuery(t *testing.T) { store, clean := testkit.CreateMockStore(t) defer clean() -======= -func TestCloneFineGrainedShuffleStreamCount(t *testing.T) { - window := &core.PhysicalWindow{} - newPlan, err := window.Clone() - require.NoError(t, err) - newWindow, ok := newPlan.(*core.PhysicalWindow) - require.Equal(t, ok, true) - require.Equal(t, window.TiFlashFineGrainedShuffleStreamCount, newWindow.TiFlashFineGrainedShuffleStreamCount) - - window.TiFlashFineGrainedShuffleStreamCount = 8 - newPlan, err = window.Clone() - require.NoError(t, err) - newWindow, ok = newPlan.(*core.PhysicalWindow) - require.Equal(t, ok, true) - require.Equal(t, window.TiFlashFineGrainedShuffleStreamCount, newWindow.TiFlashFineGrainedShuffleStreamCount) - - sort := &core.PhysicalSort{} - newPlan, err = sort.Clone() - require.NoError(t, err) - newSort, ok := newPlan.(*core.PhysicalSort) - require.Equal(t, ok, true) - require.Equal(t, sort.TiFlashFineGrainedShuffleStreamCount, newSort.TiFlashFineGrainedShuffleStreamCount) - - sort.TiFlashFineGrainedShuffleStreamCount = 8 - newPlan, err = sort.Clone() - require.NoError(t, err) - newSort, ok = newPlan.(*core.PhysicalSort) - require.Equal(t, ok, true) - require.Equal(t, sort.TiFlashFineGrainedShuffleStreamCount, newSort.TiFlashFineGrainedShuffleStreamCount) -} - -func TestIssue40535(t *testing.T) { - store := testkit.CreateMockStore(t) - var cfg kv.InjectionConfig - tk := testkit.NewTestKit(t, kv.NewInjectedStore(store, &cfg)) - tk.MustExec("use test;") - tk.MustExec("drop table if exists t1; drop table if exists t2;") - tk.MustExec("CREATE TABLE `t1`(`c1` bigint(20) NOT NULL DEFAULT '-2312745469307452950', `c2` datetime DEFAULT '5316-02-03 06:54:49', `c3` tinyblob DEFAULT NULL, PRIMARY KEY (`c1`) /*T![clustered_index] CLUSTERED */) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin;") - tk.MustExec("CREATE TABLE `t2`(`c1` set('kn8pu','7et','vekx6','v3','liwrh','q14','1met','nnd5i','5o0','8cz','l') DEFAULT '7et,vekx6,liwrh,q14,1met', `c2` float DEFAULT '1.683167', KEY `k1` (`c2`,`c1`), KEY `k2` (`c2`)) ENGINE=InnoDB DEFAULT CHARSET=gbk COLLATE=gbk_chinese_ci;") - tk.MustExec("(select /*+ agg_to_cop()*/ locate(t1.c3, t1.c3) as r0, t1.c3 as r1 from t1 where not( IsNull(t1.c1)) order by r0,r1) union all (select concat_ws(',', t2.c2, t2.c1) as r0, t2.c1 as r1 from t2 order by r0, r1) order by 1 limit 273;") - require.Empty(t, tk.Session().LastMessage()) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("CREATE VIEW v0(c0) AS SELECT NULL;") + tk.MustQuery("SELECT v0.c0 FROM v0 WHERE (v0.c0 IS NULL) LIKE(NULL);").Check(testkit.Rows()) + tk.MustQuery("SELECT v0.c0 FROM (SELECT null as c0) v0 WHERE (v0.c0 IS NULL) like (NULL);").Check(testkit.Rows()) } - func TestIssue47445(t *testing.T) { - store, _ := testkit.CreateMockStoreAndDomain(t) + store, clean := testkit.CreateMockStore(t) + defer clean() tk := testkit.NewTestKit(t, store) tk.MustExec("use test;") tk.MustExec("CREATE TABLE golang1 ( `fcbpdt` CHAR (8) COLLATE utf8_general_ci NOT NULL, `fcbpsq` VARCHAR (20) COLLATE utf8_general_ci NOT NULL, `procst` char (4) COLLATE utf8_general_ci DEFAULT NULL,`cipstx` VARCHAR (105) COLLATE utf8_general_ci DEFAULT NULL, `cipsst` CHAR (4) COLLATE utf8_general_ci DEFAULT NULL, `dyngtg` VARCHAR(4) COLLATE utf8_general_ci DEFAULT NULL, `blncdt` VARCHAR (8) COLLATE utf8_general_ci DEFAULT NULL, PRIMARY KEY ( fcbpdt, fcbpsq ))") @@ -938,16 +901,6 @@ func TestIssue47445(t *testing.T) { tk.MustQuery("select * from golang1").Check(testkit.Rows("20230925 12023092502158016 1 CI010000 ACSC EAYT 20230925")) } -func TestExplainValuesStatement(t *testing.T) { - store, _ := testkit.CreateMockStoreAndDomain(t) ->>>>>>> 1c185556710 (planner: do not convert update to point get if the expr has sub-query (#47454)):pkg/planner/core/plan_test.go - tk := testkit.NewTestKit(t, store) - tk.MustExec("use test") - tk.MustExec("CREATE VIEW v0(c0) AS SELECT NULL;") - tk.MustQuery("SELECT v0.c0 FROM v0 WHERE (v0.c0 IS NULL) LIKE(NULL);").Check(testkit.Rows()) - tk.MustQuery("SELECT v0.c0 FROM (SELECT null as c0) v0 WHERE (v0.c0 IS NULL) like (NULL);").Check(testkit.Rows()) -} - // https://github.com/pingcap/tidb/issues/38310 func TestNullEQConditionPlan(t *testing.T) { store, clean := testkit.CreateMockStore(t) diff --git a/planner/core/point_get_plan.go b/planner/core/point_get_plan.go index e05bd9d6c3dd9..53149ab08aab1 100644 --- a/planner/core/point_get_plan.go +++ b/planner/core/point_get_plan.go @@ -19,11 +19,7 @@ import ( "sort" "strconv" "strings" -<<<<<<< HEAD:planner/core/point_get_plan.go -======= "sync" - "unsafe" ->>>>>>> 1c185556710 (planner: do not convert update to point get if the expr has sub-query (#47454)):pkg/planner/core/point_get_plan.go "github.com/pingcap/errors" "github.com/pingcap/tidb/config"