From d2c146decb9e1c74441625a454db21d778459623 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Fri, 20 Oct 2023 17:35:05 +0800 Subject: [PATCH 1/5] planner: add test for issue 47331 Signed-off-by: hi-rustin --- executor/explain_test.go | 51 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/executor/explain_test.go b/executor/explain_test.go index 902f7c96eb2de..03309d1f363f1 100644 --- a/executor/explain_test.go +++ b/executor/explain_test.go @@ -189,6 +189,57 @@ func checkMemoryInfo(t *testing.T, tk *testkit.TestKit, sql string) { } } +func TestIssue47331(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t1") + tk.MustExec(`create table t1( + id1 varchar(2) DEFAULT '00', + id2 varchar(30) NOT NULL, + id3 datetime DEFAULT NULL, + id4 varchar(100) NOT NULL DEFAULT 'ecifdata', + id5 datetime NOT NULL DEFAULT CURRENT_TIMESTAMP, + id6 int(11) DEFAULT NULL, + id7 int(11) DEFAULT NULL, + UNIQUE KEY UI_id2 (id2), + KEY ix_id1 (id1) + )`) + tk.MustExec("drop table if exists t2") + tk.MustExec(`create table t2( + id10 varchar(40) NOT NULL, + id2 varchar(30) NOT NULL, + KEY IX_id2 (id2), + PRIMARY KEY (id10) + )`) + tk.MustExec("drop table if exists t3") + tk.MustExec(`create table t3( + id20 varchar(40) DEFAULT NULL, + UNIQUE KEY IX_id20 (id20) + )`) + tk.MustExec(` + explain + UPDATE t1 a + SET a.id1 = '04', + a.id3 = CURRENT_TIMESTAMP, + a.id4 = SUBSTRING_INDEX(USER(), '@', 1), + a.id5 = CURRENT_TIMESTAMP + WHERE a.id1 = '03' + AND a.id6 - IFNULL(a.id7, 0) = + ( + SELECT COUNT(1) + FROM t2 b, t3 c + WHERE b.id10 = c.id20 + AND b.id2 = a.id2 + AND b.id2 in ( + SELECT rn.id2 + FROM t1 rn + WHERE rn.id1 = '03' + ) + ); + `) +} + func TestMemoryAndDiskUsageAfterClose(t *testing.T) { store := testkit.CreateMockStore(t) tk := testkit.NewTestKit(t, store) From adbbc925bcfff10d834bc597e641cd66e3fbc6ac Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Wed, 22 Nov 2023 12:14:11 +0800 Subject: [PATCH 2/5] Clone the originalSchema Signed-off-by: hi-rustin --- planner/core/rule_join_reorder.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/planner/core/rule_join_reorder.go b/planner/core/rule_join_reorder.go index fe5a27e531753..7c4f153cc91a6 100644 --- a/planner/core/rule_join_reorder.go +++ b/planner/core/rule_join_reorder.go @@ -311,7 +311,8 @@ func (s *joinReOrderSolver) optimizeRecursive(ctx sessionctx.Context, p LogicalP proj := LogicalProjection{ Exprs: expression.Column2Exprs(originalSchema.Columns), }.Init(p.SCtx(), p.SelectBlockOffset()) - proj.SetSchema(originalSchema) + // Clone the schema here, because the schema may be changed by the projection optimization later. + proj.SetSchema(originalSchema.Clone()) proj.SetChildren(p) p = proj } From 79c565d6ff26edae804a75e14b1179d267919662 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Wed, 22 Nov 2023 14:36:12 +0800 Subject: [PATCH 3/5] Better comment Signed-off-by: hi-rustin --- planner/core/rule_join_reorder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/rule_join_reorder.go b/planner/core/rule_join_reorder.go index 7c4f153cc91a6..6167faa2a3523 100644 --- a/planner/core/rule_join_reorder.go +++ b/planner/core/rule_join_reorder.go @@ -311,7 +311,7 @@ func (s *joinReOrderSolver) optimizeRecursive(ctx sessionctx.Context, p LogicalP proj := LogicalProjection{ Exprs: expression.Column2Exprs(originalSchema.Columns), }.Init(p.SCtx(), p.SelectBlockOffset()) - // Clone the schema here, because the schema may be changed by the projection optimization later. + // Clone the schema here, because the schema may be changed by column pruning rules. proj.SetSchema(originalSchema.Clone()) proj.SetChildren(p) p = proj From 3e35fc9c4399d2e1f060c2e882a314f16f5a284b Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Fri, 8 Dec 2023 15:46:28 +0800 Subject: [PATCH 4/5] fix Signed-off-by: hi-rustin --- executor/test/BUILD.bazel | 16 ++++ executor/test/explain_test.go | 147 ++++++++++++++++++++++++++++++++++ executor/test/main_test.go | 30 +++++++ executor/update_test.go | 126 ----------------------------- 4 files changed, 193 insertions(+), 126 deletions(-) create mode 100644 executor/test/BUILD.bazel create mode 100644 executor/test/explain_test.go create mode 100644 executor/test/main_test.go diff --git a/executor/test/BUILD.bazel b/executor/test/BUILD.bazel new file mode 100644 index 0000000000000..597e70176b218 --- /dev/null +++ b/executor/test/BUILD.bazel @@ -0,0 +1,16 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_test") + +go_test( + name = "test_test", + timeout = "short", + srcs = [ + "explain_test.go", + "main_test.go", + ], + flaky = True, + deps = [ + "//testkit", + "@com_github_stretchr_testify//require", + "@org_uber_go_goleak//:goleak", + ], +) diff --git a/executor/test/explain_test.go b/executor/test/explain_test.go new file mode 100644 index 0000000000000..cd446c9b43b5a --- /dev/null +++ b/executor/test/explain_test.go @@ -0,0 +1,147 @@ +// Copyright 2023 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package explain + +import ( + "fmt" + "strconv" + "testing" + "time" + + "github.com/pingcap/tidb/testkit" + "github.com/stretchr/testify/require" +) + +func TestLockUnchangedUniqueKeys(t *testing.T) { + store := testkit.CreateMockStore(t) + + tk1 := testkit.NewTestKit(t, store) + tk2 := testkit.NewTestKit(t, store) + tk1.MustExec("use test") + tk2.MustExec("use test") + + for _, shouldLock := range []bool{true, false} { + for _, tt := range []struct { + name string + create string + insert string + update string + isClusteredPK bool + }{ + { + // ref https://github.com/pingcap/tidb/issues/36438 + "Issue36438", + "create table t (i varchar(10), unique key(i))", + "insert into t values ('a')", + "update t set i = 'a'", + false, + }, + { + "ClusteredAndRowUnchanged", + "create table t (k int, v int, primary key(k) clustered, key sk(k))", + "insert into t values (1, 10)", + "update t force index(sk) set v = 10 where k = 1", + true, + }, + { + "ClusteredAndRowUnchangedAndParted", + "create table t (k int, v int, primary key(k) clustered, key sk(k)) partition by hash(k) partitions 4", + "insert into t values (1, 10)", + "update t force index(sk) set v = 10 where k = 1", + true, + }, + { + "ClusteredAndRowChanged", + "create table t (k int, v int, primary key(k) clustered, key sk(k))", + "insert into t values (1, 10)", + "update t force index(sk) set v = 11 where k = 1", + true, + }, + { + "NonClusteredAndRowUnchanged", + "create table t (k int, v int, primary key(k) nonclustered, key sk(k))", + "insert into t values (1, 10)", + "update t force index(sk) set v = 10 where k = 1", + false, + }, + { + "NonClusteredAndRowUnchangedAndParted", + "create table t (k int, v int, primary key(k) nonclustered, key sk(k)) partition by hash(k) partitions 4", + "insert into t values (1, 10)", + "update t force index(sk) set v = 10 where k = 1", + false, + }, + { + "NonClusteredAndRowChanged", + "create table t (k int, v int, primary key(k) nonclustered, key sk(k))", + "insert into t values (1, 10)", + "update t force index(sk) set v = 11 where k = 1", + false, + }, + { + "UniqueAndRowUnchanged", + "create table t (k int, v int, unique key uk(k), key sk(k))", + "insert into t values (1, 10)", + "update t force index(sk) set v = 10 where k = 1", + false, + }, + { + "UniqueAndRowUnchangedAndParted", + "create table t (k int, v int, unique key uk(k), key sk(k)) partition by hash(k) partitions 4", + "insert into t values (1, 10)", + "update t force index(sk) set v = 10 where k = 1", + false, + }, + { + "UniqueAndRowChanged", + "create table t (k int, v int, unique key uk(k), key sk(k))", + "insert into t values (1, 10)", + "update t force index(sk) set v = 11 where k = 1", + false, + }, + } { + t.Run( + tt.name+"-"+strconv.FormatBool(shouldLock), func(t *testing.T) { + tk1.MustExec(fmt.Sprintf("set @@tidb_lock_unchanged_keys = %v", shouldLock)) + tk1.MustExec("drop table if exists t") + tk1.MustExec(tt.create) + tk1.MustExec(tt.insert) + tk1.MustExec("begin pessimistic") + + tk1.MustExec(tt.update) + + errCh := make(chan error, 1) + go func() { + _, err := tk2.Exec(tt.insert) + errCh <- err + }() + + select { + case <-time.After(100 * time.Millisecond): + if !shouldLock && !tt.isClusteredPK { + require.Fail(t, "insert is blocked by update") + } + tk1.MustExec("rollback") + require.Error(t, <-errCh) + case err := <-errCh: + require.Error(t, err) + if shouldLock { + require.Fail(t, "insert is not blocked by update") + } + } + }, + ) + } + } +} diff --git a/executor/test/main_test.go b/executor/test/main_test.go new file mode 100644 index 0000000000000..365c88b4af771 --- /dev/null +++ b/executor/test/main_test.go @@ -0,0 +1,30 @@ +// Copyright 2022 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package explain + +import ( + "testing" + + "go.uber.org/goleak" +) + +func TestMain(m *testing.M) { + opts := []goleak.Option{ + goleak.IgnoreTopFunction("github.com/golang/glog.(*loggingT).flushDaemon"), + goleak.IgnoreTopFunction("github.com/lestrrat-go/httprc.runFetchWorker"), + goleak.IgnoreTopFunction("github.com/tikv/client-go/v2/txnkv/transaction.keepAlive"), + } + goleak.VerifyTestMain(m, opts...) +} diff --git a/executor/update_test.go b/executor/update_test.go index 10f1b5cb87578..04c39dbc3beb1 100644 --- a/executor/update_test.go +++ b/executor/update_test.go @@ -15,10 +15,7 @@ package executor_test import ( - "fmt" - "strconv" "testing" - "time" "github.com/pingcap/tidb/errno" "github.com/pingcap/tidb/kv" @@ -519,126 +516,3 @@ func TestIssue23553(t *testing.T) { tk.MustExec(`insert into tt values('1',0),('1',0),('1',0)`) tk.MustExec(`update tt a inner join (select m0 from tt where status!=1 group by m0 having count(*)>1) b on a.m0=b.m0 set a.status=1`) } - -func TestLockUnchangedUniqueKeys(t *testing.T) { - store := testkit.CreateMockStore(t) - - tk1 := testkit.NewTestKit(t, store) - tk2 := testkit.NewTestKit(t, store) - tk1.MustExec("use test") - tk2.MustExec("use test") - - for _, shouldLock := range []bool{true, false} { - for _, tt := range []struct { - name string - create string - insert string - update string - isClusteredPK bool - }{ - { - // ref https://github.com/pingcap/tidb/issues/36438 - "Issue36438", - "create table t (i varchar(10), unique key(i))", - "insert into t values ('a')", - "update t set i = 'a'", - false, - }, - { - "ClusteredAndRowUnchanged", - "create table t (k int, v int, primary key(k) clustered, key sk(k))", - "insert into t values (1, 10)", - "update t force index(sk) set v = 10 where k = 1", - true, - }, - { - "ClusteredAndRowUnchangedAndParted", - "create table t (k int, v int, primary key(k) clustered, key sk(k)) partition by hash(k) partitions 4", - "insert into t values (1, 10)", - "update t force index(sk) set v = 10 where k = 1", - true, - }, - { - "ClusteredAndRowChanged", - "create table t (k int, v int, primary key(k) clustered, key sk(k))", - "insert into t values (1, 10)", - "update t force index(sk) set v = 11 where k = 1", - true, - }, - { - "NonClusteredAndRowUnchanged", - "create table t (k int, v int, primary key(k) nonclustered, key sk(k))", - "insert into t values (1, 10)", - "update t force index(sk) set v = 10 where k = 1", - false, - }, - { - "NonClusteredAndRowUnchangedAndParted", - "create table t (k int, v int, primary key(k) nonclustered, key sk(k)) partition by hash(k) partitions 4", - "insert into t values (1, 10)", - "update t force index(sk) set v = 10 where k = 1", - false, - }, - { - "NonClusteredAndRowChanged", - "create table t (k int, v int, primary key(k) nonclustered, key sk(k))", - "insert into t values (1, 10)", - "update t force index(sk) set v = 11 where k = 1", - false, - }, - { - "UniqueAndRowUnchanged", - "create table t (k int, v int, unique key uk(k), key sk(k))", - "insert into t values (1, 10)", - "update t force index(sk) set v = 10 where k = 1", - false, - }, - { - "UniqueAndRowUnchangedAndParted", - "create table t (k int, v int, unique key uk(k), key sk(k)) partition by hash(k) partitions 4", - "insert into t values (1, 10)", - "update t force index(sk) set v = 10 where k = 1", - false, - }, - { - "UniqueAndRowChanged", - "create table t (k int, v int, unique key uk(k), key sk(k))", - "insert into t values (1, 10)", - "update t force index(sk) set v = 11 where k = 1", - false, - }, - } { - t.Run( - tt.name+"-"+strconv.FormatBool(shouldLock), func(t *testing.T) { - tk1.MustExec(fmt.Sprintf("set @@tidb_lock_unchanged_keys = %v", shouldLock)) - tk1.MustExec("drop table if exists t") - tk1.MustExec(tt.create) - tk1.MustExec(tt.insert) - tk1.MustExec("begin pessimistic") - - tk1.MustExec(tt.update) - - errCh := make(chan error, 1) - go func() { - _, err := tk2.Exec(tt.insert) - errCh <- err - }() - - select { - case <-time.After(100 * time.Millisecond): - if !shouldLock && !tt.isClusteredPK { - require.Fail(t, "insert is blocked by update") - } - tk1.MustExec("rollback") - require.Error(t, <-errCh) - case err := <-errCh: - require.Error(t, err) - if shouldLock { - require.Fail(t, "insert is not blocked by update") - } - } - }, - ) - } - } -} From 1a1241778c1e0518b5197d61d0ff8bd47105ac3e Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Fri, 8 Dec 2023 21:40:20 +0800 Subject: [PATCH 5/5] Fix Signed-off-by: hi-rustin --- executor/explain_test.go | 51 ---------- executor/test/BUILD.bazel | 1 - executor/test/explain_test.go | 172 ++++++++++------------------------ executor/update_test.go | 126 +++++++++++++++++++++++++ 4 files changed, 174 insertions(+), 176 deletions(-) diff --git a/executor/explain_test.go b/executor/explain_test.go index 03309d1f363f1..902f7c96eb2de 100644 --- a/executor/explain_test.go +++ b/executor/explain_test.go @@ -189,57 +189,6 @@ func checkMemoryInfo(t *testing.T, tk *testkit.TestKit, sql string) { } } -func TestIssue47331(t *testing.T) { - store := testkit.CreateMockStore(t) - tk := testkit.NewTestKit(t, store) - tk.MustExec("use test") - tk.MustExec("drop table if exists t1") - tk.MustExec(`create table t1( - id1 varchar(2) DEFAULT '00', - id2 varchar(30) NOT NULL, - id3 datetime DEFAULT NULL, - id4 varchar(100) NOT NULL DEFAULT 'ecifdata', - id5 datetime NOT NULL DEFAULT CURRENT_TIMESTAMP, - id6 int(11) DEFAULT NULL, - id7 int(11) DEFAULT NULL, - UNIQUE KEY UI_id2 (id2), - KEY ix_id1 (id1) - )`) - tk.MustExec("drop table if exists t2") - tk.MustExec(`create table t2( - id10 varchar(40) NOT NULL, - id2 varchar(30) NOT NULL, - KEY IX_id2 (id2), - PRIMARY KEY (id10) - )`) - tk.MustExec("drop table if exists t3") - tk.MustExec(`create table t3( - id20 varchar(40) DEFAULT NULL, - UNIQUE KEY IX_id20 (id20) - )`) - tk.MustExec(` - explain - UPDATE t1 a - SET a.id1 = '04', - a.id3 = CURRENT_TIMESTAMP, - a.id4 = SUBSTRING_INDEX(USER(), '@', 1), - a.id5 = CURRENT_TIMESTAMP - WHERE a.id1 = '03' - AND a.id6 - IFNULL(a.id7, 0) = - ( - SELECT COUNT(1) - FROM t2 b, t3 c - WHERE b.id10 = c.id20 - AND b.id2 = a.id2 - AND b.id2 in ( - SELECT rn.id2 - FROM t1 rn - WHERE rn.id1 = '03' - ) - ); - `) -} - func TestMemoryAndDiskUsageAfterClose(t *testing.T) { store := testkit.CreateMockStore(t) tk := testkit.NewTestKit(t, store) diff --git a/executor/test/BUILD.bazel b/executor/test/BUILD.bazel index 597e70176b218..0d0c1f1a307ec 100644 --- a/executor/test/BUILD.bazel +++ b/executor/test/BUILD.bazel @@ -10,7 +10,6 @@ go_test( flaky = True, deps = [ "//testkit", - "@com_github_stretchr_testify//require", "@org_uber_go_goleak//:goleak", ], ) diff --git a/executor/test/explain_test.go b/executor/test/explain_test.go index cd446c9b43b5a..743218844440c 100644 --- a/executor/test/explain_test.go +++ b/executor/test/explain_test.go @@ -14,134 +14,58 @@ package explain import ( - "fmt" - "strconv" "testing" - "time" "github.com/pingcap/tidb/testkit" - "github.com/stretchr/testify/require" ) -func TestLockUnchangedUniqueKeys(t *testing.T) { +func TestIssue47331(t *testing.T) { store := testkit.CreateMockStore(t) - - tk1 := testkit.NewTestKit(t, store) - tk2 := testkit.NewTestKit(t, store) - tk1.MustExec("use test") - tk2.MustExec("use test") - - for _, shouldLock := range []bool{true, false} { - for _, tt := range []struct { - name string - create string - insert string - update string - isClusteredPK bool - }{ - { - // ref https://github.com/pingcap/tidb/issues/36438 - "Issue36438", - "create table t (i varchar(10), unique key(i))", - "insert into t values ('a')", - "update t set i = 'a'", - false, - }, - { - "ClusteredAndRowUnchanged", - "create table t (k int, v int, primary key(k) clustered, key sk(k))", - "insert into t values (1, 10)", - "update t force index(sk) set v = 10 where k = 1", - true, - }, - { - "ClusteredAndRowUnchangedAndParted", - "create table t (k int, v int, primary key(k) clustered, key sk(k)) partition by hash(k) partitions 4", - "insert into t values (1, 10)", - "update t force index(sk) set v = 10 where k = 1", - true, - }, - { - "ClusteredAndRowChanged", - "create table t (k int, v int, primary key(k) clustered, key sk(k))", - "insert into t values (1, 10)", - "update t force index(sk) set v = 11 where k = 1", - true, - }, - { - "NonClusteredAndRowUnchanged", - "create table t (k int, v int, primary key(k) nonclustered, key sk(k))", - "insert into t values (1, 10)", - "update t force index(sk) set v = 10 where k = 1", - false, - }, - { - "NonClusteredAndRowUnchangedAndParted", - "create table t (k int, v int, primary key(k) nonclustered, key sk(k)) partition by hash(k) partitions 4", - "insert into t values (1, 10)", - "update t force index(sk) set v = 10 where k = 1", - false, - }, - { - "NonClusteredAndRowChanged", - "create table t (k int, v int, primary key(k) nonclustered, key sk(k))", - "insert into t values (1, 10)", - "update t force index(sk) set v = 11 where k = 1", - false, - }, - { - "UniqueAndRowUnchanged", - "create table t (k int, v int, unique key uk(k), key sk(k))", - "insert into t values (1, 10)", - "update t force index(sk) set v = 10 where k = 1", - false, - }, - { - "UniqueAndRowUnchangedAndParted", - "create table t (k int, v int, unique key uk(k), key sk(k)) partition by hash(k) partitions 4", - "insert into t values (1, 10)", - "update t force index(sk) set v = 10 where k = 1", - false, - }, - { - "UniqueAndRowChanged", - "create table t (k int, v int, unique key uk(k), key sk(k))", - "insert into t values (1, 10)", - "update t force index(sk) set v = 11 where k = 1", - false, - }, - } { - t.Run( - tt.name+"-"+strconv.FormatBool(shouldLock), func(t *testing.T) { - tk1.MustExec(fmt.Sprintf("set @@tidb_lock_unchanged_keys = %v", shouldLock)) - tk1.MustExec("drop table if exists t") - tk1.MustExec(tt.create) - tk1.MustExec(tt.insert) - tk1.MustExec("begin pessimistic") - - tk1.MustExec(tt.update) - - errCh := make(chan error, 1) - go func() { - _, err := tk2.Exec(tt.insert) - errCh <- err - }() - - select { - case <-time.After(100 * time.Millisecond): - if !shouldLock && !tt.isClusteredPK { - require.Fail(t, "insert is blocked by update") - } - tk1.MustExec("rollback") - require.Error(t, <-errCh) - case err := <-errCh: - require.Error(t, err) - if shouldLock { - require.Fail(t, "insert is not blocked by update") - } - } - }, - ) - } - } + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t1") + tk.MustExec(`create table t1( + id1 varchar(2) DEFAULT '00', + id2 varchar(30) NOT NULL, + id3 datetime DEFAULT NULL, + id4 varchar(100) NOT NULL DEFAULT 'ecifdata', + id5 datetime NOT NULL DEFAULT CURRENT_TIMESTAMP, + id6 int(11) DEFAULT NULL, + id7 int(11) DEFAULT NULL, + UNIQUE KEY UI_id2 (id2), + KEY ix_id1 (id1) + )`) + tk.MustExec("drop table if exists t2") + tk.MustExec(`create table t2( + id10 varchar(40) NOT NULL, + id2 varchar(30) NOT NULL, + KEY IX_id2 (id2), + PRIMARY KEY (id10) + )`) + tk.MustExec("drop table if exists t3") + tk.MustExec(`create table t3( + id20 varchar(40) DEFAULT NULL, + UNIQUE KEY IX_id20 (id20) + )`) + tk.MustExec(` + explain + UPDATE t1 a + SET a.id1 = '04', + a.id3 = CURRENT_TIMESTAMP, + a.id4 = SUBSTRING_INDEX(USER(), '@', 1), + a.id5 = CURRENT_TIMESTAMP + WHERE a.id1 = '03' + AND a.id6 - IFNULL(a.id7, 0) = + ( + SELECT COUNT(1) + FROM t2 b, t3 c + WHERE b.id10 = c.id20 + AND b.id2 = a.id2 + AND b.id2 in ( + SELECT rn.id2 + FROM t1 rn + WHERE rn.id1 = '03' + ) + ); + `) } diff --git a/executor/update_test.go b/executor/update_test.go index 04c39dbc3beb1..10f1b5cb87578 100644 --- a/executor/update_test.go +++ b/executor/update_test.go @@ -15,7 +15,10 @@ package executor_test import ( + "fmt" + "strconv" "testing" + "time" "github.com/pingcap/tidb/errno" "github.com/pingcap/tidb/kv" @@ -516,3 +519,126 @@ func TestIssue23553(t *testing.T) { tk.MustExec(`insert into tt values('1',0),('1',0),('1',0)`) tk.MustExec(`update tt a inner join (select m0 from tt where status!=1 group by m0 having count(*)>1) b on a.m0=b.m0 set a.status=1`) } + +func TestLockUnchangedUniqueKeys(t *testing.T) { + store := testkit.CreateMockStore(t) + + tk1 := testkit.NewTestKit(t, store) + tk2 := testkit.NewTestKit(t, store) + tk1.MustExec("use test") + tk2.MustExec("use test") + + for _, shouldLock := range []bool{true, false} { + for _, tt := range []struct { + name string + create string + insert string + update string + isClusteredPK bool + }{ + { + // ref https://github.com/pingcap/tidb/issues/36438 + "Issue36438", + "create table t (i varchar(10), unique key(i))", + "insert into t values ('a')", + "update t set i = 'a'", + false, + }, + { + "ClusteredAndRowUnchanged", + "create table t (k int, v int, primary key(k) clustered, key sk(k))", + "insert into t values (1, 10)", + "update t force index(sk) set v = 10 where k = 1", + true, + }, + { + "ClusteredAndRowUnchangedAndParted", + "create table t (k int, v int, primary key(k) clustered, key sk(k)) partition by hash(k) partitions 4", + "insert into t values (1, 10)", + "update t force index(sk) set v = 10 where k = 1", + true, + }, + { + "ClusteredAndRowChanged", + "create table t (k int, v int, primary key(k) clustered, key sk(k))", + "insert into t values (1, 10)", + "update t force index(sk) set v = 11 where k = 1", + true, + }, + { + "NonClusteredAndRowUnchanged", + "create table t (k int, v int, primary key(k) nonclustered, key sk(k))", + "insert into t values (1, 10)", + "update t force index(sk) set v = 10 where k = 1", + false, + }, + { + "NonClusteredAndRowUnchangedAndParted", + "create table t (k int, v int, primary key(k) nonclustered, key sk(k)) partition by hash(k) partitions 4", + "insert into t values (1, 10)", + "update t force index(sk) set v = 10 where k = 1", + false, + }, + { + "NonClusteredAndRowChanged", + "create table t (k int, v int, primary key(k) nonclustered, key sk(k))", + "insert into t values (1, 10)", + "update t force index(sk) set v = 11 where k = 1", + false, + }, + { + "UniqueAndRowUnchanged", + "create table t (k int, v int, unique key uk(k), key sk(k))", + "insert into t values (1, 10)", + "update t force index(sk) set v = 10 where k = 1", + false, + }, + { + "UniqueAndRowUnchangedAndParted", + "create table t (k int, v int, unique key uk(k), key sk(k)) partition by hash(k) partitions 4", + "insert into t values (1, 10)", + "update t force index(sk) set v = 10 where k = 1", + false, + }, + { + "UniqueAndRowChanged", + "create table t (k int, v int, unique key uk(k), key sk(k))", + "insert into t values (1, 10)", + "update t force index(sk) set v = 11 where k = 1", + false, + }, + } { + t.Run( + tt.name+"-"+strconv.FormatBool(shouldLock), func(t *testing.T) { + tk1.MustExec(fmt.Sprintf("set @@tidb_lock_unchanged_keys = %v", shouldLock)) + tk1.MustExec("drop table if exists t") + tk1.MustExec(tt.create) + tk1.MustExec(tt.insert) + tk1.MustExec("begin pessimistic") + + tk1.MustExec(tt.update) + + errCh := make(chan error, 1) + go func() { + _, err := tk2.Exec(tt.insert) + errCh <- err + }() + + select { + case <-time.After(100 * time.Millisecond): + if !shouldLock && !tt.isClusteredPK { + require.Fail(t, "insert is blocked by update") + } + tk1.MustExec("rollback") + require.Error(t, <-errCh) + case err := <-errCh: + require.Error(t, err) + if shouldLock { + require.Fail(t, "insert is not blocked by update") + } + } + }, + ) + } + } +}