From 7a5e5dc279cd1e49c5c2e5eab992c2ef98176aed Mon Sep 17 00:00:00 2001 From: lihaowei Date: Tue, 22 Jun 2021 09:33:14 +0800 Subject: [PATCH 1/8] fix issue#25595 Signed-off-by: lihaowei --- planner/core/preprocess.go | 2 +- planner/core/preprocess_test.go | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/planner/core/preprocess.go b/planner/core/preprocess.go index 9c3b053cff5e6..c5ec3c657373d 100644 --- a/planner/core/preprocess.go +++ b/planner/core/preprocess.go @@ -835,7 +835,7 @@ func (p *preprocessor) checkDropSequenceGrammar(stmt *ast.DropSequenceStmt) { func (p *preprocessor) checkDropTableGrammar(stmt *ast.DropTableStmt) { p.checkDropTableNames(stmt.Tables) enableNoopFuncs := p.ctx.GetSessionVars().EnableNoopFuncs - if stmt.TemporaryKeyword == ast.TemporaryLocal && !enableNoopFuncs { + if stmt.TemporaryKeyword != ast.TemporaryNone && !enableNoopFuncs { p.err = expression.ErrFunctionsNoopImpl.GenWithStackByArgs("DROP TEMPORARY TABLE") return } diff --git a/planner/core/preprocess_test.go b/planner/core/preprocess_test.go index d9f053f509e92..b76cd69c0d919 100644 --- a/planner/core/preprocess_test.go +++ b/planner/core/preprocess_test.go @@ -292,6 +292,10 @@ func (s *testValidatorSuite) TestValidator(c *C) { {"CREATE TEMPORARY TABLE t (a INT);", false, expression.ErrFunctionsNoopImpl.GenWithStackByArgs("CREATE TEMPORARY TABLE")}, {"DROP TEMPORARY TABLE t;", false, expression.ErrFunctionsNoopImpl.GenWithStackByArgs("DROP TEMPORARY TABLE")}, + // issue 25595 + {"create table tb2(id int);", false, nil}, + {"drop global temporary table tb2;", false, expression.ErrFunctionsNoopImpl.GenWithStackByArgs("DROP TEMPORARY TABLE")}, + // TABLESAMPLE {"select * from t tablesample bernoulli();", false, expression.ErrInvalidTableSample}, {"select * from t tablesample bernoulli(10 rows);", false, expression.ErrInvalidTableSample}, From 2e713b2f93ce9a93d901fc18ab4defbeb9dc910b Mon Sep 17 00:00:00 2001 From: lihaowei Date: Tue, 22 Jun 2021 15:12:27 +0800 Subject: [PATCH 2/8] add tests Signed-off-by: lihaowei --- planner/core/preprocess.go | 27 +++++++++++++++++++++------ planner/core/preprocess_test.go | 27 +++++++++++++++++++++++---- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/planner/core/preprocess.go b/planner/core/preprocess.go index c5ec3c657373d..a85ef31862408 100644 --- a/planner/core/preprocess.go +++ b/planner/core/preprocess.go @@ -55,7 +55,7 @@ func InTxnRetry(p *preprocessor) { p.flag |= inTxnRetry } -// WithPreprocessorReturn returns a PreprocessOpt to initialize the PreprocesorReturn. +// WithPreprocessorReturn returns a PreprocessOpt to initialize the PreprocessorReturn. func WithPreprocessorReturn(ret *PreprocessorReturn) PreprocessOpt { return func(p *preprocessor) { p.PreprocessorReturn = ret @@ -93,7 +93,7 @@ func TryAddExtraLimit(ctx sessionctx.Context, node ast.StmtNode) ast.StmtNode { } // Preprocess resolves table names of the node, and checks some statements validation. -// prepreocssReturn used to extract the infoschema for the tableName and the timestamp from the asof clause. +// preprocessReturn used to extract the infoschema for the tableName and the timestamp from the asof clause. func Preprocess(ctx sessionctx.Context, node ast.Node, preprocessOpt ...PreprocessOpt) error { v := preprocessor{ctx: ctx, tableAliasInJoin: make([]map[string]interface{}, 0), withName: make(map[string]interface{})} for _, optFn := range preprocessOpt { @@ -833,12 +833,27 @@ func (p *preprocessor) checkDropSequenceGrammar(stmt *ast.DropSequenceStmt) { } func (p *preprocessor) checkDropTableGrammar(stmt *ast.DropTableStmt) { - p.checkDropTableNames(stmt.Tables) + currentDB := model.NewCIStr(p.ctx.GetSessionVars().CurrentDB) enableNoopFuncs := p.ctx.GetSessionVars().EnableNoopFuncs - if stmt.TemporaryKeyword != ast.TemporaryNone && !enableNoopFuncs { - p.err = expression.ErrFunctionsNoopImpl.GenWithStackByArgs("DROP TEMPORARY TABLE") - return + for _, t := range stmt.Tables { + if isIncorrectName(t.Name.String()) { + p.err = ddl.ErrWrongTableName.GenWithStackByArgs(t.Name.String()) + return + } + if t.Schema.String() != "" { + currentDB = t.Schema + } + tableInfo, err := p.ensureInfoSchema().TableByName(currentDB, t.Name) + if err != nil { + p.err = err + return + } + if stmt.TemporaryKeyword != ast.TemporaryNone && !enableNoopFuncs && tableInfo.Meta().TempTableType == model.TempTableNone { + p.err = expression.ErrFunctionsNoopImpl.GenWithStackByArgs("DROP TEMPORARY TABLE") + return + } } + } func (p *preprocessor) checkDropTableNames(tables []*ast.TableName) { diff --git a/planner/core/preprocess_test.go b/planner/core/preprocess_test.go index b76cd69c0d919..109d36213113c 100644 --- a/planner/core/preprocess_test.go +++ b/planner/core/preprocess_test.go @@ -292,10 +292,6 @@ func (s *testValidatorSuite) TestValidator(c *C) { {"CREATE TEMPORARY TABLE t (a INT);", false, expression.ErrFunctionsNoopImpl.GenWithStackByArgs("CREATE TEMPORARY TABLE")}, {"DROP TEMPORARY TABLE t;", false, expression.ErrFunctionsNoopImpl.GenWithStackByArgs("DROP TEMPORARY TABLE")}, - // issue 25595 - {"create table tb2(id int);", false, nil}, - {"drop global temporary table tb2;", false, expression.ErrFunctionsNoopImpl.GenWithStackByArgs("DROP TEMPORARY TABLE")}, - // TABLESAMPLE {"select * from t tablesample bernoulli();", false, expression.ErrInvalidTableSample}, {"select * from t tablesample bernoulli(10 rows);", false, expression.ErrInvalidTableSample}, @@ -341,3 +337,26 @@ func (s *testValidatorSuite) TestForeignKey(c *C) { s.runSQL(c, "ALTER TABLE test.t1 ADD CONSTRAINT fk FOREIGN KEY (c) REFERENCES test2.t (e)", false, nil) } + +func (s *testValidatorSuite) TestDropGlobalTempTable(c *C) { + defer testleak.AfterTest(c)() + defer func() { + s.dom.Close() + s.store.Close() + }() + + ctx := context.Background() + execSqlList := []string{ + "use test", + "set tidb_enable_global_temporary_table=true", + "create table tb(id int);", + "create global temporary table temp(id int) on commit delete rows;", + } + for _, execSql := range execSqlList { + _, err := s.se.Execute(ctx, execSql) + c.Assert(err, IsNil) + } + s.is = s.dom.InfoSchema() + s.runSQL(c, "drop global temporary table tb;", false, expression.ErrFunctionsNoopImpl.GenWithStackByArgs("DROP TEMPORARY TABLE")) + s.runSQL(c, "drop global temporary table temp", false, nil) +} From a48c6b3ea016c876466e4cfd78e221e0c9a99cf9 Mon Sep 17 00:00:00 2001 From: lihaowei Date: Tue, 22 Jun 2021 15:52:39 +0800 Subject: [PATCH 3/8] format code Signed-off-by: lihaowei --- planner/core/preprocess_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/planner/core/preprocess_test.go b/planner/core/preprocess_test.go index 109d36213113c..40d963ad51d5e 100644 --- a/planner/core/preprocess_test.go +++ b/planner/core/preprocess_test.go @@ -346,14 +346,14 @@ func (s *testValidatorSuite) TestDropGlobalTempTable(c *C) { }() ctx := context.Background() - execSqlList := []string{ + execSQLList := []string{ "use test", "set tidb_enable_global_temporary_table=true", "create table tb(id int);", "create global temporary table temp(id int) on commit delete rows;", } - for _, execSql := range execSqlList { - _, err := s.se.Execute(ctx, execSql) + for _, execSQL := range execSQLList { + _, err := s.se.Execute(ctx, execSQL) c.Assert(err, IsNil) } s.is = s.dom.InfoSchema() From af4db8952b3b4af2f72aced3c7af885e3fdc71b2 Mon Sep 17 00:00:00 2001 From: lihaowei Date: Tue, 22 Jun 2021 20:12:47 +0800 Subject: [PATCH 4/8] fix err Signed-off-by: lihaowei --- planner/core/preprocess.go | 13 ++++++++++--- planner/core/preprocess_test.go | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/planner/core/preprocess.go b/planner/core/preprocess.go index a85ef31862408..dffa6f335c9a4 100644 --- a/planner/core/preprocess.go +++ b/planner/core/preprocess.go @@ -833,8 +833,15 @@ func (p *preprocessor) checkDropSequenceGrammar(stmt *ast.DropSequenceStmt) { } func (p *preprocessor) checkDropTableGrammar(stmt *ast.DropTableStmt) { - currentDB := model.NewCIStr(p.ctx.GetSessionVars().CurrentDB) enableNoopFuncs := p.ctx.GetSessionVars().EnableNoopFuncs + if stmt.TemporaryKeyword == ast.TemporaryLocal && !enableNoopFuncs { + p.err = expression.ErrFunctionsNoopImpl.GenWithStackByArgs("DROP TEMPORARY TABLE") + return + } + currentDB := model.NewCIStr(p.ctx.GetSessionVars().CurrentDB) + if stmt.TemporaryKeyword == ast.TemporaryNone { + return + } for _, t := range stmt.Tables { if isIncorrectName(t.Name.String()) { p.err = ddl.ErrWrongTableName.GenWithStackByArgs(t.Name.String()) @@ -848,8 +855,8 @@ func (p *preprocessor) checkDropTableGrammar(stmt *ast.DropTableStmt) { p.err = err return } - if stmt.TemporaryKeyword != ast.TemporaryNone && !enableNoopFuncs && tableInfo.Meta().TempTableType == model.TempTableNone { - p.err = expression.ErrFunctionsNoopImpl.GenWithStackByArgs("DROP TEMPORARY TABLE") + if tableInfo.Meta().TempTableType == model.TempTableNone { + p.err = ErrOptOnTemporaryTable.GenWithStackByArgs("drop temporary table") return } } diff --git a/planner/core/preprocess_test.go b/planner/core/preprocess_test.go index 40d963ad51d5e..6d4fa870b5dda 100644 --- a/planner/core/preprocess_test.go +++ b/planner/core/preprocess_test.go @@ -357,6 +357,6 @@ func (s *testValidatorSuite) TestDropGlobalTempTable(c *C) { c.Assert(err, IsNil) } s.is = s.dom.InfoSchema() - s.runSQL(c, "drop global temporary table tb;", false, expression.ErrFunctionsNoopImpl.GenWithStackByArgs("DROP TEMPORARY TABLE")) + s.runSQL(c, "drop global temporary table tb;", false, core.ErrOptOnTemporaryTable.GenWithStackByArgs("drop temporary table")) s.runSQL(c, "drop global temporary table temp", false, nil) } From 561780c308d17d6aeddef20620e61b78d17dddb7 Mon Sep 17 00:00:00 2001 From: lihaowei Date: Wed, 23 Jun 2021 22:33:58 +0800 Subject: [PATCH 5/8] fix err --- planner/core/preprocess.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/planner/core/preprocess.go b/planner/core/preprocess.go index 37eda3f466b54..5e15a37602a1b 100644 --- a/planner/core/preprocess.go +++ b/planner/core/preprocess.go @@ -833,15 +833,16 @@ func (p *preprocessor) checkDropSequenceGrammar(stmt *ast.DropSequenceStmt) { } func (p *preprocessor) checkDropTableGrammar(stmt *ast.DropTableStmt) { + p.checkDropTableNames(stmt.Tables) enableNoopFuncs := p.ctx.GetSessionVars().EnableNoopFuncs if stmt.TemporaryKeyword == ast.TemporaryLocal && !enableNoopFuncs { p.err = expression.ErrFunctionsNoopImpl.GenWithStackByArgs("DROP TEMPORARY TABLE") return } - currentDB := model.NewCIStr(p.ctx.GetSessionVars().CurrentDB) if stmt.TemporaryKeyword == ast.TemporaryNone { return } + currentDB := model.NewCIStr(p.ctx.GetSessionVars().CurrentDB) for _, t := range stmt.Tables { if isIncorrectName(t.Name.String()) { p.err = ddl.ErrWrongTableName.GenWithStackByArgs(t.Name.String()) @@ -855,7 +856,7 @@ func (p *preprocessor) checkDropTableGrammar(stmt *ast.DropTableStmt) { p.err = err return } - if tableInfo.Meta().TempTableType == model.TempTableNone { + if tableInfo.Meta().TempTableType != model.TempTableGlobal { p.err = ErrOptOnTemporaryTable.GenWithStackByArgs("drop temporary table") return } From 7e507ee0e746512ed058eb3b303b32852d8c6c56 Mon Sep 17 00:00:00 2001 From: lihaowei Date: Thu, 24 Jun 2021 12:28:20 +0800 Subject: [PATCH 6/8] define err --- errno/errcode.go | 1 + errno/errname.go | 1 + errors.toml | 5 +++++ planner/core/errors.go | 11 ++++++----- planner/core/preprocess.go | 2 +- planner/core/preprocess_test.go | 2 +- 6 files changed, 15 insertions(+), 7 deletions(-) diff --git a/errno/errcode.go b/errno/errcode.go index d6b7c912a15d9..319ddd092bd82 100644 --- a/errno/errcode.go +++ b/errno/errcode.go @@ -923,6 +923,7 @@ const ( ErrTxnTooLarge = 8004 ErrWriteConflictInTiDB = 8005 ErrOptOnTemporaryTable = 8006 + ErrDropTableOnTemporaryTable = 8007 ErrUnsupportedReloadPlugin = 8018 ErrUnsupportedReloadPluginVar = 8019 ErrTableLocked = 8020 diff --git a/errno/errname.go b/errno/errname.go index 6ab983acc3f29..1fdb6b82b560b 100644 --- a/errno/errname.go +++ b/errno/errname.go @@ -925,6 +925,7 @@ var MySQLErrName = map[uint16]*mysql.ErrMessage{ ErrForUpdateCantRetry: mysql.Message("[%d] can not retry select for update statement", nil), ErrAdminCheckTable: mysql.Message("TiDB admin check table failed.", nil), ErrOptOnTemporaryTable: mysql.Message("`%s` is unsupported on temporary tables.", nil), + ErrDropTableOnTemporaryTable: mysql.Message("`drop global temporary table` can only drop global temporary table", nil), ErrTxnTooLarge: mysql.Message("Transaction is too large, size: %d", nil), ErrWriteConflictInTiDB: mysql.Message("Write conflict, txnStartTS %d is stale", nil), ErrInvalidPluginID: mysql.Message("Wrong plugin id: %s, valid plugin id is [name]-[version], both name and version should not contain '-'", nil), diff --git a/errors.toml b/errors.toml index 165226659c51e..f7508f8dba983 100644 --- a/errors.toml +++ b/errors.toml @@ -1161,6 +1161,11 @@ error = ''' `%s` is unsupported on temporary tables. ''' +["planner:8007"] +error = ''' +`drop global temporary table` can only drop global temporary table +''' + ["planner:8108"] error = ''' Unsupported type %T diff --git a/planner/core/errors.go b/planner/core/errors.go index c1166f6f288b3..e501af7676c5f 100644 --- a/planner/core/errors.go +++ b/planner/core/errors.go @@ -94,11 +94,12 @@ var ( ErrCTERecursiveForbiddenJoinOrder = dbterror.ClassOptimizer.NewStd(mysql.ErrCTERecursiveForbiddenJoinOrder) ErrInvalidRequiresSingleReference = dbterror.ClassOptimizer.NewStd(mysql.ErrInvalidRequiresSingleReference) // Since we cannot know if user logged in with a password, use message of ErrAccessDeniedNoPassword instead - ErrAccessDenied = dbterror.ClassOptimizer.NewStdErr(mysql.ErrAccessDenied, mysql.MySQLErrName[mysql.ErrAccessDeniedNoPassword]) - ErrBadNull = dbterror.ClassOptimizer.NewStd(mysql.ErrBadNull) - ErrNotSupportedWithSem = dbterror.ClassOptimizer.NewStd(mysql.ErrNotSupportedWithSem) - ErrAsOf = dbterror.ClassOptimizer.NewStd(mysql.ErrAsOf) - ErrOptOnTemporaryTable = dbterror.ClassOptimizer.NewStd(mysql.ErrOptOnTemporaryTable) + ErrAccessDenied = dbterror.ClassOptimizer.NewStdErr(mysql.ErrAccessDenied, mysql.MySQLErrName[mysql.ErrAccessDeniedNoPassword]) + ErrBadNull = dbterror.ClassOptimizer.NewStd(mysql.ErrBadNull) + ErrNotSupportedWithSem = dbterror.ClassOptimizer.NewStd(mysql.ErrNotSupportedWithSem) + ErrAsOf = dbterror.ClassOptimizer.NewStd(mysql.ErrAsOf) + ErrOptOnTemporaryTable = dbterror.ClassOptimizer.NewStd(mysql.ErrOptOnTemporaryTable) + ErrDropTableOnTemporaryTable = dbterror.ClassOptimizer.NewStd(mysql.ErrDropTableOnTemporaryTable) // ErrPartitionNoTemporary returns when partition at temporary mode ErrPartitionNoTemporary = dbterror.ClassOptimizer.NewStd(mysql.ErrPartitionNoTemporary) ) diff --git a/planner/core/preprocess.go b/planner/core/preprocess.go index 5e15a37602a1b..5c3cf73af2a46 100644 --- a/planner/core/preprocess.go +++ b/planner/core/preprocess.go @@ -857,7 +857,7 @@ func (p *preprocessor) checkDropTableGrammar(stmt *ast.DropTableStmt) { return } if tableInfo.Meta().TempTableType != model.TempTableGlobal { - p.err = ErrOptOnTemporaryTable.GenWithStackByArgs("drop temporary table") + p.err = ErrDropTableOnTemporaryTable return } } diff --git a/planner/core/preprocess_test.go b/planner/core/preprocess_test.go index 6d4fa870b5dda..af4026af77057 100644 --- a/planner/core/preprocess_test.go +++ b/planner/core/preprocess_test.go @@ -357,6 +357,6 @@ func (s *testValidatorSuite) TestDropGlobalTempTable(c *C) { c.Assert(err, IsNil) } s.is = s.dom.InfoSchema() - s.runSQL(c, "drop global temporary table tb;", false, core.ErrOptOnTemporaryTable.GenWithStackByArgs("drop temporary table")) + s.runSQL(c, "drop global temporary table tb;", false, core.ErrDropTableOnTemporaryTable) s.runSQL(c, "drop global temporary table temp", false, nil) } From 0a9caba4625305fd10f73d05a7c4a267f0cf3a1a Mon Sep 17 00:00:00 2001 From: lihaowei Date: Thu, 24 Jun 2021 15:24:05 +0800 Subject: [PATCH 7/8] add more tests --- planner/core/preprocess_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/planner/core/preprocess_test.go b/planner/core/preprocess_test.go index af4026af77057..45341b439e4f2 100644 --- a/planner/core/preprocess_test.go +++ b/planner/core/preprocess_test.go @@ -359,4 +359,6 @@ func (s *testValidatorSuite) TestDropGlobalTempTable(c *C) { s.is = s.dom.InfoSchema() s.runSQL(c, "drop global temporary table tb;", false, core.ErrDropTableOnTemporaryTable) s.runSQL(c, "drop global temporary table temp", false, nil) + s.runSQL(c, "drop global temporary table test.tb;", false, core.ErrDropTableOnTemporaryTable) + s.runSQL(c, "drop global temporary table test.temp", false, nil) } From a59fa6ca080de2f9a40520b60df7459d2ca1da80 Mon Sep 17 00:00:00 2001 From: lihaowei Date: Thu, 24 Jun 2021 15:45:27 +0800 Subject: [PATCH 8/8] mistakes --- planner/core/preprocess_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/planner/core/preprocess_test.go b/planner/core/preprocess_test.go index 45341b439e4f2..e32148c572728 100644 --- a/planner/core/preprocess_test.go +++ b/planner/core/preprocess_test.go @@ -351,6 +351,7 @@ func (s *testValidatorSuite) TestDropGlobalTempTable(c *C) { "set tidb_enable_global_temporary_table=true", "create table tb(id int);", "create global temporary table temp(id int) on commit delete rows;", + "create global temporary table temp1(id int) on commit delete rows;", } for _, execSQL := range execSQLList { _, err := s.se.Execute(ctx, execSQL) @@ -360,5 +361,5 @@ func (s *testValidatorSuite) TestDropGlobalTempTable(c *C) { s.runSQL(c, "drop global temporary table tb;", false, core.ErrDropTableOnTemporaryTable) s.runSQL(c, "drop global temporary table temp", false, nil) s.runSQL(c, "drop global temporary table test.tb;", false, core.ErrDropTableOnTemporaryTable) - s.runSQL(c, "drop global temporary table test.temp", false, nil) + s.runSQL(c, "drop global temporary table test.temp1", false, nil) }