From 223a9128b03f946a71ca8f57e1cef34ff20232ef Mon Sep 17 00:00:00 2001 From: YangKeao Date: Sat, 17 Dec 2022 00:48:52 -0500 Subject: [PATCH 1/4] staleread, session: internal write request should be accepted with external ts (#39967) close pingcap/tidb#39966 --- session/session.go | 2 +- sessiontxn/staleread/externalts_test.go | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/session/session.go b/session/session.go index 332ba34e4724c..2c6aa0567fa66 100644 --- a/session/session.go +++ b/session/session.go @@ -2241,7 +2241,7 @@ func (s *session) onTxnManagerStmtStartOrRetry(ctx context.Context, node ast.Stm func (s *session) validateStatementReadOnlyInStaleness(stmtNode ast.StmtNode) error { vars := s.GetSessionVars() - if !vars.TxnCtx.IsStaleness && vars.TxnReadTS.PeakTxnReadTS() == 0 && !vars.EnableExternalTSRead { + if !vars.TxnCtx.IsStaleness && vars.TxnReadTS.PeakTxnReadTS() == 0 && !vars.EnableExternalTSRead || vars.InRestrictedSQL { return nil } errMsg := "only support read-only statement during read-only staleness transactions" diff --git a/sessiontxn/staleread/externalts_test.go b/sessiontxn/staleread/externalts_test.go index 289c24d820d8f..2ee7c6f14f767 100644 --- a/sessiontxn/staleread/externalts_test.go +++ b/sessiontxn/staleread/externalts_test.go @@ -15,8 +15,10 @@ package staleread_test import ( + "context" "testing" + "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/parser/auth" "github.com/pingcap/tidb/testkit" "github.com/stretchr/testify/require" @@ -67,12 +69,20 @@ func TestExternalTimestampReadonly(t *testing.T) { tk.MustQuery("select @@tidb_external_ts").Check(testkit.Rows("0")) tk.MustExec("start transaction;set global tidb_external_ts=@@tidb_current_ts;commit;") + // with tidb_enable_external_ts_read enabled, this session will be readonly tk.MustExec("set tidb_enable_external_ts_read=ON") _, err := tk.Exec("insert into t values (0)") require.Error(t, err) tk.MustExec("set tidb_enable_external_ts_read=OFF") tk.MustExec("insert into t values (0)") + + // even when tidb_enable_external_ts_read is enabled, internal SQL will not be affected + tk.MustExec("set tidb_enable_external_ts_read=ON") + tk.Session().GetSessionVars().InRestrictedSQL = true + ctx := kv.WithInternalSourceType(context.Background(), kv.InternalTxnOthers) + tk.MustExecWithContext(ctx, "insert into t values (1)") + tk.Session().GetSessionVars().InRestrictedSQL = false } func TestExternalTimestampReadWithTransaction(t *testing.T) { From f150d376517e126b6ca2396f423e1709ed9e151c Mon Sep 17 00:00:00 2001 From: wjHuang Date: Sat, 17 Dec 2022 15:16:53 +0800 Subject: [PATCH 2/4] *: revert 38302, forbid modify column on partition table (#39991) ref pingcap/tidb#39915, ref pingcap/tidb#39922 --- ddl/backfilling.go | 3 - ddl/column.go | 38 ++----- ddl/db_partition_test.go | 207 ----------------------------------- ddl/ddl_api.go | 3 + ddl/failtest/fail_db_test.go | 4 +- 5 files changed, 13 insertions(+), 242 deletions(-) diff --git a/ddl/backfilling.go b/ddl/backfilling.go index 7c966591016d9..d1035bad084bd 100644 --- a/ddl/backfilling.go +++ b/ddl/backfilling.go @@ -546,9 +546,6 @@ func (dc *ddlCtx) handleRangeTasks(scheduler *backfillScheduler, t table.Table, reorgInfo := scheduler.reorgInfo physicalTableID := reorgInfo.PhysicalTableID var prefix kv.Key - if tbl, ok := t.(table.PartitionedTable); ok { - t = tbl.GetPartition(physicalTableID) - } if reorgInfo.mergingTmpIdx { prefix = t.IndexPrefix() } else { diff --git a/ddl/column.go b/ddl/column.go index d9425ceabac2c..3bbe7417fd7b5 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -1042,30 +1042,9 @@ func BuildElements(changingCol *model.ColumnInfo, changingIdxs []*model.IndexInf return elements } -func (w *worker) updatePhysicalTableRow(t table.Table, reorgInfo *reorgInfo) error { +func (w *worker) updatePhysicalTableRow(t table.PhysicalTable, reorgInfo *reorgInfo) error { logutil.BgLogger().Info("[ddl] start to update table row", zap.String("job", reorgInfo.Job.String()), zap.String("reorgInfo", reorgInfo.String())) - if tbl, ok := t.(table.PartitionedTable); ok { - done := false - for !done { - p := tbl.GetPartition(reorgInfo.PhysicalTableID) - if p == nil { - return dbterror.ErrCancelledDDLJob.GenWithStack("Can not find partition id %d for table %d", reorgInfo.PhysicalTableID, t.Meta().ID) - } - err := w.writePhysicalTableRecord(w.sessPool, p, typeUpdateColumnWorker, reorgInfo) - if err != nil { - return err - } - done, err = w.updateReorgInfo(tbl, reorgInfo) - if err != nil { - return errors.Trace(err) - } - } - return nil - } - if tbl, ok := t.(table.PhysicalTable); ok { - return w.writePhysicalTableRecord(w.sessPool, tbl, typeUpdateColumnWorker, reorgInfo) - } - return dbterror.ErrCancelledDDLJob.GenWithStack("internal error for phys tbl id: %d tbl id: %d", reorgInfo.PhysicalTableID, t.Meta().ID) + return w.writePhysicalTableRecord(w.sessPool, t, typeUpdateColumnWorker, reorgInfo) } // TestReorgGoroutineRunning is only used in test to indicate the reorg goroutine has been started. @@ -1087,25 +1066,22 @@ func (w *worker) updateCurrentElement(t table.Table, reorgInfo *reorgInfo) error } } }) + // TODO: Support partition tables. if bytes.Equal(reorgInfo.currElement.TypeKey, meta.ColumnElementKey) { - err := w.updatePhysicalTableRow(t, reorgInfo) + //nolint:forcetypeassert + err := w.updatePhysicalTableRow(t.(table.PhysicalTable), reorgInfo) if err != nil { return errors.Trace(err) } } - var physTbl table.PhysicalTable - if tbl, ok := t.(table.PartitionedTable); ok { - physTbl = tbl.GetPartition(reorgInfo.PhysicalTableID) - } else if tbl, ok := t.(table.PhysicalTable); ok { - physTbl = tbl - } // Get the original start handle and end handle. currentVer, err := getValidCurrentVersion(reorgInfo.d.store) if err != nil { return errors.Trace(err) } - originalStartHandle, originalEndHandle, err := getTableRange(reorgInfo.d.jobContext(reorgInfo.Job), reorgInfo.d, physTbl, currentVer.Ver, reorgInfo.Job.Priority) + //nolint:forcetypeassert + originalStartHandle, originalEndHandle, err := getTableRange(reorgInfo.d.jobContext(reorgInfo.Job), reorgInfo.d, t.(table.PhysicalTable), currentVer.Ver, reorgInfo.Job.Priority) if err != nil { return errors.Trace(err) } diff --git a/ddl/db_partition_test.go b/ddl/db_partition_test.go index 67e7d6a0484d7..38aa43e4e7755 100644 --- a/ddl/db_partition_test.go +++ b/ddl/db_partition_test.go @@ -4536,136 +4536,6 @@ func TestPartitionTableWithAnsiQuotes(t *testing.T) { ` PARTITION "pMax" VALUES LESS THAN (MAXVALUE,MAXVALUE))`)) } -func TestAlterModifyColumnOnPartitionedTable(t *testing.T) { - store := testkit.CreateMockStore(t) - tk := testkit.NewTestKit(t, store) - tk.MustExec("create database AlterPartTable") - tk.MustExec("use AlterPartTable") - tk.MustExec(`create table t (a int unsigned PRIMARY KEY, b varchar(255), key (b))`) - tk.MustExec(`insert into t values (7, "07"), (8, "08"),(23,"23"),(34,"34πŸ’₯"),(46,"46"),(57,"57")`) - tk.MustQuery(`show create table t`).Check(testkit.Rows( - "t CREATE TABLE `t` (\n" + - " `a` int(10) unsigned NOT NULL,\n" + - " `b` varchar(255) DEFAULT NULL,\n" + - " PRIMARY KEY (`a`) /*T![clustered_index] CLUSTERED */,\n" + - " KEY `b` (`b`)\n" + - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) - // TODO: Why does it allow πŸ’₯ as a latin1 character? - tk.MustQuery(`select hex(b) from t where a = 34`).Check(testkit.Rows("3334F09F92A5")) - tk.MustExec(`alter table t modify b varchar(200) charset latin1`) - tk.MustQuery(`show create table t`).Check(testkit.Rows( - "t CREATE TABLE `t` (\n" + - " `a` int(10) unsigned NOT NULL,\n" + - " `b` varchar(200) CHARACTER SET latin1 COLLATE latin1_bin DEFAULT NULL,\n" + - " PRIMARY KEY (`a`) /*T![clustered_index] CLUSTERED */,\n" + - " KEY `b` (`b`)\n" + - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) - tk.MustQuery(`select hex(b) from t where a = 34`).Check(testkit.Rows("3334F09F92A5")) - tk.MustQuery(`select * from t`).Sort().Check(testkit.Rows(""+ - "23 23", - "34 34πŸ’₯", - "46 46", - "57 57", - "7 07", - "8 08")) - tk.MustQuery(`select * from t order by b`).Check(testkit.Rows(""+ - "7 07", - "8 08", - "23 23", - "34 34πŸ’₯", - "46 46", - "57 57")) - tk.MustExec(`alter table t change b c varchar(200) charset utf8mb4`) - tk.MustExec(`drop table t`) - tk.MustExec(`create table t (a int unsigned PRIMARY KEY, b varchar(255), key (b)) partition by range (a) ` + - `(partition p0 values less than (10),` + - ` partition p1 values less than (20),` + - ` partition p2 values less than (30),` + - ` partition pMax values less than (MAXVALUE))`) - tk.MustExec(`insert into t values (7, "07"), (8, "08"),(23,"23"),(34,"34πŸ’₯"),(46,"46"),(57,"57")`) - tk.MustQuery(`select * from t`).Sort().Check(testkit.Rows(""+ - "23 23", - "34 34πŸ’₯", - "46 46", - "57 57", - "7 07", - "8 08")) - tk.MustQuery(`select * from t order by b`).Check(testkit.Rows(""+ - "7 07", - "8 08", - "23 23", - "34 34πŸ’₯", - "46 46", - "57 57")) - tk.MustQuery(`show create table t`).Check(testkit.Rows( - "t CREATE TABLE `t` (\n" + - " `a` int(10) unsigned NOT NULL,\n" + - " `b` varchar(255) DEFAULT NULL,\n" + - " PRIMARY KEY (`a`) /*T![clustered_index] CLUSTERED */,\n" + - " KEY `b` (`b`)\n" + - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin\n" + - "PARTITION BY RANGE (`a`)\n" + - "(PARTITION `p0` VALUES LESS THAN (10),\n" + - " PARTITION `p1` VALUES LESS THAN (20),\n" + - " PARTITION `p2` VALUES LESS THAN (30),\n" + - " PARTITION `pMax` VALUES LESS THAN (MAXVALUE))")) - tk.MustExec(`alter table t modify b varchar(200) charset latin1`) - tk.MustQuery(`show create table t`).Check(testkit.Rows( - "t CREATE TABLE `t` (\n" + - " `a` int(10) unsigned NOT NULL,\n" + - " `b` varchar(200) CHARACTER SET latin1 COLLATE latin1_bin DEFAULT NULL,\n" + - " PRIMARY KEY (`a`) /*T![clustered_index] CLUSTERED */,\n" + - " KEY `b` (`b`)\n" + - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin\n" + - "PARTITION BY RANGE (`a`)\n" + - "(PARTITION `p0` VALUES LESS THAN (10),\n" + - " PARTITION `p1` VALUES LESS THAN (20),\n" + - " PARTITION `p2` VALUES LESS THAN (30),\n" + - " PARTITION `pMax` VALUES LESS THAN (MAXVALUE))")) - tk.MustQuery(`select * from t`).Sort().Check(testkit.Rows(""+ - "23 23", - "34 34πŸ’₯", - "46 46", - "57 57", - "7 07", - "8 08")) - tk.MustQuery(`select * from t order by b`).Check(testkit.Rows(""+ - "7 07", - "8 08", - "23 23", - "34 34πŸ’₯", - "46 46", - "57 57")) - tk.MustExec(`alter table t change b c varchar(150) charset utf8mb4`) - tk.MustQuery(`show create table t`).Check(testkit.Rows( - "t CREATE TABLE `t` (\n" + - " `a` int(10) unsigned NOT NULL,\n" + - " `c` varchar(150) DEFAULT NULL,\n" + - " PRIMARY KEY (`a`) /*T![clustered_index] CLUSTERED */,\n" + - " KEY `b` (`c`)\n" + - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin\n" + - "PARTITION BY RANGE (`a`)\n" + - "(PARTITION `p0` VALUES LESS THAN (10),\n" + - " PARTITION `p1` VALUES LESS THAN (20),\n" + - " PARTITION `p2` VALUES LESS THAN (30),\n" + - " PARTITION `pMax` VALUES LESS THAN (MAXVALUE))")) - tk.MustQuery(`select * from t`).Sort().Check(testkit.Rows(""+ - "23 23", - "34 34πŸ’₯", - "46 46", - "57 57", - "7 07", - "8 08")) - tk.MustQuery(`select * from t order by c`).Check(testkit.Rows(""+ - "7 07", - "8 08", - "23 23", - "34 34πŸ’₯", - "46 46", - "57 57")) - tk.MustGetErrCode(`alter table t modify a varchar(20)`, errno.ErrUnsupportedDDLOperation) -} - func TestAlterModifyColumnOnPartitionedTableRename(t *testing.T) { store := testkit.CreateMockStore(t) tk := testkit.NewTestKit(t, store) @@ -4687,80 +4557,3 @@ func TestAlterModifyColumnOnPartitionedTableRename(t *testing.T) { tk.MustExec(`create table t (a int, b char) partition by hash (a) partitions 3`) tk.MustContainErrMsg(`alter table t change a c int`, "[planner:1054]Unknown column 'a' in 'expression'") } - -func TestAlterModifyColumnOnPartitionedTableFail(t *testing.T) { - store := testkit.CreateMockStore(t) - tk := testkit.NewTestKit(t, store) - schemaName := "modColPartFail" - tk.MustExec("create database " + schemaName) - tk.MustExec("use " + schemaName) - tk.MustExec(`create table t (a int unsigned, b varchar(255), key (b)) partition by range (a) (partition p0 values less than (10), partition p1 values less than (20), partition pMax values less than (MAXVALUE))`) - tk.MustExec(`insert into t values (7, "07"), (8, "08"),(23,"23"),(34,"34πŸ’₯"),(46,"46"),(57,"57")`) - tk.MustGetErrCode(`alter table t modify a varchar(255)`, errno.ErrUnsupportedDDLOperation) - tk.MustGetErrCode(`alter table t modify a float`, mysql.ErrFieldTypeNotAllowedAsPartitionField) - tk.MustExec(`drop table t`) - tk.MustExec(`create table t (b int unsigned, a varchar(255), key (b)) partition by range columns (a) (partition p0 values less than (""), partition p1 values less than ("11111"), partition pMax values less than (MAXVALUE))`) - tk.MustExec(`insert into t values (7, "07"), (8, "08"),(23,"23"),(34,"34 πŸ’₯πŸ’₯Longer than 11111"),(46,"46"),(57,"57")`) - tk.MustExec(`alter table t modify a varchar(50)`) - tk.MustGetErrCode(`alter table t modify a float`, mysql.ErrFieldTypeNotAllowedAsPartitionField) - tk.MustGetErrCode(`alter table t modify a int`, errno.ErrUnsupportedDDLOperation) - tk.MustContainErrMsg(`alter table t modify a varchar(4)`, "[ddl:8200]New column does not match partition definitions: [ddl:1654]Partition column values of incorrect type") - tk.MustGetErrCode(`alter table t modify a varchar(5)`, errno.WarnDataTruncated) - tk.MustExec(`SET SQL_MODE = ''`) - tk.MustExec(`alter table t modify a varchar(5)`) - // fix https://github.com/pingcap/tidb/issues/38669 and update this - //tk.MustQuery(`show warnings`).Check(testkit.Rows("Warning 1265 Data truncated for column 'a', value is '34 πŸ’₯πŸ’₯Longer than 11111'")) - tk.MustExec(`SET SQL_MODE = DEFAULT`) - tk.MustQuery(`select * from t`).Sort().Check(testkit.Rows(""+ - "23 23", - "34 34 πŸ’₯πŸ’₯", - "46 46", - "57 57", - "7 07", - "8 08")) - tStr := "" + - "CREATE TABLE `t` (\n" + - " `b` int(10) unsigned DEFAULT NULL,\n" + - " `a` varchar(5) DEFAULT NULL,\n" + - " KEY `b` (`b`)\n" + - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin\n" + - "PARTITION BY RANGE COLUMNS(`a`)\n" + - "(PARTITION `p0` VALUES LESS THAN (''),\n" + - " PARTITION `p1` VALUES LESS THAN ('11111'),\n" + - " PARTITION `pMax` VALUES LESS THAN (MAXVALUE))" - tk.MustQuery(`show create table t`).Check(testkit.Rows("t " + tStr)) - tk.MustExec(`drop table t`) - tk.MustExec(tStr) - tk.MustExec(`drop table t`) - tk.MustExec("create table t (a int, b varchar(255), key (b)) partition by range (a) (partition `p-300` values less than (-300), partition p0 values less than (0), partition p300 values less than (300))") - tk.MustExec(`insert into t values (-400, "-400"), (-100, "-100"), (0, "0"), (100, "100"), (290, "290")`) - tk.MustContainErrMsg(`alter table t modify a int unsigned`, "[ddl:8200]Unsupported modify column, decreasing length of int may result in truncation and change of partition") - tk.MustContainErrMsg(`alter table t modify a tinyint`, "[ddl:8200]Unsupported modify column, decreasing length of int may result in truncation and change of partition") - tk.MustExec(`set sql_mode = ''`) - tk.MustContainErrMsg(`alter table t modify a tinyint`, "[ddl:8200]Unsupported modify column, decreasing length of int may result in truncation and change of partition") - tk.MustQuery("select * from t partition (`p-300`)").Sort().Check(testkit.Rows("-400 -400")) - tk.MustExec(`set sql_mode = default`) - tk.MustContainErrMsg(`alter table t modify a smallint`, "[ddl:8200]Unsupported modify column, decreasing length of int may result in truncation and change of partition") - tk.MustExec(`alter table t modify a bigint`) - tk.MustExec(`drop table t`) - tk.MustExec("create table t (a int, b varchar(255), key (b)) partition by range columns (a) (partition `p-300` values less than (-300), partition p0 values less than (0), partition p300 values less than (300))") - tk.MustExec(`insert into t values (-400, "-400"), (-100, "-100"), (0, "0"), (100, "100"), (290, "290")`) - tk.MustContainErrMsg(`alter table t modify a int unsigned`, "[ddl:8200]Unsupported modify column: can't change the partitioning column, since it would require reorganize all partitions") - tk.MustContainErrMsg(`alter table t modify a tinyint`, "[ddl:8200]New column does not match partition definitions: [ddl:1654]Partition column values of incorrect type") - tk.MustExec(`set sql_mode = ''`) - tk.MustContainErrMsg(`alter table t modify a tinyint`, "[ddl:8200]New column does not match partition definitions: [ddl:1654]Partition column values of incorrect type") - tk.MustQuery("select * from t partition (`p-300`)").Sort().Check(testkit.Rows("-400 -400")) - tk.MustExec(`set sql_mode = default`) - // OK to decrease, since with RANGE COLUMNS, it will check the partition definition values against the new type - tk.MustExec(`alter table t modify a smallint`) - tk.MustExec(`alter table t modify a bigint`) - - tk.MustExec(`drop table t`) - - tk.MustExec(`create table t (a int, b varchar(255), key (b)) partition by list columns (b) (partition p1 values in ("1", "ab", "12345"), partition p2 values in ("2", "abc", "999999"))`) - tk.MustExec(`insert into t values (1, "1"), (2, "2"), (999999, "999999")`) - tk.MustContainErrMsg(`alter table t modify column b varchar(5)`, "[ddl:8200]New column does not match partition definitions: [ddl:1654]Partition column values of incorrect type") - tk.MustExec(`set sql_mode = ''`) - tk.MustContainErrMsg(`alter table t modify column b varchar(5)`, "[ddl:8200]New column does not match partition definitions: [ddl:1654]Partition column values of incorrect type") - tk.MustExec(`set sql_mode = default`) -} diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 84e64dcb61d6b..2850a3aa968a5 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -4681,6 +4681,9 @@ func GetModifiableColumnJob( if err = isGeneratedRelatedColumn(t.Meta(), newCol.ColumnInfo, col.ColumnInfo); err != nil { return nil, errors.Trace(err) } + if t.Meta().Partition != nil { + return nil, dbterror.ErrUnsupportedModifyColumn.GenWithStackByArgs("table is partition table") + } } // Check that the column change does not affect the partitioning column diff --git a/ddl/failtest/fail_db_test.go b/ddl/failtest/fail_db_test.go index d12c2182f9730..4a938e5fd2ad4 100644 --- a/ddl/failtest/fail_db_test.go +++ b/ddl/failtest/fail_db_test.go @@ -494,6 +494,8 @@ func TestModifyColumn(t *testing.T) { tk.MustExec("admin check table t") // Test unsupported statements. + tk.MustExec("create table t1(a int) partition by hash (a) partitions 2") + tk.MustGetErrMsg("alter table t1 modify column a mediumint", "[ddl:8200]Unsupported modify column: table is partition table") tk.MustExec("create table t2(id int, a int, b int generated always as (abs(a)) virtual, c int generated always as (a+1) stored)") tk.MustGetErrMsg("alter table t2 modify column b mediumint", "[ddl:8200]Unsupported modify column: newCol IsGenerated false, oldCol IsGenerated true") tk.MustGetErrMsg("alter table t2 modify column c mediumint", "[ddl:8200]Unsupported modify column: newCol IsGenerated false, oldCol IsGenerated true") @@ -530,7 +532,7 @@ func TestModifyColumn(t *testing.T) { tk.MustExec("insert into t5 values (1,1),(2,2),(3,3),(4,4),(5,5);") tk.MustExec("alter table t5 modify a int not null;") - tk.MustExec("drop table t, t2, t3, t4, t5") + tk.MustExec("drop table t, t1, t2, t3, t4, t5") } func TestIssue38699(t *testing.T) { From 47f54603faebc207b7f2141ed60fd68c262d1b1d Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Mon, 19 Dec 2022 11:40:54 +0800 Subject: [PATCH 3/4] executor: close recordset again (#40010) --- .bazelrc | 2 +- ddl/sequence_test.go | 3 +-- executor/admin_test.go | 2 +- executor/autoidtest/autoid_test.go | 3 +-- executor/executor_test.go | 26 +++++++++----------------- executor/grant_test.go | 12 ++++++------ executor/seqtest/seq_executor_test.go | 19 ++++++------------- 7 files changed, 25 insertions(+), 42 deletions(-) diff --git a/.bazelrc b/.bazelrc index 8339fc80e2d67..61356f086fda5 100644 --- a/.bazelrc +++ b/.bazelrc @@ -1,4 +1,4 @@ -startup --host_jvm_args=-Xmx5g +startup --host_jvm_args=-Xmx8g startup --unlimit_coredumps run:ci --color=yes diff --git a/ddl/sequence_test.go b/ddl/sequence_test.go index df58df12b0ebd..9b798c9f45eea 100644 --- a/ddl/sequence_test.go +++ b/ddl/sequence_test.go @@ -62,8 +62,7 @@ func TestCreateSequence(t *testing.T) { // test unsupported table option in sequence. tk.MustGetErrCode("create sequence seq CHARSET=utf8", mysql.ErrSequenceUnsupportedTableOption) - _, err := tk.Exec("create sequence seq comment=\"test\"") - require.NoError(t, err) + tk.MustExec("create sequence seq comment=\"test\"") sequenceTable := external.GetTableByName(t, tk, "test", "seq") diff --git a/executor/admin_test.go b/executor/admin_test.go index 23b57e9c316b6..41b926cf2c377 100644 --- a/executor/admin_test.go +++ b/executor/admin_test.go @@ -133,7 +133,7 @@ func TestAdminCheckIndexInLocalTemporaryMode(t *testing.T) { tk.MustExec("drop table if exists local_temporary_admin_test;") tk.MustExec("create temporary table local_temporary_admin_test (c1 int, c2 int, c3 int default 1, primary key (c1), index (c1), unique key(c2))") tk.MustExec("insert local_temporary_admin_test (c1, c2) values (1,1), (2,2), (3,3);") - _, err := tk.Exec("admin check table local_temporary_admin_test;") + err := tk.ExecToErr("admin check table local_temporary_admin_test;") require.EqualError(t, err, core.ErrOptOnTemporaryTable.GenWithStackByArgs("admin check table").Error()) tk.MustExec("drop table if exists temporary_admin_test;") diff --git a/executor/autoidtest/autoid_test.go b/executor/autoidtest/autoid_test.go index d191e52e38cdf..7823a7488bf98 100644 --- a/executor/autoidtest/autoid_test.go +++ b/executor/autoidtest/autoid_test.go @@ -734,8 +734,7 @@ func TestAlterTableAutoIDCache(t *testing.T) { // Note that auto_id_cache=1 use a different implementation, switch between them is not allowed. // TODO: relax this restriction and update the test case. - _, err = tk.Exec("alter table t_473 auto_id_cache = 1") - require.Error(t, err) + tk.MustExecToErr("alter table t_473 auto_id_cache = 1") } func TestMockAutoIDServiceError(t *testing.T) { diff --git a/executor/executor_test.go b/executor/executor_test.go index 0bdfe1199eb61..bd64c39e5a134 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -4623,13 +4623,10 @@ func TestUnion2(t *testing.T) { terr = errors.Cause(err).(*terror.Error) require.Equal(t, errors.ErrCode(mysql.ErrWrongUsage), terr.Code()) - _, err = tk.Exec("(select a from t order by a) union all select a from t limit 1 union all select a from t limit 1") - require.Truef(t, terror.ErrorEqual(err, plannercore.ErrWrongUsage), "err %v", err) + tk.MustGetDBError("(select a from t order by a) union all select a from t limit 1 union all select a from t limit 1", plannercore.ErrWrongUsage) - _, err = tk.Exec("(select a from t limit 1) union all select a from t limit 1") - require.NoError(t, err) - _, err = tk.Exec("(select a from t order by a) union all select a from t order by a") - require.NoError(t, err) + tk.MustExec("(select a from t limit 1) union all select a from t limit 1") + tk.MustExec("(select a from t order by a) union all select a from t order by a") tk.MustExec("drop table if exists t") tk.MustExec("create table t(a int)") @@ -4700,8 +4697,8 @@ func TestUnion2(t *testing.T) { tk.MustExec("insert into t2 values(3,'c'),(4,'d'),(5,'f'),(6,'e')") tk.MustExec("analyze table t1") tk.MustExec("analyze table t2") - _, err = tk.Exec("(select a,b from t1 limit 2) union all (select a,b from t2 order by a limit 1) order by t1.b") - require.Equal(t, "[planner:1250]Table 't1' from one of the SELECTs cannot be used in global ORDER clause", err.Error()) + tk.MustGetErrMsg("(select a,b from t1 limit 2) union all (select a,b from t2 order by a limit 1) order by t1.b", + "[planner:1250]Table 't1' from one of the SELECTs cannot be used in global ORDER clause") // #issue 9900 tk.MustExec("drop table if exists t") @@ -4855,15 +4852,11 @@ func TestSQLMode(t *testing.T) { tk.MustExec("drop table if exists t") tk.MustExec("create table t (a tinyint not null)") tk.MustExec("set sql_mode = 'STRICT_TRANS_TABLES'") - _, err := tk.Exec("insert t values ()") - require.Error(t, err) - - _, err = tk.Exec("insert t values ('1000')") - require.Error(t, err) + tk.ExecToErr("insert t values ()") + tk.ExecToErr("insert t values ('1000')") tk.MustExec("create table if not exists tdouble (a double(3,2))") - _, err = tk.Exec("insert tdouble values (10.23)") - require.Error(t, err) + tk.ExecToErr("insert tdouble values (10.23)") tk.MustExec("set sql_mode = ''") tk.MustExec("insert t values ()") @@ -4891,8 +4884,7 @@ func TestSQLMode(t *testing.T) { tk2.MustQuery("select * from t2").Check(testkit.Rows("abc")) // session1 is still in strict mode. - _, err = tk.Exec("insert t2 values ('abcd')") - require.Error(t, err) + tk.ExecToErr("insert t2 values ('abcd')") // Restore original global strict mode. tk.MustExec("set @@global.sql_mode = 'STRICT_TRANS_TABLES'") } diff --git a/executor/grant_test.go b/executor/grant_test.go index 517bc716c2a40..3a7720d620673 100644 --- a/executor/grant_test.go +++ b/executor/grant_test.go @@ -99,10 +99,10 @@ func TestGrantDBScope(t *testing.T) { } // Grant in wrong scope. - _, err := tk.Exec(` grant create user on test.* to 'testDB1'@'localhost';`) + err := tk.ExecToErr(` grant create user on test.* to 'testDB1'@'localhost';`) require.True(t, terror.ErrorEqual(err, executor.ErrWrongUsage.GenWithStackByArgs("DB GRANT", "GLOBAL PRIVILEGES"))) - _, err = tk.Exec("GRANT SUPER ON test.* TO 'testDB1'@'localhost';") + err = tk.ExecToErr("GRANT SUPER ON test.* TO 'testDB1'@'localhost';") require.True(t, terror.ErrorEqual(err, executor.ErrWrongUsage.GenWithStackByArgs("DB GRANT", "NON-DB PRIVILEGES"))) } @@ -168,8 +168,8 @@ func TestGrantTableScope(t *testing.T) { require.Greater(t, strings.Index(p, mysql.Priv2SetStr[v]), -1) } - _, err := tk.Exec("GRANT SUPER ON test2 TO 'testTbl1'@'localhost';") - require.EqualError(t, err, "[executor:1144]Illegal GRANT/REVOKE command; please consult the manual to see which privileges can be used") + tk.MustGetErrMsg("GRANT SUPER ON test2 TO 'testTbl1'@'localhost';", + "[executor:1144]Illegal GRANT/REVOKE command; please consult the manual to see which privileges can be used") } func TestGrantColumnScope(t *testing.T) { @@ -213,8 +213,8 @@ func TestGrantColumnScope(t *testing.T) { require.Greater(t, strings.Index(p, mysql.Priv2SetStr[v]), -1) } - _, err := tk.Exec("GRANT SUPER(c2) ON test3 TO 'testCol1'@'localhost';") - require.EqualError(t, err, "[executor:1221]Incorrect usage of COLUMN GRANT and NON-COLUMN PRIVILEGES") + tk.MustGetErrMsg("GRANT SUPER(c2) ON test3 TO 'testCol1'@'localhost';", + "[executor:1221]Incorrect usage of COLUMN GRANT and NON-COLUMN PRIVILEGES") } func TestIssue2456(t *testing.T) { diff --git a/executor/seqtest/seq_executor_test.go b/executor/seqtest/seq_executor_test.go index 849ad5cdbfa37..6c042340740e8 100644 --- a/executor/seqtest/seq_executor_test.go +++ b/executor/seqtest/seq_executor_test.go @@ -900,7 +900,7 @@ func TestPrepareMaxParamCountCheck(t *testing.T) { require.NoError(t, err) bigSQL, bigParams := generateBatchSQL(math.MaxUint16 + 2) - _, err = tk.Exec(bigSQL, bigParams...) + err = tk.ExecToErr(bigSQL, bigParams...) require.Error(t, err) require.EqualError(t, err, "[executor:1390]Prepared statement contains too many placeholders") } @@ -987,16 +987,12 @@ func TestBatchInsertDelete(t *testing.T) { // Test tidb_batch_insert could not work if enable-batch-dml is disabled. tk.MustExec("set @@session.tidb_batch_insert=1;") - _, err = tk.Exec("insert into batch_insert (c) select * from batch_insert;") - require.Error(t, err) - require.True(t, kv.ErrTxnTooLarge.Equal(err)) + tk.MustGetErrCode("insert into batch_insert (c) select * from batch_insert;", errno.ErrTxnTooLarge) tk.MustExec("set @@session.tidb_batch_insert=0;") // for on duplicate key - _, err = tk.Exec(`insert into batch_insert_on_duplicate select * from batch_insert_on_duplicate as tt - on duplicate key update batch_insert_on_duplicate.id=batch_insert_on_duplicate.id+1000;`) - require.Error(t, err) - require.Truef(t, kv.ErrTxnTooLarge.Equal(err), "%v", err) + tk.MustGetErrCode(`insert into batch_insert_on_duplicate select * from batch_insert_on_duplicate as tt + on duplicate key update batch_insert_on_duplicate.id=batch_insert_on_duplicate.id+1000;`, errno.ErrTxnTooLarge) r = tk.MustQuery("select count(*) from batch_insert;") r.Check(testkit.Rows("320")) @@ -1022,17 +1018,14 @@ func TestBatchInsertDelete(t *testing.T) { tk.MustExec("set @@session.tidb_dml_batch_size=50;") // for on duplicate key - _, err = tk.Exec(`insert into batch_insert_on_duplicate select * from batch_insert_on_duplicate as tt + tk.MustExec(`insert into batch_insert_on_duplicate select * from batch_insert_on_duplicate as tt on duplicate key update batch_insert_on_duplicate.id=batch_insert_on_duplicate.id+1000;`) - require.NoError(t, err) r = tk.MustQuery("select count(*) from batch_insert_on_duplicate;") r.Check(testkit.Rows("320")) // Disable BachInsert mode in transition. tk.MustExec("begin;") - _, err = tk.Exec("insert into batch_insert (c) select * from batch_insert;") - require.Error(t, err) - require.True(t, kv.ErrTxnTooLarge.Equal(err)) + tk.MustGetErrCode("insert into batch_insert (c) select * from batch_insert;", errno.ErrTxnTooLarge) tk.MustExec("rollback;") r = tk.MustQuery("select count(*) from batch_insert;") r.Check(testkit.Rows("640")) From 48585a7823e2166bb7c570596ad71b5b6bcdac99 Mon Sep 17 00:00:00 2001 From: Hangjie Mo Date: Mon, 19 Dec 2022 12:50:54 +0800 Subject: [PATCH 4/4] ddl: retry prepare RPC when meets region error (#39834) close pingcap/tidb#39836 --- ddl/BUILD.bazel | 1 + ddl/cluster.go | 28 +++++++++-- tests/realtikvtest/brietest/BUILD.bazel | 1 + tests/realtikvtest/brietest/flashback_test.go | 46 +++++++++++++++++++ 4 files changed, 73 insertions(+), 3 deletions(-) diff --git a/ddl/BUILD.bazel b/ddl/BUILD.bazel index 5d9d554f7b873..dc179250ad4bd 100644 --- a/ddl/BUILD.bazel +++ b/ddl/BUILD.bazel @@ -120,6 +120,7 @@ go_library( "@com_github_ngaut_pools//:pools", "@com_github_pingcap_errors//:errors", "@com_github_pingcap_failpoint//:failpoint", + "@com_github_pingcap_kvproto//pkg/errorpb", "@com_github_pingcap_kvproto//pkg/kvrpcpb", "@com_github_pingcap_kvproto//pkg/metapb", "@com_github_pingcap_log//:log", diff --git a/ddl/cluster.go b/ddl/cluster.go index 96a7cd8544abb..fbcfa9cd8a49f 100644 --- a/ddl/cluster.go +++ b/ddl/cluster.go @@ -24,6 +24,7 @@ import ( "github.com/pingcap/errors" "github.com/pingcap/failpoint" + "github.com/pingcap/kvproto/pkg/errorpb" "github.com/pingcap/kvproto/pkg/kvrpcpb" "github.com/pingcap/tidb/ddl/util" "github.com/pingcap/tidb/domain/infosync" @@ -324,15 +325,36 @@ func SendPrepareFlashbackToVersionRPC( if err != nil { return taskStat, err } + failpoint.Inject("mockPrepareMeetsEpochNotMatch", func(val failpoint.Value) { + if val.(bool) && bo.ErrorsNum() == 0 { + regionErr = &errorpb.Error{ + Message: "stale epoch", + EpochNotMatch: &errorpb.EpochNotMatch{}, + } + } + }) if regionErr != nil { - return taskStat, errors.Errorf(regionErr.String()) + err = bo.Backoff(tikv.BoRegionMiss(), errors.New(regionErr.String())) + if err != nil { + return taskStat, err + } + continue } if resp.Resp == nil { - return taskStat, errors.Errorf("prepare flashback missing resp body") + logutil.BgLogger().Warn("prepare flashback miss resp body", zap.Uint64("region_id", loc.Region.GetID())) + err = bo.Backoff(tikv.BoTiKVRPC(), errors.New("prepare flashback rpc miss resp body")) + if err != nil { + return taskStat, err + } + continue } prepareFlashbackToVersionResp := resp.Resp.(*kvrpcpb.PrepareFlashbackToVersionResponse) if err := prepareFlashbackToVersionResp.GetError(); err != "" { - return taskStat, errors.Errorf(err) + boErr := bo.Backoff(tikv.BoTiKVRPC(), errors.New(err)) + if boErr != nil { + return taskStat, boErr + } + continue } taskStat.CompletedRegions++ if isLast { diff --git a/tests/realtikvtest/brietest/BUILD.bazel b/tests/realtikvtest/brietest/BUILD.bazel index 49ea32406c7d6..62de71ea3b77d 100644 --- a/tests/realtikvtest/brietest/BUILD.bazel +++ b/tests/realtikvtest/brietest/BUILD.bazel @@ -14,6 +14,7 @@ go_test( deps = [ "//config", "//ddl/util", + "//parser/model", "//parser/mysql", "//sessionctx/binloginfo", "//store/mockstore/mockcopr", diff --git a/tests/realtikvtest/brietest/flashback_test.go b/tests/realtikvtest/brietest/flashback_test.go index 322359fff411a..470a62fb90d93 100644 --- a/tests/realtikvtest/brietest/flashback_test.go +++ b/tests/realtikvtest/brietest/flashback_test.go @@ -22,6 +22,7 @@ import ( "github.com/pingcap/failpoint" ddlutil "github.com/pingcap/tidb/ddl/util" + "github.com/pingcap/tidb/parser/model" "github.com/pingcap/tidb/testkit" "github.com/pingcap/tidb/tests/realtikvtest" "github.com/stretchr/testify/require" @@ -90,3 +91,48 @@ func TestFlashback(t *testing.T) { require.NoError(t, failpoint.Disable("tikvclient/injectSafeTS")) } } + +func TestPrepareFlashbackFailed(t *testing.T) { + if *realtikvtest.WithRealTiKV { + store := realtikvtest.CreateMockStoreAndSetup(t) + + tk := testkit.NewTestKit(t, store) + + timeBeforeDrop, _, safePointSQL, resetGC := MockGC(tk) + defer resetGC() + + tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + tk.MustExec("create table t(a int, index i(a))") + tk.MustExec("insert t values (1), (2), (3)") + + time.Sleep(1 * time.Second) + + ts, err := tk.Session().GetStore().GetOracle().GetTimestamp(context.Background(), &oracle.Option{}) + require.NoError(t, err) + + injectSafeTS := oracle.GoTimeToTS(oracle.GetTimeFromTS(ts).Add(100 * time.Second)) + require.NoError(t, failpoint.Enable("tikvclient/injectSafeTS", + fmt.Sprintf("return(%v)", injectSafeTS))) + require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/expression/injectSafeTS", + fmt.Sprintf("return(%v)", injectSafeTS))) + require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/ddl/mockPrepareMeetsEpochNotMatch", `return(true)`)) + + tk.MustExec("insert t values (4), (5), (6)") + tk.MustExec(fmt.Sprintf("flashback cluster to timestamp '%s'", oracle.GetTimeFromTS(ts))) + + tk.MustExec("admin check table t") + require.Equal(t, tk.MustQuery("select max(a) from t").Rows()[0][0], "3") + require.Equal(t, tk.MustQuery("select max(a) from t use index(i)").Rows()[0][0], "3") + + jobMeta := tk.MustQuery("select job_meta from mysql.tidb_ddl_history order by job_id desc limit 1").Rows()[0][0].(string) + job := model.Job{} + require.NoError(t, job.Decode([]byte(jobMeta))) + require.Equal(t, job.ErrorCount, int64(0)) + + require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/expression/injectSafeTS")) + require.NoError(t, failpoint.Disable("tikvclient/injectSafeTS")) + require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/ddl/mockPrepareMeetsEpochNotMatch")) + } +}