From dedbde0f2d2eec0d411c0b0164975a6e5836436b Mon Sep 17 00:00:00 2001 From: Lei Zhao Date: Thu, 12 Aug 2021 17:31:16 +0800 Subject: [PATCH 01/22] lock_resolver: avoid pessimistic transactions using resolveLocksForWrite (#25974) --- store/tikv/async_commit_test.go | 22 ++++++++++++++++++++++ store/tikv/lock_resolver.go | 6 ++++-- store/tikv/prewrite.go | 2 +- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/store/tikv/async_commit_test.go b/store/tikv/async_commit_test.go index af8efaa74a656..6f22359644a36 100644 --- a/store/tikv/async_commit_test.go +++ b/store/tikv/async_commit_test.go @@ -168,6 +168,7 @@ func (s *testAsyncCommitSuite) lockKeysWithAsyncCommit(c *C, keys, values [][]by tpc, err := newTwoPhaseCommitterWithInit(txn, 0) c.Assert(err, IsNil) tpc.primaryKey = primaryKey + tpc.setAsyncCommit(true) ctx := context.Background() err = tpc.prewriteMutations(NewBackofferWithVars(ctx, PrewriteMaxBackoff, nil), tpc.mutations) @@ -529,3 +530,24 @@ func (m *mockResolveClient) SendRequest(ctx context.Context, addr string, req *t func (m *mockResolveClient) Close() error { return m.inner.Close() } + +// TestPessimisticTxnResolveAsyncCommitLock tests that pessimistic transactions resolve non-expired async-commit locks during the prewrite phase. +// Pessimistic transactions will resolve locks immediately during the prewrite phase because of the special logic for handling non-pessimistic lock conflict. +// However, async-commit locks can't be resolved until they expire. This test covers it. +func (s *testAsyncCommitSuite) TestPessimisticTxnResolveAsyncCommitLock(c *C) { + ctx := context.Background() + k := []byte("k") + + txn, err := s.store.Begin() + c.Assert(err, IsNil) + txn.SetOption(kv.Pessimistic, true) + err = txn.LockKeys(ctx, &kv.LockCtx{ForUpdateTS: txn.StartTS()}, []byte("k1")) + c.Assert(err, IsNil) + + // Lock the key with a async-commit lock. + s.lockKeysWithAsyncCommit(c, [][]byte{}, [][]byte{}, k, k, false) + + txn.Set(k, k) + err = txn.Commit(context.Background()) + c.Assert(err, IsNil) +} diff --git a/store/tikv/lock_resolver.go b/store/tikv/lock_resolver.go index 9fb2b2fa3aaa3..e388bca8be9bb 100644 --- a/store/tikv/lock_resolver.go +++ b/store/tikv/lock_resolver.go @@ -430,8 +430,10 @@ func (lr *LockResolver) resolveLocks(bo *Backoffer, callerStartTS uint64, locks return msBeforeTxnExpired.value(), pushed, nil } -func (lr *LockResolver) resolveLocksForWrite(bo *Backoffer, callerStartTS uint64, locks []*Lock) (int64, error) { - msBeforeTxnExpired, _, err := lr.resolveLocks(bo, callerStartTS, locks, true, false) +func (lr *LockResolver) resolveLocksForWrite(bo *Backoffer, callerStartTS, callerForUpdateTS uint64, locks []*Lock) (int64, error) { + // The forWrite parameter is only useful for optimistic transactions which can avoid deadlock between large transactions, + // so only use forWrite if the callerForUpdateTS is zero. + msBeforeTxnExpired, _, err := lr.resolveLocks(bo, callerStartTS, locks, callerForUpdateTS == 0, false) return msBeforeTxnExpired, err } diff --git a/store/tikv/prewrite.go b/store/tikv/prewrite.go index 4cf92cf40689d..e4e55f04d75f6 100644 --- a/store/tikv/prewrite.go +++ b/store/tikv/prewrite.go @@ -261,7 +261,7 @@ func (action actionPrewrite) handleSingleBatch(c *twoPhaseCommitter, bo *Backoff locks = append(locks, lock) } start := time.Now() - msBeforeExpired, err := c.store.lockResolver.resolveLocksForWrite(bo, c.startTS, locks) + msBeforeExpired, err := c.store.lockResolver.resolveLocksForWrite(bo, c.startTS, c.forUpdateTS, locks) if err != nil { return errors.Trace(err) } From f7eaba6d86ed8298057133291b19346a459f0ea9 Mon Sep 17 00:00:00 2001 From: Lei Zhao Date: Thu, 12 Aug 2021 19:01:18 +0800 Subject: [PATCH 02/22] store/tikv: fix backoff panic when resolving async-commit locks (#25863) --- store/tikv/lock_resolver.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/store/tikv/lock_resolver.go b/store/tikv/lock_resolver.go index e388bca8be9bb..e1f812154b462 100644 --- a/store/tikv/lock_resolver.go +++ b/store/tikv/lock_resolver.go @@ -783,8 +783,10 @@ func (lr *LockResolver) resolveLockAsync(bo *Backoffer, l *Lock, status TxnStatu for region, locks := range keysByRegion { curLocks := locks curRegion := region + resolveBo, cancel := bo.Fork() + defer cancel() go func() { - errChan <- lr.resolveRegionLocks(bo, l, curRegion, curLocks, status) + errChan <- lr.resolveRegionLocks(resolveBo, l, curRegion, curLocks, status) }() } @@ -819,11 +821,11 @@ func (lr *LockResolver) checkAllSecondaries(bo *Backoffer, l *Lock, status *TxnS } errChan := make(chan error, len(regions)) - checkBo, cancel := bo.Fork() - defer cancel() for regionID, keys := range regions { curRegionID := regionID curKeys := keys + checkBo, cancel := bo.Fork() + defer cancel() go func() { errChan <- lr.checkSecondaries(checkBo, l.TxnID, curKeys, curRegionID, &shared) From 0e8957ede76ac3dd7c6bfdd1b1264c15cd6663be Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Thu, 12 Aug 2021 23:13:17 +0800 Subject: [PATCH 03/22] expression: fix approx_percent panic on bit column (#23687) (#23703) --- executor/aggfuncs/builder.go | 6 +++++- expression/integration_test.go | 9 +++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/executor/aggfuncs/builder.go b/executor/aggfuncs/builder.go index 29f079823e66f..3fa256d6b3974 100644 --- a/executor/aggfuncs/builder.go +++ b/executor/aggfuncs/builder.go @@ -155,9 +155,13 @@ func buildApproxPercentile(sctx sessionctx.Context, aggFuncDesc *aggregation.Agg base := basePercentile{percent: int(percent), baseAggFunc: baseAggFunc{args: aggFuncDesc.Args, ordinal: ordinal}} + evalType := aggFuncDesc.Args[0].GetType().EvalType() + if aggFuncDesc.Args[0].GetType().Tp == mysql.TypeBit { + evalType = types.ETString // same as other aggregate function + } switch aggFuncDesc.Mode { case aggregation.CompleteMode, aggregation.Partial1Mode, aggregation.FinalMode: - switch aggFuncDesc.Args[0].GetType().EvalType() { + switch evalType { case types.ETInt: return &percentileOriginal4Int{base} case types.ETReal: diff --git a/expression/integration_test.go b/expression/integration_test.go index 5143e6702e67a..e5dfe15894c0f 100644 --- a/expression/integration_test.go +++ b/expression/integration_test.go @@ -8991,6 +8991,15 @@ func (s *testIntegrationSerialSuite) TestCollationForBinaryLiteral(c *C) { tk.MustExec("drop table t") } +func (s *testIntegrationSuite) TestApproximatePercentile(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + tk.MustExec("create table t (a bit(10))") + tk.MustExec("insert into t values(b'1111')") + tk.MustQuery("select approx_percentile(a, 10) from t").Check(testkit.Rows("")) +} + func (s *testIntegrationSuite) TestIssue23623(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") From 4f9b0b6a77f31e3d9c58311a440e66cd22227a8d Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Thu, 12 Aug 2021 23:27:17 +0800 Subject: [PATCH 04/22] config: typo fix for `distinct-agg-push-down` (#24011) (#24094) --- config/config.go | 2 +- config/config_test.go | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/config/config.go b/config/config.go index a1b2b4938e92a..16cfa4ff93237 100644 --- a/config/config.go +++ b/config/config.go @@ -416,7 +416,7 @@ type Performance struct { TCPKeepAlive bool `toml:"tcp-keep-alive" json:"tcp-keep-alive"` CrossJoin bool `toml:"cross-join" json:"cross-join"` RunAutoAnalyze bool `toml:"run-auto-analyze" json:"run-auto-analyze"` - DistinctAggPushDown bool `toml:"distinct-agg-push-down" json:"agg-push-down-join"` + DistinctAggPushDown bool `toml:"distinct-agg-push-down" json:"distinct-agg-push-down"` CommitterConcurrency int `toml:"committer-concurrency" json:"committer-concurrency"` MaxTxnTTL uint64 `toml:"max-txn-ttl" json:"max-txn-ttl"` MemProfileInterval string `toml:"mem-profile-interval" json:"mem-profile-interval"` diff --git a/config/config_test.go b/config/config_test.go index 39bc1448b61a5..7201f5fb42f1e 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -18,6 +18,7 @@ import ( "os" "os/user" "path/filepath" + "reflect" "runtime" "testing" @@ -421,6 +422,14 @@ xkNuJ2BlEGkwWLiRbKy1lNBBFUXKuhh3L/EIY10WTnr3TQzeL6H1 // is recycled when the reference count drops to 0. c.Assert(os.Remove(certFile), IsNil) c.Assert(os.Remove(keyFile), IsNil) + + // test for config `toml` and `json` tag names + c1 := Config{} + st := reflect.TypeOf(c1) + for i := 0; i < st.NumField(); i++ { + field := st.Field(i) + c.Assert(field.Tag.Get("toml"), Equals, field.Tag.Get("json")) + } } func (s *testConfigSuite) TestOOMActionValid(c *C) { From 899c2f05f67f67e3b1c191e7b14ea9591338b256 Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Thu, 12 Aug 2021 23:39:17 +0800 Subject: [PATCH 05/22] expression: fix wrong flen infer for bit constant (#23867) (#24267) --- expression/integration_test.go | 5 +++++ expression/typeinfer_test.go | 6 +++--- types/field_type.go | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/expression/integration_test.go b/expression/integration_test.go index e5dfe15894c0f..fd90a59ac5fb8 100644 --- a/expression/integration_test.go +++ b/expression/integration_test.go @@ -4618,6 +4618,11 @@ func (s *testIntegrationSuite) TestIssues(c *C) { tk.MustExec(`insert into t2 values(1,"1111"),(2,"2222"),(3,"3333"),(4,"4444"),(5,"5555"),(6,"6666"),(7,"7777"),(8,"8888"),(9,"9999"),(10,"0000")`) tk.MustQuery(`select (@j := case when substr(t2.b,1,3)=@i then 1 else @j+1 end) from t2, (select @j := 0, @i := "0") tt limit 10`).Check(testkit.Rows( "1", "2", "3", "4", "5", "6", "7", "8", "9", "10")) + + // for issue #23479 + tk.MustQuery("select b'10000000' DIV 10").Check(testkit.Rows("12")) + tk.MustQuery("select cast(b'10000000' as unsigned) / 10").Check(testkit.Rows("12.8000")) + tk.MustQuery("select b'10000000' / 10").Check(testkit.Rows("12.8000")) } func (s *testIntegrationSuite) TestInPredicate4UnsignedInt(c *C) { diff --git a/expression/typeinfer_test.go b/expression/typeinfer_test.go index 321598891d374..10e146d6189ab 100644 --- a/expression/typeinfer_test.go +++ b/expression/typeinfer_test.go @@ -170,9 +170,9 @@ func (s *testInferTypeSuite) createTestCase4Constants() []typeInferTestCase { {"'1234'", mysql.TypeVarString, charset.CharsetUTF8MB4, 0 | mysql.NotNullFlag, 4, types.UnspecifiedLength}, {"_utf8'1234'", mysql.TypeVarString, charset.CharsetUTF8, 0 | mysql.NotNullFlag, 4, types.UnspecifiedLength}, {"_binary'1234'", mysql.TypeVarString, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag, 4, types.UnspecifiedLength}, - {"b'0001'", mysql.TypeVarString, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag, 1, 0}, - {"b'000100001'", mysql.TypeVarString, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag, 2, 0}, - {"b'0000000000010000'", mysql.TypeVarString, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag, 2, 0}, + {"b'0001'", mysql.TypeVarString, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag, 3, 0}, + {"b'000100001'", mysql.TypeVarString, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag, 6, 0}, + {"b'0000000000010000'", mysql.TypeVarString, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag, 6, 0}, {"x'10'", mysql.TypeVarString, charset.CharsetBin, mysql.BinaryFlag | mysql.UnsignedFlag | mysql.NotNullFlag, 3, 0}, {"x'ff10'", mysql.TypeVarString, charset.CharsetBin, mysql.BinaryFlag | mysql.UnsignedFlag | mysql.NotNullFlag, 6, 0}, {"x'0000000000000000ff10'", mysql.TypeVarString, charset.CharsetBin, mysql.BinaryFlag | mysql.UnsignedFlag | mysql.NotNullFlag, 30, 0}, diff --git a/types/field_type.go b/types/field_type.go index d73b05cbf8365..025b01ba8badf 100644 --- a/types/field_type.go +++ b/types/field_type.go @@ -250,7 +250,7 @@ func DefaultTypeForValue(value interface{}, tp *FieldType, char string, collate SetBinChsClnFlag(tp) case BitLiteral: tp.Tp = mysql.TypeVarString - tp.Flen = len(x) + tp.Flen = len(x) * 3 tp.Decimal = 0 SetBinChsClnFlag(tp) case HexLiteral: From 8a51f598aa5fd629ec946fb2785ff3236bb7d4d0 Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Fri, 13 Aug 2021 00:13:17 +0800 Subject: [PATCH 06/22] privilege: fix RequestVerificationWithUser use of default roles (#24442) (#24532) --- privilege/privileges/privileges.go | 3 ++- privilege/privileges/privileges_test.go | 28 +++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/privilege/privileges/privileges.go b/privilege/privileges/privileges.go index a4edaca2782a1..ff583c94b6d3a 100644 --- a/privilege/privileges/privileges.go +++ b/privilege/privileges/privileges.go @@ -99,7 +99,8 @@ func (p *UserPrivileges) RequestVerificationWithUser(db, table, column string, p } mysqlPriv := p.Handle.Get() - return mysqlPriv.RequestVerification(nil, user.Username, user.Hostname, db, table, column, priv) + roles := mysqlPriv.getDefaultRoles(user.Username, user.Hostname) + return mysqlPriv.RequestVerification(roles, user.Username, user.Hostname, db, table, column, priv) } // GetEncodedPassword implements the Manager interface. diff --git a/privilege/privileges/privileges_test.go b/privilege/privileges/privileges_test.go index 7a8c0e14795d9..fd6e644e068a4 100644 --- a/privilege/privileges/privileges_test.go +++ b/privilege/privileges/privileges_test.go @@ -1257,3 +1257,31 @@ func newSession(c *C, store kv.Storage, dbName string) session.Session { mustExec(c, se, "use "+dbName) return se } + +// TestViewDefiner tests that default roles are correctly applied in the algorithm definer +// See: https://github.com/pingcap/tidb/issues/24414 +func (s *testPrivilegeSuite) TestViewDefiner(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("CREATE DATABASE issue24414") + tk.MustExec("USE issue24414") + tk.MustExec(`create table table1( + col1 int, + col2 int, + col3 int + )`) + tk.MustExec(`insert into table1 values (1,1,1),(2,2,2)`) + tk.MustExec(`CREATE ROLE 'ACL-mobius-admin'`) + tk.MustExec(`CREATE USER 'mobius-admin'`) + tk.MustExec(`CREATE USER 'mobius-admin-no-role'`) + tk.MustExec(`GRANT Select,Insert,Update,Delete,Create,Drop,Alter,Index,Create View,Show View ON issue24414.* TO 'ACL-mobius-admin'@'%'`) + tk.MustExec(`GRANT Select,Insert,Update,Delete,Create,Drop,Alter,Index,Create View,Show View ON issue24414.* TO 'mobius-admin-no-role'@'%'`) + tk.MustExec(`GRANT 'ACL-mobius-admin'@'%' to 'mobius-admin'@'%'`) + tk.MustExec(`SET DEFAULT ROLE ALL TO 'mobius-admin'`) + // create tables + tk.MustExec(`CREATE ALGORITHM = UNDEFINED DEFINER = 'mobius-admin'@'127.0.0.1' SQL SECURITY DEFINER VIEW test_view (col1 , col2 , col3) AS SELECT * from table1`) + tk.MustExec(`CREATE ALGORITHM = UNDEFINED DEFINER = 'mobius-admin-no-role'@'127.0.0.1' SQL SECURITY DEFINER VIEW test_view2 (col1 , col2 , col3) AS SELECT * from table1`) + + // all examples should work + tk.MustExec("select * from test_view") + tk.MustExec("select * from test_view2") +} From 85b4864c5afd8579a3865431fb7dd0bc987b3383 Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Fri, 13 Aug 2021 00:25:17 +0800 Subject: [PATCH 07/22] executor: accelerate TestVectorizedMergeJoin and TestVectorizedShuffleMergeJoin (#24177) (#24278) --- executor/merge_join_test.go | 176 +++++++++++++++++++++--------------- 1 file changed, 105 insertions(+), 71 deletions(-) diff --git a/executor/merge_join_test.go b/executor/merge_join_test.go index d3ce3efeadd45..94b9c33a5a9d7 100644 --- a/executor/merge_join_test.go +++ b/executor/merge_join_test.go @@ -17,6 +17,7 @@ import ( "bytes" "fmt" "math/rand" + "strconv" "strings" . "github.com/pingcap/check" @@ -728,20 +729,25 @@ func (s *testSuite2) TestMergeJoinDifferentTypes(c *C) { func (s *testSuiteJoin3) TestVectorizedMergeJoin(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") - tk.MustExec("drop table if exists t1") - tk.MustExec("drop table if exists t2") - tk.MustExec("create table t1 (a int, b int)") - tk.MustExec("create table t2 (a int, b int)") - runTest := func(t1, t2 []int) { - tk.MustExec("truncate table t1") - tk.MustExec("truncate table t2") - insert := func(tName string, ts []int) { + existTableMap := make(map[string]struct{}) + runTest := func(ts1, ts2 []int) { + getTable := func(prefix string, ts []int) string { + tableName := prefix + for _, i := range ts { + tableName = tableName + "_" + strconv.Itoa(i) + } + if _, ok := existTableMap[tableName]; ok { + return tableName + } + tk.MustExec(fmt.Sprintf("drop table if exists %s", tableName)) + tk.MustExec(fmt.Sprintf("create table %s (a int, b int)", tableName)) + existTableMap[tableName] = struct{}{} for i, n := range ts { if n == 0 { continue } var buf bytes.Buffer - buf.WriteString(fmt.Sprintf("insert into %v values ", tName)) + buf.WriteString(fmt.Sprintf("insert into %v values ", tableName)) for j := 0; j < n; j++ { if j > 0 { buf.WriteString(", ") @@ -750,33 +756,45 @@ func (s *testSuiteJoin3) TestVectorizedMergeJoin(c *C) { } tk.MustExec(buf.String()) } + return tableName } - insert("t1", t1) - insert("t2", t2) - - tk.MustQuery("explain format = 'brief' select /*+ TIDB_SMJ(t1, t2) */ * from t1, t2 where t1.a=t2.a and t1.b>5 and t2.b<5").Check(testkit.Rows( - `MergeJoin 4150.01 root inner join, left key:test.t1.a, right key:test.t2.a`, - `├─Sort(Build) 3320.01 root test.t2.a`, - `│ └─TableReader 3320.01 root data:Selection`, - `│ └─Selection 3320.01 cop[tikv] lt(test.t2.b, 5), not(isnull(test.t2.a))`, - `│ └─TableFullScan 10000.00 cop[tikv] table:t2 keep order:false, stats:pseudo`, - `└─Sort(Probe) 3330.00 root test.t1.a`, - ` └─TableReader 3330.00 root data:Selection`, - ` └─Selection 3330.00 cop[tikv] gt(test.t1.b, 5), not(isnull(test.t1.a))`, - ` └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo`, + t1 := getTable("t", ts1) + t2 := getTable("t", ts2) + if t1 == t2 { + t2 = getTable("t2", ts2) + } + + tk.MustQuery(fmt.Sprintf("explain format = 'brief' select /*+ TIDB_SMJ(%s, %s) */ * from %s, %s where %s.a=%s.a and %s.b>5 and %s.b<5", + t1, t2, t1, t2, t1, t2, t1, t2, + )).Check(testkit.Rows( + fmt.Sprintf(`MergeJoin 4150.01 root inner join, left key:test.%s.a, right key:test.%s.a`, t1, t2), + fmt.Sprintf(`├─Sort(Build) 3320.01 root test.%s.a`, t2), + fmt.Sprintf(`│ └─TableReader 3320.01 root data:Selection`), + fmt.Sprintf(`│ └─Selection 3320.01 cop[tikv] lt(test.%s.b, 5), not(isnull(test.%s.a))`, t2, t2), + fmt.Sprintf(`│ └─TableFullScan 10000.00 cop[tikv] table:%s keep order:false, stats:pseudo`, t2), + fmt.Sprintf(`└─Sort(Probe) 3330.00 root test.%s.a`, t1), + fmt.Sprintf(` └─TableReader 3330.00 root data:Selection`), + fmt.Sprintf(` └─Selection 3330.00 cop[tikv] gt(test.%s.b, 5), not(isnull(test.%s.a))`, t1, t1), + fmt.Sprintf(` └─TableFullScan 10000.00 cop[tikv] table:%s keep order:false, stats:pseudo`, t1), )) - tk.MustQuery("explain format = 'brief' select /*+ TIDB_HJ(t1, t2) */ * from t1, t2 where t1.a=t2.a and t1.b>5 and t2.b<5").Check(testkit.Rows( - `HashJoin 4150.01 root inner join, equal:[eq(test.t1.a, test.t2.a)]`, - `├─TableReader(Build) 3320.01 root data:Selection`, - `│ └─Selection 3320.01 cop[tikv] lt(test.t2.b, 5), not(isnull(test.t2.a))`, - `│ └─TableFullScan 10000.00 cop[tikv] table:t2 keep order:false, stats:pseudo`, - `└─TableReader(Probe) 3330.00 root data:Selection`, - ` └─Selection 3330.00 cop[tikv] gt(test.t1.b, 5), not(isnull(test.t1.a))`, - ` └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo`, + tk.MustQuery(fmt.Sprintf("explain format = 'brief' select /*+ TIDB_HJ(%s, %s) */ * from %s, %s where %s.a=%s.a and %s.b>5 and %s.b<5", + t1, t2, t1, t2, t1, t2, t1, t2, + )).Check(testkit.Rows( + fmt.Sprintf(`HashJoin 4150.01 root inner join, equal:[eq(test.%s.a, test.%s.a)]`, t1, t2), + fmt.Sprintf(`├─TableReader(Build) 3320.01 root data:Selection`), + fmt.Sprintf(`│ └─Selection 3320.01 cop[tikv] lt(test.%s.b, 5), not(isnull(test.%s.a))`, t2, t2), + fmt.Sprintf(`│ └─TableFullScan 10000.00 cop[tikv] table:%s keep order:false, stats:pseudo`, t2), + fmt.Sprintf(`└─TableReader(Probe) 3330.00 root data:Selection`), + fmt.Sprintf(` └─Selection 3330.00 cop[tikv] gt(test.%s.b, 5), not(isnull(test.%s.a))`, t1, t1), + fmt.Sprintf(` └─TableFullScan 10000.00 cop[tikv] table:%s keep order:false, stats:pseudo`, t1), )) - r1 := tk.MustQuery("select /*+ TIDB_SMJ(t1, t2) */ * from t1, t2 where t1.a=t2.a and t1.b>5 and t2.b<5").Sort() - r2 := tk.MustQuery("select /*+ TIDB_HJ(t1, t2) */ * from t1, t2 where t1.a=t2.a and t1.b>5 and t2.b<5").Sort() + r1 := tk.MustQuery(fmt.Sprintf("select /*+ TIDB_SMJ(%s, %s) */ * from %s, %s where %s.a=%s.a and %s.b>5 and %s.b<5", + t1, t2, t1, t2, t1, t2, t1, t2, + )).Sort() + r2 := tk.MustQuery(fmt.Sprintf("select /*+ TIDB_HJ(%s, %s) */ * from %s, %s where %s.a=%s.a and %s.b>5 and %s.b<5", + t1, t2, t1, t2, t1, t2, t1, t2, + )).Sort() c.Assert(len(r1.Rows()), Equals, len(r2.Rows())) i := 0 @@ -806,10 +824,7 @@ func (s *testSuiteJoin3) TestVectorizedMergeJoin(c *C) { {[]int{chunkSize - 1}, []int{chunkSize - 1}}, {[]int{chunkSize - 1}, []int{chunkSize + 1}}, {[]int{chunkSize}, []int{chunkSize}}, - {[]int{chunkSize}, []int{chunkSize - 1}}, {[]int{chunkSize}, []int{chunkSize + 1}}, - {[]int{chunkSize + 1}, []int{chunkSize}}, - {[]int{chunkSize + 1}, []int{chunkSize - 1}}, {[]int{chunkSize + 1}, []int{chunkSize + 1}}, {[]int{1, 1, 1}, []int{chunkSize + 1, chunkSize*5 + 5, chunkSize - 5}}, {[]int{0, 0, chunkSize}, []int{chunkSize + 1, chunkSize*5 + 5, chunkSize - 5}}, @@ -819,6 +834,10 @@ func (s *testSuiteJoin3) TestVectorizedMergeJoin(c *C) { runTest(ca.t1, ca.t2) runTest(ca.t2, ca.t1) } + fmt.Println(existTableMap) + for tableName := range existTableMap { + tk.MustExec(fmt.Sprintf("drop table if exists %s", tableName)) + } } // TestVectorizedShuffleMergeJoin is used to test vectorized shuffle merge join with some corner cases. @@ -826,20 +845,26 @@ func (s *testSuiteJoin3) TestVectorizedShuffleMergeJoin(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("set @@session.tidb_merge_join_concurrency = 4;") tk.MustExec("use test") - tk.MustExec("drop table if exists t1") - tk.MustExec("drop table if exists t2") - tk.MustExec("create table t1 (a int, b int)") - tk.MustExec("create table t2 (a int, b int)") - runTest := func(t1, t2 []int) { - tk.MustExec("truncate table t1") - tk.MustExec("truncate table t2") - insert := func(tName string, ts []int) { + tk.MustExec("use test") + existTableMap := make(map[string]struct{}) + runTest := func(ts1, ts2 []int) { + getTable := func(prefix string, ts []int) string { + tableName := prefix + for _, i := range ts { + tableName = tableName + "_" + strconv.Itoa(i) + } + if _, ok := existTableMap[tableName]; ok { + return tableName + } + tk.MustExec(fmt.Sprintf("drop table if exists %s", tableName)) + tk.MustExec(fmt.Sprintf("create table %s (a int, b int)", tableName)) + existTableMap[tableName] = struct{}{} for i, n := range ts { if n == 0 { continue } var buf bytes.Buffer - buf.WriteString(fmt.Sprintf("insert into %v values ", tName)) + buf.WriteString(fmt.Sprintf("insert into %v values ", tableName)) for j := 0; j < n; j++ { if j > 0 { buf.WriteString(", ") @@ -848,34 +873,46 @@ func (s *testSuiteJoin3) TestVectorizedShuffleMergeJoin(c *C) { } tk.MustExec(buf.String()) } + return tableName + } + t1 := getTable("t", ts1) + t2 := getTable("t", ts2) + if t1 == t2 { + t2 = getTable("t2", ts2) } - insert("t1", t1) - insert("t2", t2) - - tk.MustQuery("explain format = 'brief' select /*+ TIDB_SMJ(t1, t2) */ * from t1, t2 where t1.a=t2.a and t1.b>5 and t2.b<5").Check(testkit.Rows( - `Shuffle 4150.01 root execution info: concurrency:4, data sources:[TableReader TableReader]`, - `└─MergeJoin 4150.01 root inner join, left key:test.t1.a, right key:test.t2.a`, - ` ├─Sort(Build) 3320.01 root test.t2.a`, - ` │ └─TableReader 3320.01 root data:Selection`, - ` │ └─Selection 3320.01 cop[tikv] lt(test.t2.b, 5), not(isnull(test.t2.a))`, - ` │ └─TableFullScan 10000.00 cop[tikv] table:t2 keep order:false, stats:pseudo`, - ` └─Sort(Probe) 3330.00 root test.t1.a`, - ` └─TableReader 3330.00 root data:Selection`, - ` └─Selection 3330.00 cop[tikv] gt(test.t1.b, 5), not(isnull(test.t1.a))`, - ` └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo`, + + tk.MustQuery(fmt.Sprintf("explain format = 'brief' select /*+ TIDB_SMJ(%s, %s) */ * from %s, %s where %s.a=%s.a and %s.b>5 and %s.b<5", + t1, t2, t1, t2, t1, t2, t1, t2, + )).Check(testkit.Rows( + fmt.Sprintf(`Shuffle 4150.01 root execution info: concurrency:4, data sources:[TableReader TableReader]`), + fmt.Sprintf(`└─MergeJoin 4150.01 root inner join, left key:test.%s.a, right key:test.%s.a`, t1, t2), + fmt.Sprintf(` ├─Sort(Build) 3320.01 root test.%s.a`, t2), + fmt.Sprintf(` │ └─TableReader 3320.01 root data:Selection`), + fmt.Sprintf(` │ └─Selection 3320.01 cop[tikv] lt(test.%s.b, 5), not(isnull(test.%s.a))`, t2, t2), + fmt.Sprintf(` │ └─TableFullScan 10000.00 cop[tikv] table:%s keep order:false, stats:pseudo`, t2), + fmt.Sprintf(` └─Sort(Probe) 3330.00 root test.%s.a`, t1), + fmt.Sprintf(` └─TableReader 3330.00 root data:Selection`), + fmt.Sprintf(` └─Selection 3330.00 cop[tikv] gt(test.%s.b, 5), not(isnull(test.%s.a))`, t1, t1), + fmt.Sprintf(` └─TableFullScan 10000.00 cop[tikv] table:%s keep order:false, stats:pseudo`, t1), )) - tk.MustQuery("explain format = 'brief' select /*+ TIDB_HJ(t1, t2) */ * from t1, t2 where t1.a=t2.a and t1.b>5 and t2.b<5").Check(testkit.Rows( - `HashJoin 4150.01 root inner join, equal:[eq(test.t1.a, test.t2.a)]`, - `├─TableReader(Build) 3320.01 root data:Selection`, - `│ └─Selection 3320.01 cop[tikv] lt(test.t2.b, 5), not(isnull(test.t2.a))`, - `│ └─TableFullScan 10000.00 cop[tikv] table:t2 keep order:false, stats:pseudo`, - `└─TableReader(Probe) 3330.00 root data:Selection`, - ` └─Selection 3330.00 cop[tikv] gt(test.t1.b, 5), not(isnull(test.t1.a))`, - ` └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo`, + tk.MustQuery(fmt.Sprintf("explain format = 'brief' select /*+ TIDB_HJ(%s, %s) */ * from %s, %s where %s.a=%s.a and %s.b>5 and %s.b<5", + t1, t2, t1, t2, t1, t2, t1, t2, + )).Check(testkit.Rows( + fmt.Sprintf(`HashJoin 4150.01 root inner join, equal:[eq(test.%s.a, test.%s.a)]`, t1, t2), + fmt.Sprintf(`├─TableReader(Build) 3320.01 root data:Selection`), + fmt.Sprintf(`│ └─Selection 3320.01 cop[tikv] lt(test.%s.b, 5), not(isnull(test.%s.a))`, t2, t2), + fmt.Sprintf(`│ └─TableFullScan 10000.00 cop[tikv] table:%s keep order:false, stats:pseudo`, t2), + fmt.Sprintf(`└─TableReader(Probe) 3330.00 root data:Selection`), + fmt.Sprintf(` └─Selection 3330.00 cop[tikv] gt(test.%s.b, 5), not(isnull(test.%s.a))`, t1, t1), + fmt.Sprintf(` └─TableFullScan 10000.00 cop[tikv] table:%s keep order:false, stats:pseudo`, t1), )) - r1 := tk.MustQuery("select /*+ TIDB_SMJ(t1, t2) */ * from t1, t2 where t1.a=t2.a and t1.b>5 and t2.b<5").Sort() - r2 := tk.MustQuery("select /*+ TIDB_HJ(t1, t2) */ * from t1, t2 where t1.a=t2.a and t1.b>5 and t2.b<5").Sort() + r1 := tk.MustQuery(fmt.Sprintf("select /*+ TIDB_SMJ(%s, %s) */ * from %s, %s where %s.a=%s.a and %s.b>5 and %s.b<5", + t1, t2, t1, t2, t1, t2, t1, t2, + )).Sort() + r2 := tk.MustQuery(fmt.Sprintf("select /*+ TIDB_HJ(%s, %s) */ * from %s, %s where %s.a=%s.a and %s.b>5 and %s.b<5", + t1, t2, t1, t2, t1, t2, t1, t2, + )).Sort() c.Assert(len(r1.Rows()), Equals, len(r2.Rows())) i := 0 @@ -905,10 +942,7 @@ func (s *testSuiteJoin3) TestVectorizedShuffleMergeJoin(c *C) { {[]int{chunkSize - 1}, []int{chunkSize - 1}}, {[]int{chunkSize - 1}, []int{chunkSize + 1}}, {[]int{chunkSize}, []int{chunkSize}}, - {[]int{chunkSize}, []int{chunkSize - 1}}, {[]int{chunkSize}, []int{chunkSize + 1}}, - {[]int{chunkSize + 1}, []int{chunkSize}}, - {[]int{chunkSize + 1}, []int{chunkSize - 1}}, {[]int{chunkSize + 1}, []int{chunkSize + 1}}, {[]int{1, 1, 1}, []int{chunkSize + 1, chunkSize*5 + 5, chunkSize - 5}}, {[]int{0, 0, chunkSize}, []int{chunkSize + 1, chunkSize*5 + 5, chunkSize - 5}}, From 75894321f5e6e076dd94676b6d29881608e2f627 Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Fri, 13 Aug 2021 00:37:17 +0800 Subject: [PATCH 08/22] expression: fix wrong type infer for agg function when type is null (#24290) (#24354) --- executor/window_test.go | 28 ++++++++++++++++++++++++++++ expression/aggregation/base_func.go | 3 +++ 2 files changed, 31 insertions(+) diff --git a/executor/window_test.go b/executor/window_test.go index 219113cf38c0c..e530aa03f0549 100644 --- a/executor/window_test.go +++ b/executor/window_test.go @@ -402,3 +402,31 @@ func baseTestSlidingWindowFunctions(tk *testkit.TestKit) { result = tk.MustQuery("SELECT sex, MAX(id) OVER (ORDER BY id DESC RANGE BETWEEN 1 PRECEDING and 2 FOLLOWING) FROM t;") result.Check(testkit.Rows(" 11", " 11", "M 5", "F 5", "F 4", "F 3", "M 2")) } + +func (s *testSuite7) TestIssue24264(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("drop table if exists tbl_2") + tk.MustExec("create table tbl_2 ( col_10 char(65) collate utf8mb4_unicode_ci not null , col_11 bigint not null , col_12 datetime not null , col_13 bigint unsigned default 327695751717730004 , col_14 timestamp default '2010-11-18' not null , primary key idx_5 ( col_11,col_13 ) /*T![clustered_index] clustered */ , unique key idx_6 ( col_10,col_11,col_13 ) , unique key idx_7 ( col_14,col_12,col_13 ) )") + tk.MustExec("insert into tbl_2 values ( 'RmF',-5353757041350034197,'1996-01-22',1866803697729291364,'1996-09-11' )") + tk.MustExec("insert into tbl_2 values ( 'xEOGaB',-6602924241498980347,'2019-02-22',8297270320597030697,'1972-04-04' )") + tk.MustExec("insert into tbl_2 values ( 'dvUztqgTPAhLdzgEsV',3316448219481769821,'2034-09-12',937089564901142512,'2030-12-04' )") + tk.MustExec("insert into tbl_2 values ( 'mNoyfbT',-6027094365061219400,'2035-10-10',1752804734961508175,'1992-08-09' )") + tk.MustExec("insert into tbl_2 values ( 'BDPJMhLYXuKB',6823702503458376955,'2015-04-09',737914379167848827,'2026-04-29' )") + tk.MustExec("insert into tbl_2 values ( 'WPiaVfPstGohvHd',1308183537252932688,'2020-05-03',5364104746649397703,'1979-01-28' )") + tk.MustExec("insert into tbl_2 values ( 'lrm',4642935044097656317,'1973-04-29',149081313305673035,'2013-02-03' )") + tk.MustExec("insert into tbl_2 values ( '',-7361040853169906422,'2024-10-22',6308270832310351889,'1981-02-01' )") + tk.MustExec("insert into tbl_2 values ( 'uDANahGcLwpSssabD',2235074865448210231,'1992-10-10',7140606140672586593,'1992-11-25' )") + tk.MustExec("insert into tbl_2 values ( 'TDH',-1911014243756021618,'2013-01-26',2022218243939205750,'1982-04-04' )") + tk.MustQuery("select lead(col_13,1,NULL) over w from tbl_2 window w as (order by col_13)").Check(testkit.Rows( + "737914379167848827", + "937089564901142512", + "1752804734961508175", + "1866803697729291364", + "2022218243939205750", + "5364104746649397703", + "6308270832310351889", + "7140606140672586593", + "8297270320597030697", + "")) +} diff --git a/expression/aggregation/base_func.go b/expression/aggregation/base_func.go index 6abd5e8cfc0aa..9a5ef95d49bb2 100644 --- a/expression/aggregation/base_func.go +++ b/expression/aggregation/base_func.go @@ -411,6 +411,9 @@ func (a *baseFuncDesc) WrapCastForAggArgs(ctx sessionctx.Context) { if i == 1 && (a.Name == ast.WindowFuncLead || a.Name == ast.WindowFuncLag || a.Name == ast.WindowFuncNthValue) { continue } + if a.Args[i].GetType().Tp == mysql.TypeNull { + continue + } a.Args[i] = castFunc(ctx, a.Args[i]) if a.Name != ast.AggFuncAvg && a.Name != ast.AggFuncSum { continue From 210764b6a77634d443b2037afd8f70fb78333a91 Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Fri, 13 Aug 2021 00:53:16 +0800 Subject: [PATCH 09/22] executor: add table name in log (#24666) (#24802) --- executor/distsql.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/executor/distsql.go b/executor/distsql.go index 316697230f2db..27251d77886ad 100644 --- a/executor/distsql.go +++ b/executor/distsql.go @@ -1140,7 +1140,9 @@ func (w *tableWorker) executeTask(ctx context.Context, task *lookupTableTask) er obtainedHandlesMap.Set(handle, true) } - logutil.Logger(ctx).Error("inconsistent index handles", zap.String("index", w.idxLookup.index.Name.O), + logutil.Logger(ctx).Error("inconsistent index handles", + zap.String("table_name", w.idxLookup.index.Table.O), + zap.String("index", w.idxLookup.index.Name.O), zap.Int("index_cnt", handleCnt), zap.Int("table_cnt", len(task.rows)), zap.String("missing_handles", fmt.Sprint(GetLackHandles(task.handles, obtainedHandlesMap))), zap.String("total_handles", fmt.Sprint(task.handles))) From 415477877447bf5274a631636658c3cfdc0f8414 Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Fri, 13 Aug 2021 01:05:17 +0800 Subject: [PATCH 10/22] executor: fix wrong enum key in point get (#24618) (#24772) --- executor/batch_point_get_test.go | 10 ++++++++++ executor/point_get.go | 12 ++++++++++++ 2 files changed, 22 insertions(+) diff --git a/executor/batch_point_get_test.go b/executor/batch_point_get_test.go index 926834dc9281b..8f8c39d4b0eed 100644 --- a/executor/batch_point_get_test.go +++ b/executor/batch_point_get_test.go @@ -156,6 +156,16 @@ func (s *testBatchPointGetSuite) TestIssue18843(c *C) { tk.MustQuery("select * from t18843 where f is null").Check(testkit.Rows("2 ")) } +func (s *testBatchPointGetSuite) TestIssue24562(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("drop table if exists ttt") + tk.MustExec("create table ttt(a enum(\"a\",\"b\",\"c\",\"d\"), primary key(a));") + tk.MustExec("insert into ttt values(1)") + tk.MustQuery("select * from ttt where ttt.a in (\"1\",\"b\")").Check(testkit.Rows()) + tk.MustQuery("select * from ttt where ttt.a in (1,\"b\")").Check(testkit.Rows("a")) +} + func (s *testBatchPointGetSuite) TestBatchPointGetUnsignedHandleWithSort(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") diff --git a/executor/point_get.go b/executor/point_get.go index e87f3ab7bfb01..48c7a377ccf6e 100644 --- a/executor/point_get.go +++ b/executor/point_get.go @@ -441,6 +441,18 @@ func EncodeUniqueIndexValuesForKey(ctx sessionctx.Context, tblInfo *model.TableI var str string str, err = idxVals[i].ToString() idxVals[i].SetString(str, colInfo.FieldType.Collate) + } else if colInfo.Tp == mysql.TypeEnum && (idxVals[i].Kind() == types.KindString || idxVals[i].Kind() == types.KindBytes || idxVals[i].Kind() == types.KindBinaryLiteral) { + var str string + var e types.Enum + str, err = idxVals[i].ToString() + if err != nil { + return nil, kv.ErrNotExist + } + e, err = types.ParseEnumName(colInfo.FieldType.Elems, str, colInfo.FieldType.Collate) + if err != nil { + return nil, kv.ErrNotExist + } + idxVals[i].SetMysqlEnum(e, colInfo.FieldType.Collate) } else { // If a truncated error or an overflow error is thrown when converting the type of `idxVal[i]` to // the type of `colInfo`, the `idxVal` does not exist in the `idxInfo` for sure. From 8af3a7bfd8437bf81df0a61e66ccdbfa84572935 Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Fri, 13 Aug 2021 01:21:17 +0800 Subject: [PATCH 11/22] metrics: Add err label for TiFlashQueryTotalCounter (#25317) (#25327) --- executor/adapter.go | 10 +++++----- metrics/server.go | 2 +- server/conn.go | 2 +- session/session.go | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/executor/adapter.go b/executor/adapter.go index 066a1c81aeb97..573e046817d70 100644 --- a/executor/adapter.go +++ b/executor/adapter.go @@ -821,8 +821,7 @@ func FormatSQL(sql string, pps variable.PreparedParams) stringutil.StringerFunc var ( sessionExecuteRunDurationInternal = metrics.SessionExecuteRunDuration.WithLabelValues(metrics.LblInternal) sessionExecuteRunDurationGeneral = metrics.SessionExecuteRunDuration.WithLabelValues(metrics.LblGeneral) - totalTiFlashQueryFailCounter = metrics.TiFlashQueryTotalCounter.WithLabelValues(metrics.LblError) - totalTiFlashQuerySuccCounter = metrics.TiFlashQueryTotalCounter.WithLabelValues(metrics.LblOK) + totalTiFlashQuerySuccCounter = metrics.TiFlashQueryTotalCounter.WithLabelValues("", metrics.LblOK) ) // FinishExecuteStmt is used to record some information after `ExecStmt` execution finished: @@ -830,7 +829,7 @@ var ( // 2. record summary statement. // 3. record execute duration metric. // 4. update the `PrevStmt` in session variable. -func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, succ bool, hasMoreResults bool) { +func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, err error, hasMoreResults bool) { sessVars := a.Ctx.GetSessionVars() execDetail := sessVars.StmtCtx.GetExecDetails() // Attach commit/lockKeys runtime stats to executor runtime stats. @@ -849,6 +848,7 @@ func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, succ bool, hasMoreResults boo // Only record the read keys in write statement which affect row more than 0. a.Ctx.GetTxnWriteThroughputSLI().AddReadKeys(execDetail.ScanDetail.ProcessedKeys) } + succ := err == nil // `LowSlowQuery` and `SummaryStmt` must be called before recording `PrevStmt`. a.LogSlowQuery(txnTS, succ, hasMoreResults) a.SummaryStmt(succ) @@ -856,7 +856,7 @@ func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, succ bool, hasMoreResults boo if succ { totalTiFlashQuerySuccCounter.Inc() } else { - totalTiFlashQueryFailCounter.Inc() + metrics.TiFlashQueryTotalCounter.WithLabelValues(metrics.ExecuteErrorToLabel(err), metrics.LblError).Inc() } } prevStmt := a.GetTextToLog() @@ -877,7 +877,7 @@ func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, succ bool, hasMoreResults boo // CloseRecordSet will finish the execution of current statement and do some record work func (a *ExecStmt) CloseRecordSet(txnStartTS uint64, lastErr error) { - a.FinishExecuteStmt(txnStartTS, lastErr == nil, false) + a.FinishExecuteStmt(txnStartTS, lastErr, false) a.logAudit() // Detach the Memory and disk tracker for the previous stmtCtx from GlobalMemoryUsageTracker and GlobalDiskUsageTracker if stmtCtx := a.Ctx.GetSessionVars().StmtCtx; stmtCtx != nil { diff --git a/metrics/server.go b/metrics/server.go index 3f01eec30bea8..f83c7afee2732 100644 --- a/metrics/server.go +++ b/metrics/server.go @@ -227,7 +227,7 @@ var ( Subsystem: "server", Name: "tiflash_query_total", Help: "Counter of TiFlash queries.", - }, []string{LblResult}) + }, []string{LblType, LblResult}) ) // ExecuteErrorToLabel converts an execute error to label. diff --git a/server/conn.go b/server/conn.go index 7a674c57532dd..ffad5d84263b8 100644 --- a/server/conn.go +++ b/server/conn.go @@ -1662,7 +1662,7 @@ func (cc *clientConn) handleStmt(ctx context.Context, stmt ast.StmtNode, warns [ if handled { execStmt := cc.ctx.Value(session.ExecStmtVarKey) if execStmt != nil { - execStmt.(*executor.ExecStmt).FinishExecuteStmt(0, err == nil, false) + execStmt.(*executor.ExecStmt).FinishExecuteStmt(0, err, false) } } if err != nil { diff --git a/session/session.go b/session/session.go index 9d520b3d735ce..da871b1179de9 100644 --- a/session/session.go +++ b/session/session.go @@ -1546,7 +1546,7 @@ func runStmt(ctx context.Context, se *session, s sqlexec.Statement) (rs sqlexec. } else { // If it is not a select statement or special query, we record its slow log here, // then it could include the transaction commit time. - s.(*executor.ExecStmt).FinishExecuteStmt(origTxnCtx.StartTS, err == nil, false) + s.(*executor.ExecStmt).FinishExecuteStmt(origTxnCtx.StartTS, err, false) } return nil, err } From 6f345df65f5da33fcd19f4b5e26174e6aa071b82 Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Fri, 13 Aug 2021 01:33:16 +0800 Subject: [PATCH 12/22] planner: check filter condition in func convertToPartialTableScan (#25294) (#25806) --- executor/index_merge_reader_test.go | 16 ++++++++++++++++ planner/core/find_best_task.go | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/executor/index_merge_reader_test.go b/executor/index_merge_reader_test.go index 647ab6911e358..cfb761cae3de2 100644 --- a/executor/index_merge_reader_test.go +++ b/executor/index_merge_reader_test.go @@ -62,6 +62,22 @@ func (s *testSuite1) TestIndexMergeReaderAndGeneratedColumn(c *C) { tk.MustQuery("SELECT t0.c0 FROM t0 WHERE t0.c1 OR t0.c0").Check(testkit.Rows("1")) } +// issue 25045 +func (s *testSuite1) TestIndexMergeReaderIssue25045(c *C) { + tk := testkit.NewTestKitWithInit(c, s.store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t1") + tk.MustExec("create table t1(a int primary key, b int, c int, key(b), key(c));") + tk.MustExec("INSERT INTO t1 VALUES (10, 10, 10), (11, 11, 11)") + tk.MustQuery("explain format='brief' select /*+ use_index_merge(t1) */ * from t1 where c=10 or (b=10 and a=10);").Check(testkit.Rows( + "IndexMerge 0.01 root ", + "├─IndexRangeScan(Build) 10.00 cop[tikv] table:t1, index:c(c) range:[10,10], keep order:false, stats:pseudo", + "├─TableRangeScan(Build) 1.00 cop[tikv] table:t1 range:[10,10], keep order:false, stats:pseudo", + "└─Selection(Probe) 0.01 cop[tikv] or(eq(test.t1.c, 10), and(eq(test.t1.b, 10), eq(test.t1.a, 10)))", + " └─TableRowIDScan 11.00 cop[tikv] table:t1 keep order:false, stats:pseudo")) + tk.MustQuery("select /*+ use_index_merge(t1) */ * from t1 where c=10 or (b=10 and a=10);").Check(testkit.Rows("10 10 10")) +} + func (s *testSuite1) TestIssue16910(c *C) { tk := testkit.NewTestKitWithInit(c, s.store) tk.MustExec("use test;") diff --git a/planner/core/find_best_task.go b/planner/core/find_best_task.go index 27c4d43274c61..9ca916a7a1b92 100644 --- a/planner/core/find_best_task.go +++ b/planner/core/find_best_task.go @@ -864,10 +864,28 @@ func (ds *DataSource) convertToPartialIndexScan(prop *property.PhysicalProperty, return indexPlan, partialCost } +func checkColinSchema(cols []*expression.Column, schema *expression.Schema) bool { + for _, col := range cols { + if schema.ColumnIndex(col) == -1 { + return false + } + } + return true +} + func (ds *DataSource) convertToPartialTableScan(prop *property.PhysicalProperty, path *util.AccessPath) ( tablePlan PhysicalPlan, partialCost float64) { ts, partialCost, rowCount := ds.getOriginalPhysicalTableScan(prop, path, false) overwritePartialTableScanSchema(ds, ts) + // remove ineffetive filter condition after overwriting physicalscan schema + newFilterConds := make([]expression.Expression, 0, len(path.TableFilters)) + for _, cond := range ts.filterCondition { + cols := expression.ExtractColumns(cond) + if checkColinSchema(cols, ts.schema) { + newFilterConds = append(newFilterConds, cond) + } + } + ts.filterCondition = newFilterConds rowSize := ds.TblColHists.GetAvgRowSize(ds.ctx, ts.schema.Columns, false, false) sessVars := ds.ctx.GetSessionVars() if len(ts.filterCondition) > 0 { From 05f97c85b3ca02e3bff5c1c62987956d887cb7f1 Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Fri, 13 Aug 2021 01:53:17 +0800 Subject: [PATCH 13/22] util/stmtsummary: discard the plan if it is too long and enlarge the tidb_stmt_summary_max_stmt_count value to 3000 (#25843) (#25873) --- config/config.go | 2 +- config/config.toml.example | 2 +- util/plancodec/codec.go | 9 ++++++++ util/plancodec/codec_test.go | 6 +++++ util/stmtsummary/statement_summary.go | 5 +++++ util/stmtsummary/statement_summary_test.go | 26 ++++++++++++++++++++++ 6 files changed, 48 insertions(+), 2 deletions(-) diff --git a/config/config.go b/config/config.go index 16cfa4ff93237..c4408bc4a2e09 100644 --- a/config/config.go +++ b/config/config.go @@ -648,7 +648,7 @@ var defaultConf = Config{ StmtSummary: StmtSummary{ Enable: true, EnableInternalQuery: false, - MaxStmtCount: 200, + MaxStmtCount: 3000, MaxSQLLength: 4096, RefreshInterval: 1800, HistorySize: 24, diff --git a/config/config.toml.example b/config/config.toml.example index 6db0b5c517277..6ac986bb95807 100644 --- a/config/config.toml.example +++ b/config/config.toml.example @@ -448,7 +448,7 @@ enable = true enable-internal-query = false # max number of statements kept in memory. -max-stmt-count = 200 +max-stmt-count = 3000 # max length of displayed normalized sql and sample sql. max-sql-length = 4096 diff --git a/util/plancodec/codec.go b/util/plancodec/codec.go index e7568493fed94..65fd2c64b899f 100644 --- a/util/plancodec/codec.go +++ b/util/plancodec/codec.go @@ -40,6 +40,12 @@ const ( separatorStr = "\t" ) +var ( + // PlanDiscardedEncoded indicates the discard plan because it is too long + PlanDiscardedEncoded = "[discard]" + planDiscardedDecoded = "(plan discarded because too long)" +) + var decoderPool = sync.Pool{ New: func() interface{} { return &planDecoder{} @@ -87,6 +93,9 @@ type planInfo struct { func (pd *planDecoder) decode(planString string) (string, error) { str, err := decompress(planString) if err != nil { + if planString == PlanDiscardedEncoded { + return planDiscardedDecoded, nil + } return "", err } return pd.buildPlanTree(str) diff --git a/util/plancodec/codec_test.go b/util/plancodec/codec_test.go index 1f98adda4cf99..a3375673c95d4 100644 --- a/util/plancodec/codec_test.go +++ b/util/plancodec/codec_test.go @@ -50,3 +50,9 @@ func (s *testPlanCodecSuite) TestEncodeTaskType(c *C) { _, err = decodeTaskType("1_x") c.Assert(err, NotNil) } + +func (s *testPlanCodecSuite) TestDecodeDiscardPlan(c *C) { + plan, err := DecodePlan(PlanDiscardedEncoded) + c.Assert(err, IsNil) + c.Assert(plan, DeepEquals, planDiscardedDecoded) +} diff --git a/util/stmtsummary/statement_summary.go b/util/stmtsummary/statement_summary.go index 35c93c4e926d2..4d1d4839c745c 100644 --- a/util/stmtsummary/statement_summary.go +++ b/util/stmtsummary/statement_summary.go @@ -615,10 +615,15 @@ func (ssbd *stmtSummaryByDigest) collectHistorySummaries(historySize int) []*stm return ssElements } +var maxEncodedPlanSizeInBytes = 1024 * 1024 + func newStmtSummaryByDigestElement(sei *StmtExecInfo, beginTime int64, intervalSeconds int64) *stmtSummaryByDigestElement { // sampleSQL / authUsers(sampleUser) / samplePlan / prevSQL / indexNames store the values shown at the first time, // because it compacts performance to update every time. samplePlan, planHint := sei.PlanGenerator() + if len(samplePlan) > maxEncodedPlanSizeInBytes { + samplePlan = plancodec.PlanDiscardedEncoded + } ssElement := &stmtSummaryByDigestElement{ beginTime: beginTime, sampleSQL: formatSQL(sei.OriginalSQL), diff --git a/util/stmtsummary/statement_summary_test.go b/util/stmtsummary/statement_summary_test.go index d81527494dbd1..f1ef5b6599b70 100644 --- a/util/stmtsummary/statement_summary_test.go +++ b/util/stmtsummary/statement_summary_test.go @@ -30,6 +30,7 @@ import ( "github.com/pingcap/tidb/store/tikv" "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/execdetails" + "github.com/pingcap/tidb/util/plancodec" ) var _ = Suite(&testStmtSummarySuite{}) @@ -423,6 +424,31 @@ func (s *testStmtSummarySuite) TestAddStatement(c *C) { c.Assert(s.ssMap.summaryMap.Size(), Equals, 4) _, ok = s.ssMap.summaryMap.Get(key) c.Assert(ok, IsTrue) + + // Test for plan too large + stmtExecInfo7 := stmtExecInfo1 + stmtExecInfo7.PlanDigest = "plan_digest7" + stmtExecInfo7.PlanGenerator = func() (string, string) { + buf := make([]byte, maxEncodedPlanSizeInBytes+1) + for i := range buf { + buf[i] = 'a' + } + return string(buf), "" + } + key = &stmtSummaryByDigestKey{ + schemaName: stmtExecInfo7.SchemaName, + digest: stmtExecInfo7.Digest, + planDigest: stmtExecInfo7.PlanDigest, + } + s.ssMap.AddStatement(stmtExecInfo7) + c.Assert(s.ssMap.summaryMap.Size(), Equals, 5) + v, ok := s.ssMap.summaryMap.Get(key) + c.Assert(ok, IsTrue) + stmt := v.(*stmtSummaryByDigest) + c.Assert(stmt.digest, DeepEquals, key.digest) + e := stmt.history.Back() + ssElement := e.Value.(*stmtSummaryByDigestElement) + c.Assert(ssElement.samplePlan, Equals, plancodec.PlanDiscardedEncoded) } func matchStmtSummaryByDigest(first, second *stmtSummaryByDigest) bool { From 702bed1ae4c371d3203ce8801efa59cc1394dac6 Mon Sep 17 00:00:00 2001 From: lysu Date: Fri, 13 Aug 2021 02:05:16 +0800 Subject: [PATCH 14/22] tables: fix the wrong result for "insert ignore duplicate up" on partition table when handle changed (#25859) --- executor/write_test.go | 6 ++++++ table/tables/tables.go | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/executor/write_test.go b/executor/write_test.go index 0334430b7a3c9..b97eccabb79f2 100644 --- a/executor/write_test.go +++ b/executor/write_test.go @@ -806,6 +806,12 @@ func (s *testSuite4) TestInsertIgnoreOnDup(c *C) { tk.MustQuery("select * from t6").Check(testkit.Rows("100 10 20")) tk.MustExec("insert ignore into t6 set a = 200, b= 10 on duplicate key update c = 1000") tk.MustQuery("select * from t6").Check(testkit.Rows("100 10 1000")) + + tk.MustExec("drop table if exists t7") + tk.MustExec("CREATE TABLE t7 (`col_334` mediumint(9) NOT NULL DEFAULT '-3217641', `col_335` mediumint(8) unsigned NOT NULL DEFAULT '2002468', `col_336` enum('alice','bob','charlie','david') COLLATE utf8_general_ci NOT NULL DEFAULT 'alice', PRIMARY KEY (`col_334`,`col_336`,`col_335`) CLUSTERED, UNIQUE KEY `idx_116` (`col_334`,`col_335`), UNIQUE KEY `idx_117` (`col_336`,`col_334`), KEY `idx_118` (`col_336`)) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_general_ci PARTITION BY HASH( `col_334` ) PARTITIONS 6;") + tk.MustExec("insert into t7(col_335, col_336) values(7685969, 'alice'),(2002468, 'bob')") + tk.MustExec("insert ignore into t7(col_335, col_336) values(2002468, 'david') on duplicate key update col_335 = 7685969") + tk.MustQuery("select * from t7").Check(testkit.Rows("-3217641 7685969 alice", "-3217641 2002468 bob")) } func (s *testSuite4) TestInsertSetWithDefault(c *C) { diff --git a/table/tables/tables.go b/table/tables/tables.go index 0746f0e6f33e5..913de778a4eb8 100644 --- a/table/tables/tables.go +++ b/table/tables/tables.go @@ -1451,6 +1451,7 @@ func FindIndexByColName(t table.Table, name string) table.Index { // otherwise return kv.ErrKeyExists error. func CheckHandleOrUniqueKeyExistForUpdateIgnoreOrInsertOnDupIgnore(ctx context.Context, sctx sessionctx.Context, t table.Table, recordID kv.Handle, newRow []types.Datum, modified []bool) error { physicalTableID := t.Meta().ID + idxs := t.Indices() if pt, ok := t.(*partitionedTable); ok { info := t.Meta().GetPartitionInfo() pid, err := pt.locatePartition(sctx, info, newRow) @@ -1459,6 +1460,7 @@ func CheckHandleOrUniqueKeyExistForUpdateIgnoreOrInsertOnDupIgnore(ctx context.C } partition := pt.GetPartition(pid) physicalTableID = partition.GetPhysicalID() + idxs = partition.Indices() } txn, err := sctx.Txn(true) if err != nil { @@ -1492,7 +1494,7 @@ func CheckHandleOrUniqueKeyExistForUpdateIgnoreOrInsertOnDupIgnore(ctx context.C return true } - for _, idx := range t.Indices() { + for _, idx := range idxs { if shouldSkipIgnoreCheck(idx) { continue } From da52bcc97450e316f3e18afb56ab0cf1259de7cb Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Fri, 13 Aug 2021 02:19:16 +0800 Subject: [PATCH 15/22] server, sessionctx: improved mysql compatibility with support for init_connect (#23713) (#26072) --- server/conn.go | 64 +++++++++++++++++++++++++++++++++++ server/server.go | 1 + server/server_test.go | 48 ++++++++++++++++++++++++++ server/tidb_test.go | 4 +++ sessionctx/variable/noop.go | 1 - sessionctx/variable/sysvar.go | 1 + 6 files changed, 118 insertions(+), 1 deletion(-) diff --git a/server/conn.go b/server/conn.go index ffad5d84263b8..7ef95d7bd8ad7 100644 --- a/server/conn.go +++ b/server/conn.go @@ -71,6 +71,7 @@ import ( "github.com/pingcap/tidb/metrics" plannercore "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/plugin" + "github.com/pingcap/tidb/privilege" "github.com/pingcap/tidb/session" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/sessionctx/stmtctx" @@ -251,6 +252,18 @@ func (cc *clientConn) handshake(ctx context.Context) error { } return err } + + // MySQL supports an "init_connect" query, which can be run on initial connection. + // The query must return a non-error or the client is disconnected. + if err := cc.initConnect(ctx); err != nil { + logutil.Logger(ctx).Warn("init_connect failed", zap.Error(err)) + initErr := errNewAbortingConnection.FastGenByArgs(cc.connectionID, "unconnected", cc.user, cc.peerHost, "init_connect command failed") + if err1 := cc.writeError(ctx, initErr); err1 != nil { + terror.Log(err1) + } + return initErr + } + data := cc.alloc.AllocWithLen(4, 32) data = append(data, mysql.OKHeader) data = append(data, 0, 0) @@ -722,6 +735,57 @@ func (cc *clientConn) PeerHost(hasPassword string) (host, port string, err error return } +// skipInitConnect follows MySQL's rules of when init-connect should be skipped. +// In 5.7 it is any user with SUPER privilege, but in 8.0 it is: +// - SUPER or the CONNECTION_ADMIN dynamic privilege. +// - (additional exception) users with expired passwords (not yet supported) +func (cc *clientConn) skipInitConnect() bool { + checker := privilege.GetPrivilegeManager(cc.ctx.Session) + activeRoles := cc.ctx.GetSessionVars().ActiveRoles + return checker != nil && checker.RequestVerification(activeRoles, "", "", "", mysql.SuperPriv) +} + +// initConnect runs the initConnect SQL statement if it has been specified. +// The semantics are MySQL compatible. +func (cc *clientConn) initConnect(ctx context.Context) error { + val, err := cc.ctx.GetSessionVars().GlobalVarsAccessor.GetGlobalSysVar(variable.InitConnect) + if err != nil { + return err + } + if val == "" || cc.skipInitConnect() { + return nil + } + logutil.Logger(ctx).Debug("init_connect starting") + stmts, err := cc.ctx.Parse(ctx, val) + if err != nil { + return err + } + for _, stmt := range stmts { + rs, err := cc.ctx.ExecuteStmt(ctx, stmt) + if err != nil { + return err + } + // init_connect does not care about the results, + // but they need to be drained because of lazy loading. + if rs != nil { + req := rs.NewChunk() + for { + if err = rs.Next(ctx, req); err != nil { + return err + } + if req.NumRows() == 0 { + break + } + } + if err := rs.Close(); err != nil { + return err + } + } + } + logutil.Logger(ctx).Debug("init_connect complete") + return nil +} + // Run reads client query and writes query result to client in for loop, if there is a panic during query handling, // it will be recovered and log the panic error. // This function returns and the connection is closed if there is an IO error or there is a panic. diff --git a/server/server.go b/server/server.go index 0f839cb559029..1b1929a9854d8 100644 --- a/server/server.go +++ b/server/server.go @@ -99,6 +99,7 @@ var ( errConCount = dbterror.ClassServer.NewStd(errno.ErrConCount) errSecureTransportRequired = dbterror.ClassServer.NewStd(errno.ErrSecureTransportRequired) errMultiStatementDisabled = dbterror.ClassServer.NewStd(errno.ErrMultiStatementDisabled) + errNewAbortingConnection = dbterror.ClassServer.NewStd(errno.ErrNewAbortingConnection) ) // DefaultCapability is the capability of the server when it is created using the default configuration. diff --git a/server/server_test.go b/server/server_test.go index 48eb0b7639ed7..a2f3c8d03244b 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -1864,6 +1864,54 @@ func (cli *testServerClient) waitUntilServerOnline() { } } +func (cli *testServerClient) runTestInitConnect(c *C) { + + cli.runTests(c, nil, func(dbt *DBTest) { + dbt.mustExec(`SET GLOBAL init_connect="insert into test.ts VALUES (NOW());SET @a=1;"`) + dbt.mustExec(`CREATE USER init_nonsuper`) + dbt.mustExec(`CREATE USER init_super`) + dbt.mustExec(`GRANT SELECT, INSERT, DROP ON test.* TO init_nonsuper`) + dbt.mustExec(`GRANT SELECT, INSERT, DROP, SUPER ON *.* TO init_super`) + dbt.mustExec(`CREATE TABLE ts (a TIMESTAMP)`) + }) + + // test init_nonsuper + cli.runTests(c, func(config *mysql.Config) { + config.User = "init_nonsuper" + }, func(dbt *DBTest) { + rows := dbt.mustQuery(`SELECT @a`) + c.Assert(rows.Next(), IsTrue) + var a int + err := rows.Scan(&a) + c.Assert(err, IsNil) + dbt.Check(a, Equals, 1) + c.Assert(rows.Close(), IsNil) + }) + + // test init_super + cli.runTests(c, func(config *mysql.Config) { + config.User = "init_super" + }, func(dbt *DBTest) { + rows := dbt.mustQuery(`SELECT IFNULL(@a,"")`) + c.Assert(rows.Next(), IsTrue) + var a string + err := rows.Scan(&a) + c.Assert(err, IsNil) + dbt.Check(a, Equals, "") // null + c.Assert(rows.Close(), IsNil) + // change the init-connect to invalid. + dbt.mustExec(`SET GLOBAL init_connect="invalidstring"`) + }) + + db, err := sql.Open("mysql", cli.getDSN(func(config *mysql.Config) { + config.User = "init_nonsuper" + })) + c.Assert(err, IsNil, Commentf("Error connecting")) // doesn't fail because of lazy loading + defer db.Close() // may already be closed + _, err = db.Exec("SELECT 1") // fails because of init sql + c.Assert(err, NotNil) +} + // Client errors are only incremented when using the TiDB Server protocol, // and not internal SQL statements. Thus, this test is in the server-test suite. func (cli *testServerClient) runTestInfoschemaClientErrors(t *C) { diff --git a/server/tidb_test.go b/server/tidb_test.go index 7bea9bd655d9d..9c5b76bf7d251 100644 --- a/server/tidb_test.go +++ b/server/tidb_test.go @@ -952,6 +952,10 @@ func (ts *tidbTestSuite) TestClientErrors(c *C) { ts.runTestInfoschemaClientErrors(c) } +func (ts *tidbTestSuite) TestInitConnect(c *C) { + ts.runTestInitConnect(c) +} + func (ts *tidbTestSuite) TestSumAvg(c *C) { c.Parallel() ts.runTestSumAvg(c) diff --git a/sessionctx/variable/noop.go b/sessionctx/variable/noop.go index 1eba30079ee2d..7ba11357b81fb 100644 --- a/sessionctx/variable/noop.go +++ b/sessionctx/variable/noop.go @@ -153,7 +153,6 @@ var noopSysVars = []*SysVar{ {Scope: ScopeGlobal | ScopeSession, Name: QueryCacheType, Value: BoolOff, Type: TypeEnum, PossibleValues: []string{BoolOff, BoolOn, "DEMAND"}}, {Scope: ScopeNone, Name: "innodb_rollback_on_timeout", Value: "0"}, {Scope: ScopeGlobal | ScopeSession, Name: "query_alloc_block_size", Value: "8192"}, - {Scope: ScopeGlobal | ScopeSession, Name: InitConnect, Value: ""}, {Scope: ScopeNone, Name: "have_compress", Value: "YES"}, {Scope: ScopeNone, Name: "thread_concurrency", Value: "10"}, {Scope: ScopeGlobal | ScopeSession, Name: "query_prealloc_size", Value: "8192"}, diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index 6eb8ce417e0d3..da010df34ac24 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -579,6 +579,7 @@ var defaultSysVars = []*SysVar{ } return oracle.LocalTxnScope }()}, + {Scope: ScopeGlobal, Name: InitConnect, Value: ""}, /* TiDB specific variables */ {Scope: ScopeGlobal | ScopeSession, Name: TiDBAllowMPPExecution, Type: TypeBool, Value: BoolToOnOff(DefTiDBAllowMPPExecution)}, {Scope: ScopeSession, Name: TiDBEnforceMPPExecution, Type: TypeBool, Value: BoolToOnOff(config.GetGlobalConfig().Performance.EnforceMPP)}, From 6b70fe8077eee02776db6a77a75370b86489d13b Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Fri, 13 Aug 2021 02:31:16 +0800 Subject: [PATCH 16/22] expression, executor: fix type infer for greatest/leastest(datetime) (#26533) (#26565) --- executor/executor_test.go | 9 +++++++++ expression/builtin_compare.go | 15 +++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/executor/executor_test.go b/executor/executor_test.go index a47dc09be8e72..0784661c4e1dc 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -8220,3 +8220,12 @@ func (s *testSerialSuite) TestIssue24210(c *C) { c.Assert(err, IsNil) } + +func (s *testSuite) TestIssue26532(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustQuery("select greatest(cast(\"2020-01-01 01:01:01\" as datetime), cast(\"2019-01-01 01:01:01\" as datetime) )union select null;").Sort().Check(testkit.Rows("2020-01-01 01:01:01", "")) + tk.MustQuery("select least(cast(\"2020-01-01 01:01:01\" as datetime), cast(\"2019-01-01 01:01:01\" as datetime) )union select null;").Sort().Check(testkit.Rows("2019-01-01 01:01:01", "")) + tk.MustQuery("select greatest(\"2020-01-01 01:01:01\" ,\"2019-01-01 01:01:01\" )union select null;").Sort().Check(testkit.Rows("2020-01-01 01:01:01", "")) + tk.MustQuery("select least(\"2020-01-01 01:01:01\" , \"2019-01-01 01:01:01\" )union select null;").Sort().Check(testkit.Rows("2019-01-01 01:01:01", "")) +} diff --git a/expression/builtin_compare.go b/expression/builtin_compare.go index 774680efb173d..5225ddf773337 100644 --- a/expression/builtin_compare.go +++ b/expression/builtin_compare.go @@ -485,9 +485,23 @@ func (c *greatestFunctionClass) getFunction(ctx sessionctx.Context, args []Expre sig = &builtinGreatestTimeSig{bf} sig.setPbCode(tipb.ScalarFuncSig_GreatestTime) } + sig.getRetTp().Flen, sig.getRetTp().Decimal = fixFlenAndDecimalForGreatestAndLeast(args) return sig, nil } +func fixFlenAndDecimalForGreatestAndLeast(args []Expression) (flen, decimal int) { + for _, arg := range args { + argFlen, argDecimal := arg.GetType().Flen, arg.GetType().Decimal + if argFlen > flen { + flen = argFlen + } + if argDecimal > decimal { + decimal = argDecimal + } + } + return flen, decimal +} + type builtinGreatestIntSig struct { baseBuiltinFunc } @@ -702,6 +716,7 @@ func (c *leastFunctionClass) getFunction(ctx sessionctx.Context, args []Expressi sig = &builtinLeastTimeSig{bf} sig.setPbCode(tipb.ScalarFuncSig_LeastTime) } + sig.getRetTp().Flen, sig.getRetTp().Decimal = fixFlenAndDecimalForGreatestAndLeast(args) return sig, nil } From d79ef68b57f598801ed631a35f030b629727de81 Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Fri, 13 Aug 2021 10:55:16 +0800 Subject: [PATCH 17/22] metric: fix wrong total-query-counter label (#25402) (#25445) --- server/conn.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/conn.go b/server/conn.go index 7ef95d7bd8ad7..b671ec1502fc7 100644 --- a/server/conn.go +++ b/server/conn.go @@ -946,7 +946,7 @@ func (cc *clientConn) addMetrics(cmd byte, startTime time.Time, err error) { } else { label := strconv.Itoa(int(cmd)) if err != nil { - metrics.QueryTotalCounter.WithLabelValues(label, "ERROR").Inc() + metrics.QueryTotalCounter.WithLabelValues(label, "Error").Inc() } else { metrics.QueryTotalCounter.WithLabelValues(label, "OK").Inc() } From d969bcbdc149dbd9d8e1776188d626c689eb63fc Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Fri, 13 Aug 2021 11:33:16 +0800 Subject: [PATCH 18/22] ddl: rollingback add index meets panic leads json unmarshal object error (#23848) (#24441) --- ddl/db_test.go | 20 ++++++++++++++++++++ ddl/ddl_worker.go | 7 +++++++ 2 files changed, 27 insertions(+) diff --git a/ddl/db_test.go b/ddl/db_test.go index d4d116cf54450..d62d82a1c78ab 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -6590,3 +6590,23 @@ func (s *testSerialDBSuite) TestIssue22819(c *C) { _, err := tk1.Exec("commit") c.Assert(err, ErrorMatches, ".*8028.*Information schema is changed during the execution of the statement.*") } + +// Close issue #23321. +// See https://github.com/pingcap/tidb/issues/23321 +func (s *testSerialDBSuite) TestJsonUnmarshalErrWhenPanicInCancellingPath(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + + tk.MustExec("drop table if exists test_add_index_after_add_col") + tk.MustExec("create table test_add_index_after_add_col(a int, b int not null default '0');") + tk.MustExec("insert into test_add_index_after_add_col values(1, 2),(2,2);") + tk.MustExec("alter table test_add_index_after_add_col add column c int not null default '0';") + + c.Assert(failpoint.Enable("github.com/pingcap/tidb/ddl/mockExceedErrorLimit", `return(true)`), IsNil) + defer func() { + c.Assert(failpoint.Disable("github.com/pingcap/tidb/ddl/mockExceedErrorLimit"), IsNil) + }() + + _, err := tk.Exec("alter table test_add_index_after_add_col add unique index cc(c);") + c.Assert(err.Error(), Equals, "[kv:1062]DDL job cancelled by panic in rollingback, error msg: Duplicate entry '0' for key 'cc'") +} diff --git a/ddl/ddl_worker.go b/ddl/ddl_worker.go index 1c263821b932b..326c69cccbfd0 100644 --- a/ddl/ddl_worker.go +++ b/ddl/ddl_worker.go @@ -624,6 +624,13 @@ func chooseLeaseTime(t, max time.Duration) time.Duration { // countForPanic records the error count for DDL job. func (w *worker) countForPanic(job *model.Job) { // If run DDL job panic, just cancel the DDL jobs. + if job.State == model.JobStateRollingback { + job.State = model.JobStateCancelled + msg := fmt.Sprintf("DDL job cancelled by panic in rollingback, error msg: %s", terror.ToSQLError(job.Error).Message) + job.Error = terror.GetErrClass(job.Error).Synthesize(terror.ErrCode(job.Error.Code()), msg) + logutil.Logger(w.logCtx).Warn(msg) + return + } job.State = model.JobStateCancelling job.ErrorCount++ From 862439f1f5ecb993594bfcdd299b6124e3c14611 Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Fri, 13 Aug 2021 17:55:59 +0800 Subject: [PATCH 19/22] planner: Fix the problem that `PlanBuilder.buildWindowFunctions` may change sub operator's schema. (#27176) (#27201) --- expression/aggregation/base_func.go | 2 +- planner/core/logical_plan_builder.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/expression/aggregation/base_func.go b/expression/aggregation/base_func.go index 9a5ef95d49bb2..16d63efa933e3 100644 --- a/expression/aggregation/base_func.go +++ b/expression/aggregation/base_func.go @@ -431,7 +431,7 @@ func (a *baseFuncDesc) WrapCastForAggArgs(ctx sessionctx.Context) { if col, ok := a.Args[i].(*expression.Column); ok { col.RetType = types.NewFieldType(col.RetType.Tp) } - // originTp is used when the the `Tp` of column is TypeFloat32 while + // originTp is used when the `Tp` of column is TypeFloat32 while // the type of the aggregation function is TypeFloat64. originTp := a.Args[i].GetType().Tp *(a.Args[i].GetType()) = *(a.RetTp) diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index f3e8d7393a081..1d78d5fb9b658 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -4830,7 +4830,7 @@ func (b *PlanBuilder) buildProjectionForWindow(ctx context.Context, p LogicalPla p = np switch newArg.(type) { case *expression.Column, *expression.Constant: - newArgList = append(newArgList, newArg) + newArgList = append(newArgList, newArg.Clone()) continue } proj.Exprs = append(proj.Exprs, newArg) @@ -4862,7 +4862,7 @@ func (b *PlanBuilder) buildArgs4WindowFunc(ctx context.Context, p LogicalPlan, a p = np switch newArg.(type) { case *expression.Column, *expression.Constant: - newArgList = append(newArgList, newArg) + newArgList = append(newArgList, newArg.Clone()) continue } col := &expression.Column{ From 9355681402cd73763c24c25ee5a06c6b2fb5f1ce Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Fri, 13 Aug 2021 19:07:59 +0800 Subject: [PATCH 20/22] executor: fix hash join between datetime and timestamp (#25915) (#25990) --- executor/join_test.go | 17 +++++++++++++++++ util/codec/codec.go | 18 ++---------------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/executor/join_test.go b/executor/join_test.go index bfd0048a63b3d..a5ca48e7b1976 100644 --- a/executor/join_test.go +++ b/executor/join_test.go @@ -2592,3 +2592,20 @@ func (s *testSuiteJoinSerial) TestIssue20219(c *C) { tk.MustQuery("select /*+ inl_join(s)*/ t.a from t left join s on t.a = s.a;").Check(testkit.Rows("i", "j")) tk.MustQuery("show warnings").Check(testkit.Rows()) } + +func (s *testSuiteJoinSerial) TestIssue25902(c *C) { + tk := testkit.NewTestKitWithInit(c, s.store) + tk.MustExec("drop table if exists tt1,tt2,tt3; ") + tk.MustExec("create table tt1 (ts timestamp);") + tk.MustExec("create table tt2 (ts varchar(32));") + tk.MustExec("create table tt3 (ts datetime);") + tk.MustExec("insert into tt1 values (\"2001-01-01 00:00:00\");") + tk.MustExec("insert into tt2 values (\"2001-01-01 00:00:00\");") + tk.MustExec("insert into tt3 values (\"2001-01-01 00:00:00\");") + tk.MustQuery("select * from tt1 where ts in (select ts from tt2);").Check(testkit.Rows("2001-01-01 00:00:00")) + tk.MustQuery("select * from tt1 where ts in (select ts from tt3);").Check(testkit.Rows("2001-01-01 00:00:00")) + tk.MustExec("set @tmp=(select @@session.time_zone);") + tk.MustExec("set @@session.time_zone = '+10:00';") + tk.MustQuery("select * from tt1 where ts in (select ts from tt2);").Check(testkit.Rows()) + tk.MustExec("set @@session.time_zone = @tmp;") +} diff --git a/util/codec/codec.go b/util/codec/codec.go index 9b5e6b82a808d..641c543b60a63 100644 --- a/util/codec/codec.go +++ b/util/codec/codec.go @@ -330,14 +330,7 @@ func encodeHashChunkRowIdx(sc *stmtctx.StatementContext, row chunk.Row, tp *type case mysql.TypeDate, mysql.TypeDatetime, mysql.TypeTimestamp: flag = uintFlag t := row.GetTime(idx) - // Encoding timestamp need to consider timezone. - // If it's not in UTC, transform to UTC first. - if t.Type() == mysql.TypeTimestamp && sc.TimeZone != time.UTC { - err = t.ConvertTimeZone(sc.TimeZone, time.UTC) - if err != nil { - return - } - } + var v uint64 v, err = t.ToPackedUint() if err != nil { @@ -501,14 +494,7 @@ func HashChunkSelected(sc *stmtctx.StatementContext, h []hash.Hash64, chk *chunk isNull[i] = !ignoreNull } else { buf[0] = uintFlag - // Encoding timestamp need to consider timezone. - // If it's not in UTC, transform to UTC first. - if t.Type() == mysql.TypeTimestamp && sc.TimeZone != time.UTC { - err = t.ConvertTimeZone(sc.TimeZone, time.UTC) - if err != nil { - return - } - } + var v uint64 v, err = t.ToPackedUint() if err != nil { From eb462f2db4702d3bf4963a92d520484db1e31b8e Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Fri, 13 Aug 2021 19:27:58 +0800 Subject: [PATCH 21/22] config: ignore tiflash when show config (#24770) (#24806) --- executor/memtable_reader.go | 3 +++ executor/memtable_reader_test.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/executor/memtable_reader.go b/executor/memtable_reader.go index a5d39d1c7b21d..5850570456f3b 100644 --- a/executor/memtable_reader.go +++ b/executor/memtable_reader.go @@ -191,6 +191,9 @@ func fetchClusterConfig(sctx sessionctx.Context, nodeTypes, nodeAddrs set.String url = fmt.Sprintf("%s://%s%s", util.InternalHTTPSchema(), statusAddr, pdapi.Config) case "tikv", "tidb": url = fmt.Sprintf("%s://%s/config", util.InternalHTTPSchema(), statusAddr) + case "tiflash": + // TODO: support show tiflash config once tiflash supports it + return default: ch <- result{err: errors.Errorf("unknown node type: %s(%s)", typ, address)} return diff --git a/executor/memtable_reader_test.go b/executor/memtable_reader_test.go index efd7f331cb0e6..c81801856f6fe 100644 --- a/executor/memtable_reader_test.go +++ b/executor/memtable_reader_test.go @@ -170,7 +170,7 @@ func (s *testMemTableReaderSuite) TestTiDBClusterConfig(c *C) { // mock servers servers := []string{} - for _, typ := range []string{"tidb", "tikv", "pd"} { + for _, typ := range []string{"tidb", "tikv", "tiflash", "pd"} { for _, server := range testServers { servers = append(servers, strings.Join([]string{typ, server.address, server.address}, ",")) } From 1172d1f9e8ec1d0b19215c24dd06dc0e72259b86 Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Mon, 16 Aug 2021 11:17:59 +0800 Subject: [PATCH 22/22] sessionctx: Fix tidb_gc_scan_lock_mode sysvar doesn't show correct default value on new clusters (#25112) (#25118) --- sessionctx/variable/sysvar.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index da010df34ac24..9113ec1a228ef 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -819,7 +819,7 @@ var defaultSysVars = []*SysVar{ {Scope: ScopeGlobal, Name: TiDBGCRunInterval, Value: "10m0s", Type: TypeDuration, MinValue: int64(time.Minute * 10), MaxValue: math.MaxInt64}, {Scope: ScopeGlobal, Name: TiDBGCLifetime, Value: "10m0s", Type: TypeDuration, MinValue: int64(time.Minute * 10), MaxValue: math.MaxInt64}, {Scope: ScopeGlobal, Name: TiDBGCConcurrency, Value: "-1", Type: TypeInt, MinValue: 1, MaxValue: 128, AllowAutoValue: true}, - {Scope: ScopeGlobal, Name: TiDBGCScanLockMode, Value: "PHYSICAL", Type: TypeEnum, PossibleValues: []string{"PHYSICAL", "LEGACY"}}, + {Scope: ScopeGlobal, Name: TiDBGCScanLockMode, Value: "LEGACY", Type: TypeEnum, PossibleValues: []string{"PHYSICAL", "LEGACY"}}, } // FeedbackProbability points to the FeedbackProbability in statistics package.